RDKEMW-20784: Support ctrlmain to use thunder plugin for libds methods and events - #259
RDKEMW-20784: Support ctrlmain to use thunder plugin for libds methods and events#259balav08 wants to merge 41 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates ControlMgr to use Thunder plugin APIs (instead of direct libds/DSMgr calls) for audio ducking and front panel LED control, and extends the Thunder plugin wrapper utilities to better support these calls.
Changes:
- Added a new FrontPanel Thunder plugin wrapper to control the power LED.
- Added
DisplaySettings.setAudioDuckingwrapper and migrated audio mute/ducking callers to use it. - Added a retry-capable
call_plugin()overload and updated build files to compile/link the new Thunder sources (including for factory builds).
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/voice/ctrlm_voice_obj.cpp | Minor whitespace-only change in voice session request code. |
| src/thunder/plugins/ctrlm_thunder_plugin_front_panel.h | New FrontPanel wrapper interface for power LED operations. |
| src/thunder/plugins/ctrlm_thunder_plugin_front_panel.cpp | New FrontPanel wrapper implementation (setLED / powerLedOn/Off / setBrightness). |
| src/thunder/plugins/ctrlm_thunder_plugin_display_settings.h | Added set_audio_ducking() API using dsAudio ducking enums. |
| src/thunder/plugins/ctrlm_thunder_plugin_display_settings.cpp | Implemented Thunder call to setAudioDucking and checks response. |
| src/factory/thunder/ctrlmf_thunder_plugin.h | Added retry-capable call_plugin() overload declaration. |
| src/factory/thunder/ctrlmf_thunder_plugin.cpp | Implemented retry-on-timeout logic for Thunder JSON-RPC calls. |
| src/factory/ctrlmf_audio_control.cpp | Migrated mute/attenuation audio ducking to Thunder DisplaySettings plugin. |
| src/factory/CMakeLists.txt | Removed ds link from ctrlm-fta; added Thunder sources and WPEFramework link libs for factory. |
| src/ctrlm_utils.cpp | Migrated DSMgr helper functions (audio ducking + LED) to Thunder plugin wrappers. |
| src/CMakeLists.txt | Added new FrontPanel plugin wrapper source to the main build. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
src/factory/ctrlmf_audio_control.cpp:127
- Same issue as above:
CTRLM_USE_THUNDER_FR_DSis undefined, so the Thunder attenuate path is dead code. UseCTRLMF_THUNDERfor consistency with the rest of the factory Thunder code.
dwolaver
left a comment
There was a problem hiding this comment.
Kindly investigate and resolve all copilot comments.
The build must complete successfully when THUNDER is disabled.
Re-submit for code review after all the changes are completed. Thanks!
| if(vol < 0 || vol > 1) { | ||
| XLOGD_INFO("[CTRLM_DUCK_AUDIO] Function called: enable=%d, relative=%d, vol=%f", enable, relative, vol); | ||
| if(vol < 0 || vol > 1) { | ||
| XLOGD_ERROR("[CTRLM_DUCK_AUDIO] Invalid volume %f (must be 0.0-1.0)", vol); |
There was a problem hiding this comment.
Remove this line. This doesn't add any value.
|
|
||
| bool ctrlm_dsmgr_duck_audio(bool enable, bool relative, double vol) { | ||
| if(vol < 0 || vol > 1) { | ||
| XLOGD_INFO("[CTRLM_DUCK_AUDIO] Function called: enable=%d, relative=%d, vol=%f", enable, relative, vol); |
There was a problem hiding this comment.
Please remove all the extra log lines that were added in this function. They look like unnecessary debug logs that don't belong in the final implementation.
| return ret; | ||
| } | ||
|
|
||
| bool ctrlm_dsmgr_duck_audio(bool enable, bool relative, double vol) { |
There was a problem hiding this comment.
I'm pretty sure this function will not compile when thunder is disabled. The build must complete successfully when thunder is disabled.
|
|
||
| bool ctrlm_thunder_plugin_display_settings_t::set_audio_ducking( | ||
| bool action, bool type, unsigned char level) { | ||
| XLOGD_INFO("[THUNDER_DS_DUCKING] Function called: action=%d (%s), type=%d (%s), level=%u", action, action ? "start" : "stop", type, type ? "relative" : "absolute", level); |
There was a problem hiding this comment.
This part adds no value "[THUNDER_DS_DUCKING] Function called: ". Please remove it.
Each log line includes the function name automatically.
A function should only print one log line (at most) under normal circumstances. It can print two lines when there is an error/warning. The rest of the logs need to be removed or set to DEBUG level.
Reason for change: Support ctrlmain to use thunder plugin for libds methods.
Test procedure: refer ticket
Risks: Medium
Priority: P2
version: Patch