Skip to content

Conversation

@yuvaramachandran-gurusamy
Copy link
Contributor

Revert "Merge pull request #47 from rdkcentral/feature/RDK-55702-use-pwr-plugin_retry"

This reverts commit 09d4d8c, reversing changes made to 6f7faef.

…pwr-plugin_retry"

This reverts commit 09d4d8c, reversing
changes made to 6f7faef.
Copilot AI review requested due to automatic review settings February 2, 2026 15:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reverts the earlier move to the Thunder PowerController client by switching TR-69 HostIF power interactions back to the IARM Power Manager (PWRMgr) API and removing the related Thunder stub/linking.

Changes:

  • Replace PowerStatus retrieval and power-mode change notifications from Thunder PowerController to IARM PWRMgr.
  • Remove PowerController unit-test stubs and drop the -lWPEFrameworkPowerController linkage/build steps.
  • Simplify/cleanup DeviceInfo and IARM handler code paths affected by the revert.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/unittest/stubs/powerctrl_stubs.cpp Deletes the Thunder PowerController stub implementation.
src/unittest/stubs/power_controller.h Deletes the Thunder PowerController stub header/API surface.
src/unittest/stubs/dm_stubs.cpp Removes embedded PowerController stub functions and related include.
src/hostif/profiles/DeviceInfo/gtest/Makefile.am Minor whitespace cleanup.
src/hostif/profiles/DeviceInfo/Device_DeviceInfo.h Removes PowerController-enable interface hook.
src/hostif/profiles/DeviceInfo/Device_DeviceInfo.cpp Uses IARM PWRMgr GetPowerState for X_COMCAST_COM_PowerStatus.
src/hostif/handlers/src/hostIf_IARM_ReqHandler.cpp Registers PWRMgr mode-change event handler instead of PowerController callback/thread.
src/Makefile.am Removes -lWPEFrameworkPowerController from link flags.
cov_build.sh Removes building/linking the PowerController stub library during coverage build.
Comments suppressed due to low confidence (1)

cov_build.sh:77

  • cov_build.sh still builds mock-parodus (via mock_parodus_build.sh), but src/hostif/parodusClient/pal/mock-parodus/Makefile.am currently references the deleted src/unittest/stubs/powerctrl_stubs.cpp. This will break CI/coverage builds; remove that source from the mock-parodus build (or provide an alternative stub) to match the deletion in this PR.

cd ./src/hostif/parodusClient/pal/mock-parodus/
sh mock_parodus_build.sh

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

Comment on lines 161 to +163
IARM_Bus_RegisterEvent(IARM_BUS_TR69HOSTIFMGR_EVENT_MAX);
IARM_Bus_RegisterEventHandler(IARM_BUS_PWRMGR_NAME,IARM_BUS_PWRMGR_EVENT_MODECHANGED, _hostIf_EventHandler);

Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IARM_Bus_RegisterEventHandler(...) return value is ignored. If registration fails, deep-sleep notifications will silently stop working while TR69_HostIf_Mgr_Get_RegisterCall() still reports success. Check the return code, log on failure, and incorporate it into ret.

Suggested change
IARM_Bus_RegisterEvent(IARM_BUS_TR69HOSTIFMGR_EVENT_MAX);
IARM_Bus_RegisterEventHandler(IARM_BUS_PWRMGR_NAME,IARM_BUS_PWRMGR_EVENT_MODECHANGED, _hostIf_EventHandler);
IARM_Result_t iarmRet;
iarmRet = IARM_Bus_RegisterEvent(IARM_BUS_TR69HOSTIFMGR_EVENT_MAX);
if (iarmRet != IARM_RESULT_SUCCESS)
{
RDK_LOG(RDK_LOG_ERROR, LOG_TR69HOSTIF,
"[%s:%s] IARM_Bus_RegisterEvent failed with result=%d\n",
__FUNCTION__, __FILE__, iarmRet);
ret = false;
}
iarmRet = IARM_Bus_RegisterEventHandler(IARM_BUS_PWRMGR_NAME,
IARM_BUS_PWRMGR_EVENT_MODECHANGED,
_hostIf_EventHandler);
if (iarmRet != IARM_RESULT_SUCCESS)
{
RDK_LOG(RDK_LOG_ERROR, LOG_TR69HOSTIF,
"[%s:%s] IARM_Bus_RegisterEventHandler failed with result=%d\n",
__FUNCTION__, __FILE__, iarmRet);
ret = false;
}

Copilot uses AI. Check for mistakes.
Comment on lines 272 to 276
{
RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"[%s] Failed to IARM_Bus_Term(), return with Error code: %d\n", __FUNCTION__, err);
}

if (IsPwrCtlInt)
{
RDK_LOG(RDK_LOG_DEBUG,LOG_TR69HOSTIF,"[%s:%s] Registering power mode change callback..\n", __FUNCTION__, __FILE__);
PowerController_UnRegisterPowerModeChangedCallback(_hostIf_EventHandler);
RDK_LOG(RDK_LOG_INFO,LOG_TR69HOSTIF,"[%s:%s] Registered power mode change callback..\n", __FUNCTION__, __FILE__);

RDK_LOG(RDK_LOG_DEBUG, LOG_TR69HOSTIF, "[%s:%d]: start PowerController_Term().. \n", __FUNCTION__, __LINE__);
PowerController_Term();
RDK_LOG(RDK_LOG_DEBUG, LOG_TR69HOSTIF, "[%s:%d]: completed PowerController_Term().. \n", __FUNCTION__, __LINE__);
hostIf_DeviceInfo::getInstance(0)->setPowerConInterface(false);
IsPwrCtlInt = false;
RDK_LOG(RDK_LOG_DEBUG, LOG_TR69HOSTIF, "[%s:%d]: setPowerConInterface flag to false. and IsPwrCtlInt=%s\n", __FUNCTION__, __LINE__, (IsPwrCtlInt?"true":"false"));

}
else
{
RDK_LOG(RDK_LOG_ERROR, LOG_TR69HOSTIF, "[%s:%d]: No PowerController interface .. IsPwrCtlInt = %d\n", __FUNCTION__, __LINE__, IsPwrCtlInt);
}
RDK_LOG(RDK_LOG_TRACE1,LOG_TR69HOSTIF,"[%s:%s] Exiting..\n", __FUNCTION__, __FILE__);
return err;
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PWRMgr event handler registered in TR69_HostIf_Mgr_Get_RegisterCall() is never unregistered on shutdown. Before terminating IARM, remove/unregister the handler (e.g., IARM_Bus_RemoveEventHandler(...)/IARM_Bus_UnRegisterEventHandler(...)) to avoid callbacks into torn-down code and keep lifecycle symmetric.

Copilot uses AI. Check for mistakes.
Comment on lines +1551 to +1553
strncpy((char *)stMsgData->paramValue, pwrState, str_len);
stMsgData->paramValue[str_len+1] = '\0';
stMsgData->paramLen = str_len;
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strncpy is copying str_len bytes but the code null-terminates at paramValue[str_len+1], which leaves paramValue[str_len] undefined and can write past the intended end. Null-terminate at str_len (or use a bounded API like snprintf/strncpy_s) and ensure the write stays within TR69HOSTIFMGR_MAX_PARAM_LEN.

Copilot uses AI. Check for mistakes.
else
{
RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Powercontroller Interface failed : %d. Try after sometime. \n", bPowerControllerEnable);
RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Failed in IARM_Bus_Call() for parameter : %s [param.type:%s with error code:%d]\n",stMsgData->paramName, pwrState, ret);
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failure log prints ret (still NOK) as the error code, and the format label param.type:%s is misleading (it prints pwrState). Log the actual IARM_Result_t err (and/or method/owner) so failures are diagnosable.

Suggested change
RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Failed in IARM_Bus_Call() for parameter : %s [param.type:%s with error code:%d]\n",stMsgData->paramName, pwrState, ret);
RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Failed in IARM_Bus_Call() for parameter: %s [IARM_Result:%d]\n", stMsgData->paramName, err);

Copilot uses AI. Check for mistakes.
Comment on lines +1539 to 1546
err = IARM_Bus_Call(IARM_BUS_PWRMGR_NAME,
IARM_BUS_PWRMGR_API_GetPowerState,
(void *)&param,
sizeof(param));
if(err == IARM_RESULT_SUCCESS)
{
pwrState = (param.curState==IARM_BUS_PWRMGR_POWERSTATE_OFF)?"PowerOFF":(param.curState==IARM_BUS_PWRMGR_POWERSTATE_ON)?"PowerON":"Standby";

Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new PowerStatus implementation depends on the IARM_BUS_PWRMGR_API_GetPowerState response/mapping, but there is no unit test covering the returned strings for OFF/ON/STANDBY (or IARM call failure). Adding a gtest for get_Device_DeviceInfo_X_COMCAST_COM_PowerStatus would help prevent regressions in the state mapping.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant