Update ctrlm_thunder_plugin_powermanager.cpp#193
Update ctrlm_thunder_plugin_powermanager.cpp#193jthomp007c wants to merge 3 commits intosupport/1.1.11from
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the PowerManager Thunder plugin integration to determine whether the last wakeup reason was voice-related, affecting how Control Manager interprets wake events.
Changes:
- Changed
get_wakeup_reason_voice()to use an exact-string comparison for the wakeup reason value.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sem_wait(&this->semaphore); | ||
| if(this->call_plugin("getLastWakeupReason", (void *)¶ms, (void *)&response)) { | ||
| wakeup_reason_voice = (0 == strncmp(response["result"].String().c_str(), "VOICE", 5)); | ||
| wakeup_reason_voice = (response["result"].String() == "WAKEUP_REASON_VOICE"); |
There was a problem hiding this comment.
The wakeup reason check changed from a prefix match on "VOICE" to strict equality with "WAKEUP_REASON_VOICE". In this file’s own curl example, getLastWakeupReason returns plain strings like "COLDBOOT", and there are no other references in the repo to a "WAKEUP_REASON_VOICE" value; as written this will likely never evaluate true and break voice-wakeup detection. Consider comparing against the actual PowerManager response value(s) (e.g., "VOICE" / other documented strings) or preserving the previous prefix/enum-style match if multiple VOICE-related reasons are expected.
| wakeup_reason_voice = (response["result"].String() == "WAKEUP_REASON_VOICE"); | |
| string wakeup_reason = response["result"].String(); | |
| wakeup_reason_voice = (wakeup_reason == "VOICE") || | |
| (wakeup_reason.rfind("VOICE", 0) == 0) || | |
| (wakeup_reason.rfind("WAKEUP_REASON_VOICE", 0) == 0); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sem_wait(&this->semaphore); | ||
| if(this->call_plugin("getLastWakeupReason", (void *)¶ms, (void *)&response)) { | ||
| wakeup_reason_voice = (0 == strncmp(response["result"].String().c_str(), "VOICE", 5)); | ||
| wakeup_reason_voice = (response["wakeupReason"].String() == "WAKEUP_REASON_VOICE"); | ||
| XLOGD_DEBUG("voice_wakeup is %s", wakeup_reason_voice?"TRUE":"FALSE"); |
There was a problem hiding this comment.
get_wakeup_reason_voice() now reads response["wakeupReason"] and compares it to "WAKEUP_REASON_VOICE", but the surrounding inline curl example (and the previous implementation) indicate the method returns a string in the result field (e.g., {"result":"COLDBOOT"}), with voice detection based on a "VOICE" prefix. As written, this will likely always evaluate to false on existing PowerManager responses.
Consider parsing the response in a backward-compatible way (e.g., check for wakeupReason first, otherwise fall back to result), and guard access with HasLabel() to avoid relying on default/empty values. Also update the inline curl example if the plugin API truly changed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| XLOGD_INFO("result received <%s>", response["result"].String().c_str()); | ||
| XLOGD_INFO("wakeupReason received <%s>", response["wakeupReason"].String().c_str()); | ||
| wakeup_reason_voice = (std::string(response["result"].String()) == "VOICE"); |
There was a problem hiding this comment.
getLastWakeupReason response handling should validate expected labels before indexing. The inline curl example above shows the response contains only result (e.g., "COLDBOOT"), so unconditionally reading/logging response["wakeupReason"] risks logging misleading data (or returning an empty string) when that label isn’t present. Consider checking response.HasLabel("result") (and "wakeupReason" only if the plugin actually returns it) and treating a missing/unknown payload as a malformed response like get_power_state() does.
No description provided.