Update ctrlm_thunder_plugin_powermanager.cpp - #193
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.
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.
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.
Address copilot comments
There was a problem hiding this comment.
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_controllerdocblock lists@param paramstwice; the second one is describing the response object and should be@param responseto 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.
| } 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.
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.
| } 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); |
There was a problem hiding this comment.
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.
| * 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 |
There was a problem hiding this comment.
Docstring punctuation/grammar: add a trailing period to the @param property description to match the surrounding documentation style.
| * @param property The property the user wants to get | |
| * @param property The property the user wants to get. |
| 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 { |
There was a problem hiding this comment.
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.
No description provided.