Update ctrlm_thunder_plugin_powermanager.cpp by jthomp007c · Pull Request #193 · rdkcentral/control

jthomp007c · 2026-04-03T11:53:17Z

No description provided.

Copilot

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.

Copilot · 2026-04-03T11:55:08Z

   sem_wait(&this->semaphore);   
   if(this->call_plugin("getLastWakeupReason", (void *)&params, (void *)&response)) {
-      wakeup_reason_voice = (0 == strncmp(response["result"].String().c_str(), "VOICE", 5));
+      wakeup_reason_voice = (response["result"].String() == "WAKEUP_REASON_VOICE");


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.

Suggested change

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);

Copilot

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.

Copilot · 2026-04-03T13:19:36Z

   sem_wait(&this->semaphore);   
   if(this->call_plugin("getLastWakeupReason", (void *)&params, (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");


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.

Copilot

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.

Copilot · 2026-04-03T16:42:45Z

+      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");


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.

Address copilot comments

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/thunder/ctrlm_thunder_plugin.h:150

The call_controller docblock lists @param params twice; the second one is describing the response object and should be @param response to match the function signature (..., void *params, void *response).

     * This function is used to call a Thunder Controller method.
     * @param method The method in which the user wants to call.
     * @param params The WPEFramework JsonObject containing the parameters for the call. (We can't include WPEFramework headers in controlMgr .h files as their logging macros clash)
     * @param params The WPEFramework JsonObject containing the response from the call.  (We can't include WPEFramework headers in controlMgr .h files as their logging macros clash)
     * @return True if the call succeeded, otherwise False.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T18:04:05Z

     * This function is used to get a Thunder Plugin property.
-     * @param method The method in which the user wants to call.
+     * @param property The property the user wants to get
     * @param params The WPEFramework JsonObject containing the parameters for the call. (We can't include WPEFramework headers in controlMgr .h files as their logging macros clash)


The property_get docblock still lists a params parameter, but the function signature is property_get(std::string property, void *response, unsigned int retries = 0) and has no params argument. This can confuse callers; update the comment to describe only property, response, and retries (or adjust the API if params are actually required).

Suggested change

* @param params The WPEFramework JsonObject containing the parameters for the call. (We can't include WPEFramework headers in controlMgr .h files as their logging macros clash)

Copilot · 2026-04-08T18:04:06Z

+     * This function is used to call a Thunder Plugin method.
+     * @param method The method in which the user wants to call.
+     * @param params The WPEFramework JsonObject containing the parameters for the call. (We can't include WPEFramework headers in controlMgr .h files as their logging macros clash)
+     * @param response A pointer a string which will be assigned


Typo/grammar in the new call_plugin_string documentation: "A pointer a string" is missing "to", and the description is incomplete. Please clarify that response is an output parameter assigned to the returned string.

Suggested change

* @param response A pointer a string which will be assigned

* @param response A pointer to a string output parameter which will be assigned the returned string.

Copilot · 2026-04-08T18:04:06Z

+               XLOGD_ERROR("%s: Thunder call failed <%s> <%u>", __FUNCTION__, method.c_str(), thunderRet);
+            } else {
+                *response = jsonString.Value();
+                ret = true;
+            }
+        } else {
+            XLOGD_ERROR("%s: Invalid parameters", __FUNCTION__);
+        }
+    } else {
+        XLOGD_ERROR("%s: Client is NULL", __FUNCTION__);


call_plugin_string uses __FUNCTION__ in its error logs, but the other call helpers in this file (property_get, call_plugin, call_plugin_boolean) log without a function prefix (e.g., XLOGD_ERROR("Invalid parameters")). For consistency (and to avoid duplicating function names if the logger already adds them), align the new logging format with the existing helpers in this file.

Suggested change

XLOGD_ERROR("%s: Thunder call failed <%s> <%u>", __FUNCTION__, method.c_str(), thunderRet);

} else {

*response = jsonString.Value();

ret = true;

}

} else {

XLOGD_ERROR("%s: Invalid parameters", __FUNCTION__);

}

} else {

XLOGD_ERROR("%s: Client is NULL", __FUNCTION__);

XLOGD_ERROR("Thunder call failed <%s> <%u>", method.c_str(), thunderRet);

} else {

*response = jsonString.Value();

ret = true;

}

} else {

XLOGD_ERROR("Invalid parameters");

}

} else {

XLOGD_ERROR("Client is NULL");

Copilot · 2026-04-08T18:04:07Z

   } else {
      XLOGD_ERROR("getLastWakeupReason call failed");
   }
   sem_post(&this->semaphore);

+   XLOGD_DEBUG("voice_wakeup is %s", wakeup_reason_voice?"TRUE":"FALSE");
+


voice_wakeup is now logged unconditionally, including when getLastWakeupReason fails. This can be misleading because failures will be reported as FALSE in debug logs even though the value is actually unknown. Consider logging only on success, or include an explicit "call failed"/"unknown" indication in the debug message.

Suggested change

} else {

XLOGD_ERROR("getLastWakeupReason call failed");

}

sem_post(&this->semaphore);

XLOGD_DEBUG("voice_wakeup is %s", wakeup_reason_voice?"TRUE":"FALSE");

XLOGD_DEBUG("voice_wakeup is %s", wakeup_reason_voice?"TRUE":"FALSE");

} else {

XLOGD_ERROR("getLastWakeupReason call failed");

}

sem_post(&this->semaphore);

Fix error log

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T18:22:17Z

     * This function is used to get a Thunder Plugin property.
-     * @param method The method in which the user wants to call.
-     * @param params The WPEFramework JsonObject containing the parameters for the call. (We can't include WPEFramework headers in controlMgr .h files as their logging macros clash)
+     * @param property The property the user wants to get


Docstring punctuation/grammar: add a trailing period to the @param property description to match the surrounding documentation style.

Suggested change

* @param property The property the user wants to get

* @param property The property the user wants to get.

Copilot · 2026-04-08T18:22:18Z

+bool ctrlm_thunder_plugin_t::call_plugin_string(std::string method, void *params, std::string *response) {
+    bool ret = false;
+    auto clientObject = (JSONRPC::LinkType<Core::JSON::IElement>*)this->plugin_client;
+    JsonObject *jsonParams = (JsonObject *)params;
+    if(clientObject) {
+        if(!method.empty() && jsonParams && response) {
+            Core::JSON::String jsonString;
+            uint32_t thunderRet = clientObject->Invoke<JsonObject, Core::JSON::String>(CALL_TIMEOUT, _T(method), *jsonParams, jsonString);
+            if(thunderRet != Core::ERROR_NONE) {
+               XLOGD_ERROR("Thunder call failed <%s> <%u>", method.c_str(), thunderRet);
+            } else {


call_plugin_string largely duplicates call_plugin_boolean (client cast, parameter validation, Invoke, error handling). Consider extracting a small internal templated helper for primitive-return Invoke calls to reduce duplication and keep logging/handling consistent as more typed helpers are added.

Update ctrlm_thunder_plugin_powermanager.cpp

d98d86e

jthomp007c requested a review from a team as a code owner April 3, 2026 11:53

Copilot AI review requested due to automatic review settings April 3, 2026 11:53

Copilot started reviewing on behalf of jthomp007c April 3, 2026 11:53 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

Update ctrlm_thunder_plugin_powermanager.cpp

d2329ca

Copilot AI review requested due to automatic review settings April 3, 2026 13:17

Copilot started reviewing on behalf of jthomp007c April 3, 2026 13:17 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

Update ctrlm_thunder_plugin_powermanager.cpp

4176268

Copilot AI review requested due to automatic review settings April 3, 2026 16:40

Copilot started reviewing on behalf of jthomp007c April 3, 2026 16:40 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

jthomp007c added 2 commits April 6, 2026 12:28

Adding JSON string method to plugin

11ea30b

Update ctrlm_thunder_plugin.h

d35e7da

Address copilot comments

Copilot AI review requested due to automatic review settings April 8, 2026 17:57

Copilot started reviewing on behalf of jthomp007c April 8, 2026 17:59 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

jthomp007c added 2 commits April 8, 2026 14:15

Update ctrlm_thunder_plugin.cpp

2a3bc2a

Fix error log

Update ctrlm_thunder_plugin.h

3fd1df7

Copilot AI review requested due to automatic review settings April 8, 2026 18:18

Copilot started reviewing on behalf of jthomp007c April 8, 2026 18:19 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Update ctrlm_thunder_plugin.h

ef652ff

-      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);

	* @param response A pointer a string which will be assigned
	* @param response A pointer to a string output parameter which will be assigned the returned string.

	* @param property The property the user wants to get
	* @param property The property the user wants to get.

Conversation

jthomp007c commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants