RDKEMW-14517: Use IARM for PowerManager clients#49
RDKEMW-14517: Use IARM for PowerManager clients#49yuvaramachandran-gurusamy wants to merge 7 commits intotopic/support_2.16.9_Testfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the power-state helper tools to use the IARM PowerManager APIs instead of the WPEFramework PowerController/Thunder dependency, aligning clients with the IARM-based PowerManager for RDKEMW-14517.
Changes:
- Reworked
SetPowerStatetool to callIARM_BUS_PWRMGR_API_SetPowerStatevia IARM. - Reworked
QueryPowerStatetool to query power state viaIARM_BUS_PWRMGR_API_GetPowerState(and still supports reading/opt/uimgr_settings.bin). - Updated build/link flags to remove
-lWPEFrameworkPowerControllerusage and link against-lIARMBus; removed.gitignore.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| iarm_set_powerstate/IARM_BUS_SetPowerStatus.c | Switches power-state setting implementation to IARM bus calls. |
| iarm_query_powerstate/IARM_Bus_CheckPowerStatus.c | Switches power-state query implementation to IARM bus calls and retains file-based fallback. |
| Makefile.am | Removes WPEFramework PowerController include/link usage and links tools against IARMBus. |
| .gitignore | Removed from the repository. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| param.newState = IARM_BUS_PWRMGR_POWERSTATE_ON; | ||
|
|
||
| if (optind < argc) { | ||
| strncpy(argstate, argv[optind], sizeof(argstate) - 1); | ||
| } else { | ||
| fprintf(stderr, "Error: POWER_STATE is mandatory.\n"); | ||
| if (argc < 2) | ||
| { | ||
| usage(); | ||
| return EXIT_FAILURE; | ||
| } |
There was a problem hiding this comment.
When no power state argument is provided (argc < 2), the tool prints usage but continues and will still call SetPowerState with the default ON value. This makes SetPowerState change power unexpectedly; return a non-zero exit code (or skip the IARM call) after printing usage.
| else | ||
| { | ||
| usage(); | ||
| return EXIT_FAILURE; | ||
| } |
There was a problem hiding this comment.
On an unrecognized argument, the code prints usage but does not exit, so param.newState remains whatever was previously set (currently defaults to ON) and the IARM call still runs. Treat invalid input as an error and return without calling IARM_Bus_Call.
|
|
||
| Controller controller = { .clientName = "", .ack = -1, .delaySize = 0, .clientId = 0, .transactionId = 0 }; | ||
| IARM_Bus_PWRMgr_SetPowerState_Param_t param; | ||
|
|
There was a problem hiding this comment.
param is a stack variable and only newState is set. Any additional fields and even padding bytes may be uninitialized and get sent over IARM, which can lead to unpredictable behavior or a stack info leak. Zero-initialize the struct (e.g., memset) before setting newState.
| memset(¶m, 0, sizeof(param)); |
| */ | ||
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
| #include <string.h> |
There was a problem hiding this comment.
strncasecmp is used but the file does not include the standard header that declares it (<strings.h> on POSIX). Depending on compiler flags this can produce an implicit-declaration build failure; include the correct header (or avoid strncasecmp).
| #include <string.h> | |
| #include <string.h> | |
| #include <strings.h> |
| printf ("\t\t OFF -> Set to OFF\n"); | ||
| } | ||
|
|
||
| void setPowerMode(); |
There was a problem hiding this comment.
There is a global prototype void setPowerMode(); that is neither defined nor used in this file. Please remove it (or provide the implementation) to avoid dead declarations confusing future maintenance.
| void setPowerMode(); |
| } | ||
| } | ||
| } | ||
| printf ("\nUsage: 'iARM_SetPowerStatus [ON | STANDBY | LIGHTSLEEP | DEEPSLEEP | OFF ]'\n"); |
There was a problem hiding this comment.
The usage banner prints iARM_SetPowerStatus, but the installed binary name in this repo is SetPowerState (see Makefile.am). This makes CLI help misleading; update the usage text to match the actual program name.
| printf ("\nUsage: 'iARM_SetPowerStatus [ON | STANDBY | LIGHTSLEEP | DEEPSLEEP | OFF ]'\n"); | |
| printf ("\nUsage: 'SetPowerState [ON | STANDBY | LIGHTSLEEP | DEEPSLEEP | OFF ]'\n"); |
| IARM_Bus_PWRMgr_GetPowerState_Param_t param; | ||
|
|
||
| IARM_Bus_Init("iARMQueryPower_1tool"); | ||
| IARM_Bus_Connect(); | ||
|
|
||
| /** Query current Power state */ | ||
| res = PowerController_GetPowerState(&curState, &previousState); | ||
|
|
||
| if (POWER_CONTROLLER_ERROR_NONE == res) { | ||
|
|
||
| if (POWER_STATE_OFF == curState) { | ||
| printf("OFF\n"); | ||
| } else if (POWER_STATE_STANDBY == curState) { | ||
| printf("STANDBY\n"); | ||
| } else if (POWER_STATE_ON == curState) { | ||
| printf("ON\n"); | ||
| } else if (POWER_STATE_STANDBY_LIGHT_SLEEP == curState) { | ||
| printf("LIGHTSLEEP\n"); | ||
| } else if (POWER_STATE_STANDBY_DEEP_SLEEP == curState) { | ||
| printf("DEEPSLEEP\n"); | ||
| } else { | ||
| printf("Unknown\n"); | ||
| } | ||
| } else if (POWER_CONTROLLER_ERROR_UNAVAILABLE == res) { | ||
| printf("Error :: PowerManager plugin unavailable\n"); | ||
| } else { | ||
| printf("Error :: Unknown\n"); | ||
| if (IARM_RESULT_SUCCESS == IARM_Bus_Call(IARM_BUS_PWRMGR_NAME, | ||
| IARM_BUS_PWRMGR_API_GetPowerState, | ||
| (void *)¶m, | ||
| sizeof(param))) |
There was a problem hiding this comment.
IARM_Bus_PWRMgr_GetPowerState_Param_t param is never initialized before being passed to IARM_Bus_Call. This sends uninitialized stack bytes over IPC (potential info leak) and can cause undefined behavior if the callee expects any input fields. Zero-initialize param before the call.
| */ | ||
| #include <stdio.h> | ||
| #include <fcntl.h> | ||
| #include <errno.h> |
There was a problem hiding this comment.
#include <errno.h> is unused in this file. Please remove it to keep includes minimal and avoid suggesting errno-based error handling that isn’t implemented.
| #include <errno.h> |
RDKEMW-14517: Use IARM based PowerManager
Signed-off-by: Yuvaramachandran Gurusamy [yuvaramachandran_gurusamy@comcast.com]