RDKEMW-14517: Use IARM for PowerManager clients#137
RDKEMW-14517: Use IARM for PowerManager clients#137apatel859 merged 3 commits intosupport/2.16.9from
Conversation
Signed-off-by: apatel859 <amit_patel5@comcast.com>
Signed-off-by: apatel859 <amit_patel5@comcast.com>
RDKEMW-13117: Use IARM for PowerManager clients
There was a problem hiding this comment.
Pull request overview
This pull request migrates the FrontPanel plugin from using the WPEFramework PowerManager COM-RPC interface to using IARM-based PowerManager for power state management. This change simplifies the architecture by removing the dependency on the PowerManager plugin interface and using direct IARM bus communication instead.
Changes:
- Replaced PowerManager COM-RPC interface with IARM bus calls for power state queries and event notifications
- Removed PowerManagerInterfaceRef and associated notification handler infrastructure
- Simplified CFrontPanel::instance() to no longer require PluginHost::IShell parameter
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| helpers/frontpanel.h | Removed PluginHost::IShell parameter from instance() method signature |
| helpers/frontpanel.cpp | Replaced PowerManager interface with direct IARM_Bus_Call for power state queries |
| FrontPanel/FrontPanel.h | Removed PowerManagerNotification class and related members; added IARM initialization methods |
| FrontPanel/FrontPanel.cpp | Replaced PowerManager event registration with IARM event handler registration; implemented IARM-based power mode change handling |
| if (strcmp(owner, IARM_BUS_PWRMGR_NAME) == 0) { | ||
| if (eventId == IARM_BUS_PWRMGR_EVENT_MODECHANGED ) { | ||
| IARM_Bus_PWRMgr_EventData_t *param = (IARM_Bus_PWRMgr_EventData_t *)data; | ||
| LOGINFO("Event IARM_BUS_PWRMGR_EVENT_MODECHANGED: State Changed %d -- > %d\r", |
There was a problem hiding this comment.
The logging format string contains '\r' (carriage return) which is inconsistent with other LOGINFO statements in the codebase. Line 275 uses '\n' (newline) instead. For consistency, this should use '\n' or no line ending at all, as LOGINFO likely adds its own line termination.
| LOGINFO("Event IARM_BUS_PWRMGR_EVENT_MODECHANGED: State Changed %d -- > %d\r", | |
| LOGINFO("Event IARM_BUS_PWRMGR_EVENT_MODECHANGED: State Changed %d -- > %d", |
| IARM_Result_t res; | ||
| IARM_CHECK( IARM_Bus_RegisterEventHandler(IARM_BUS_PWRMGR_NAME,IARM_BUS_PWRMGR_EVENT_MODECHANGED, powerModeChange) ); |
There was a problem hiding this comment.
The variable 'res' is declared but never used after the IARM_CHECK macro call. The IARM_CHECK macro internally uses 'res', but this requires 'res' to be declared in the scope. However, this pattern creates unused variable warnings in some contexts. Consider whether the variable is needed or if the macro handles all necessary error checking internally.
| IARM_Result_t res; | ||
| IARM_CHECK( IARM_Bus_RemoveEventHandler(IARM_BUS_PWRMGR_NAME,IARM_BUS_PWRMGR_EVENT_MODECHANGED, powerModeChange) ); |
There was a problem hiding this comment.
The variable 'res' is declared but never used after the IARM_CHECK macro call. This is the same pattern as in InitializeIARM. Consider whether the variable is needed or if the macro handles all necessary error checking internally.
|
|
||
| if (res == IARM_RESULT_SUCCESS) { | ||
| if (param.curState == IARM_BUS_PWRMGR_POWERSTATE_ON) | ||
| powerStatus = true; |
There was a problem hiding this comment.
The IARM_Bus_Call result is checked, but if it fails (res != IARM_RESULT_SUCCESS), powerStatus remains false without any logging or error handling. This differs from the previous implementation which logged the power state. Consider adding error logging when the IARM call fails to aid debugging.
| powerStatus = true; | |
| powerStatus = true; | |
| } else { | |
| LOGERR("IARM_Bus_Call(%s, %s) failed with result %d; leaving powerStatus as %d", | |
| IARM_BUS_PWRMGR_NAME, IARM_BUS_PWRMGR_API_GetPowerState, res, powerStatus); |
RDKEMW-14517: Use IARM based PowerManager
Signed-off-by: Yuvaramachandran Gurusamy [yuvaramachandran_gurusamy@comcast.com]