Release/1.1.12#200
Conversation
* RDKEMW-5849 : remove deprecated "experience" code Reason for change: the app controls experience, ctrlm does is no longer involved Test Procedure: nothing new to test. Check that normal functionality is not affected Priority; P2 Signed-off-by: Jason Thomson <jason_thomson@comcast.com> * Align CTRLM_VOICE_QUERY_STRING_MAX_PAIRS with xrsr without including xrsr.h --------- Signed-off-by: Jason Thomson <jason_thomson@comcast.com>
* RDKEMW-16711: Update Thunder plugin to use _string and _boolean Reason for change: Thunder API now returns a string for last wakeup reason and a boolean for Networked Standby Mode Enabled, rather than a JsonObject. Updating ctrlm accordingly Test Procedure: NSM testing Priority: P0 Risks: Low Signed-off-by: Jason Thomson <jason_thomson@comcast.com> * Update ctrlm_thunder_plugin.h Address co-pilot comments * Update ctrlm_thunder_plugin.h * Update ctrlm_thunder_plugin.h --------- Signed-off-by: Jason Thomson <jason_thomson@comcast.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Release 1.1.12 removes deprecated “experience” authentication plumbing from voice/auth/network layers and modernizes Thunder plugin calls by adding typed helpers for boolean/string results.
Changes:
- Removed “experience” getters/setters and AUTH_EXPERIENCE gating across voice endpoints, authservice, and network/controller APIs.
- Added
call_plugin_boolean/call_plugin_stringhelpers and updated PowerManager calls to use them. - Increased max voice query-string pairs from 16 to 24 and updated CHANGELOG for 1.1.12.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/voice/endpoints/ctrlm_voice_endpoint_ws_nextgen.h | Removes experience setter from websocket nextgen endpoint interface. |
| src/voice/endpoints/ctrlm_voice_endpoint_ws_nextgen.cpp | Stops sending/updating experience for WS nextgen sessions. |
| src/voice/endpoints/ctrlm_voice_endpoint_http.h | Removes experience setter from HTTP endpoint interface. |
| src/voice/endpoints/ctrlm_voice_endpoint_http.cpp | Stops sending/updating experience for HTTP sessions. |
| src/voice/endpoints/ctrlm_voice_endpoint.h | Removes virtual experience setter from base endpoint API. |
| src/voice/endpoints/ctrlm_voice_endpoint.cpp | Removes base no-op implementation for experience setter. |
| src/voice/ctrlm_voice_obj.h | Removes experience field + voice_stb_data experience accessors. |
| src/voice/ctrlm_voice_obj.cpp | Removes experience propagation and AUTH_EXPERIENCE gating in session validation. |
| src/thunder/ctrlm_thunder_plugin_powermanager.cpp | Updates PowerManager queries to use typed Thunder helpers; adjusts logging. |
| src/thunder/ctrlm_thunder_plugin.h | Fixes doc text and adds typed Thunder call helpers declarations. |
| src/thunder/ctrlm_thunder_plugin.cpp | Implements typed Thunder call helpers for bool and string responses. |
| src/ctrlm_network.h | Removes experience storage/accessors from network object. |
| src/ctrlm_network.cpp | Removes experience setter/getter implementation. |
| src/ctrlm_main.cpp | Removes AUTH_EXPERIENCE load/has checks and propagation. |
| src/ctrlm_controller.h | Removes controller experience getter API. |
| src/ctrlm_controller.cpp | Removes controller experience getter implementation. |
| src/ctrlm.h | Removes ctrlm_main_has_experience_get declaration. |
| src/auth/ctrlm_thunder_plugin_authservice.h | Removes getExperience API from AuthService plugin wrapper. |
| src/auth/ctrlm_thunder_plugin_authservice.cpp | Removes getExperience implementation. |
| src/auth/ctrlm_auth_thunder.h | Removes get_experience from Thunder auth adapter. |
| src/auth/ctrlm_auth_thunder.cpp | Removes get_experience implementation. |
| src/auth/ctrlm_auth.h | Removes get_experience from auth interface. |
| include/ctrlm_ipc_voice.h | Increases max query-string pairs constant to 24. |
| CMakeLists.txt | Removes commented AUTH_EXPERIENCE compile-definition hint. |
| CHANGELOG.md | Adds 1.1.12 entry and adjusts auto-changelog marker/date formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if(this->call_plugin_string("getLastWakeupReason", (void *)¶ms, &response)) { | ||
| if(response == "VOICE") { | ||
| wakeup_reason_voice = true; | ||
| } | ||
| } else { | ||
| XLOGD_ERROR("getLastWakeupReason call failed"); | ||
| } |
There was a problem hiding this comment.
This changes the wakeup-reason check semantics from a prefix match (strncmp(..., \"VOICE\", 5)) to an exact match (response == \"VOICE\"). If the Thunder API can return values like VOICE_* (or adds suffixes in the future), voice wakeups will be incorrectly treated as false. Consider restoring a prefix check (e.g., starts-with "VOICE") to preserve prior behavior, unless the API contract guarantees the value is exactly "VOICE".
| * @return True if the call succeeded, otherwise False. | ||
| */ | ||
| bool call_plugin_boolean(std::string method, void *params, bool *response); | ||
|
|
||
| /** | ||
| * 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 The string pointer which will be assigned containing the response from the call. | ||
| * @return True if the call succeeded, otherwise False. | ||
| */ | ||
| bool call_plugin_string(std::string method, void *params, std::string *response); |
There was a problem hiding this comment.
The new call_plugin_boolean / call_plugin_string helpers don’t expose the retries parameter that call_plugin(...) provides, which makes the Thunder-call API inconsistent and reduces flexibility for callers (especially for transient timeout scenarios). Consider adding an optional unsigned int retries = 0 parameter (matching call_plugin) or refactoring to reuse the existing retry logic so all call variants behave consistently.
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_boolean(std::string method, void *params, bool *response); | |
| /** | |
| * 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 The string pointer which will be assigned containing the response from the call. | |
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_string(std::string method, void *params, std::string *response); | |
| * @param retries The number of retries if the call times out. | |
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_boolean(std::string method, void *params, bool *response, unsigned int retries = 0); | |
| /** | |
| * 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 The string pointer which will be assigned containing the response from the call. | |
| * @param retries The number of retries if the call times out. | |
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_string(std::string method, void *params, std::string *response, unsigned int retries = 0); |
| * @return True if the call succeeded, otherwise False. | ||
| */ | ||
| bool call_plugin_boolean(std::string method, void *params, bool *response); | ||
|
|
||
| /** | ||
| * 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 The string pointer which will be assigned containing the response from the call. | ||
| * @return True if the call succeeded, otherwise False. | ||
| */ | ||
| bool call_plugin_string(std::string method, void *params, std::string *response); |
There was a problem hiding this comment.
The new call_plugin_boolean / call_plugin_string helpers don’t expose the retries parameter that call_plugin(...) provides, which makes the Thunder-call API inconsistent and reduces flexibility for callers (especially for transient timeout scenarios). Consider adding an optional unsigned int retries = 0 parameter (matching call_plugin) or refactoring to reuse the existing retry logic so all call variants behave consistently.
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_boolean(std::string method, void *params, bool *response); | |
| /** | |
| * 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 The string pointer which will be assigned containing the response from the call. | |
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_string(std::string method, void *params, std::string *response); | |
| * @param retries The number of retries if the call times out. | |
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_boolean(std::string method, void *params, bool *response, unsigned int retries = 0); | |
| /** | |
| * 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 The string pointer which will be assigned containing the response from the call. | |
| * @param retries The number of retries if the call times out. | |
| * @return True if the call succeeded, otherwise False. | |
| */ | |
| bool call_plugin_string(std::string method, void *params, std::string *response, unsigned int retries = 0); |
| } else { | ||
| XLOGD_ERROR("Invalid parameters"); | ||
| } | ||
| } else { | ||
| XLOGD_ERROR("Client is NULL"); | ||
| } |
There was a problem hiding this comment.
The new error logs are very generic and lose useful context for debugging (e.g., which method was invoked and which pointer was null). Consider including method and explicit null details (clientObject/jsonParams/response) in the log message so failures can be triaged from logs without reproducing locally.
| } else { | ||
| XLOGD_ERROR("getLastWakeupReason call failed"); | ||
| } | ||
| sem_post(&this->semaphore); | ||
|
|
||
| XLOGD_DEBUG("voice_wakeup is %s", wakeup_reason_voice?"TRUE":"FALSE"); |
There was a problem hiding this comment.
The debug log prints voice_wakeup even when the Thunder call fails, which can be misleading in logs (it will always print FALSE after a failure). Consider logging the debug message only on success, or including success/failure context in the debug line (e.g., voice_wakeup=<...> (call_failed)) to avoid misinterpretation during operational troubleshooting.
dwolaver
left a comment
There was a problem hiding this comment.
Good luck getting this one past copilot review!
https://ccp.sys.comcast.net/browse/RDKEMW-16915