RDKEMW-8133: Optional param name for voiceSessionRequest #108

klu339 · 2025-09-11T18:34:30Z

Reason for change: Adding optional name param for voiceSessionRequest
which is needed to track metadata about voice
sessions from various ipcontrol clients
Test Procedure: Use VoiceControl voiceSessionRequest method with name
param
Risks: Low

dwolaver · 2025-09-12T13:29:55Z

src/voice/ipc/ctrlm_voice_ipc_iarm_thunder.cpp

                                   }
                                }
                            }
+                            if (request_config.requires_name_of_source) {


This isn't a required parameter though. How about use the name if provided otherwise use "APPLICATION"?

dwolaver · 2025-09-12T13:31:02Z

src/voice/ipc/ctrlm_voice_ipc_request_type.h

 typedef struct {
   bool                 requires_transcription;
   bool                 requires_audio_file;
+   bool                 requires_name_of_source;


delete requires_name_of_source member

dwolaver · 2025-09-17T17:40:00Z

src/voice/ipc/ctrlm_voice_ipc_iarm_thunder.cpp

                            }
+                            json_t *obj_name_of_source = json_object_get(obj, "name");
+                            if(obj_name_of_source == NULL || !json_is_string(obj_name_of_source)) {
+                                XLOGD_WARN("invalid name parameter, but this is optional");


if the "name" parameter is not present, nothing should be printed about it.
if it's present and not a string, then yes print a warning.

check for if(obj_name_of_source != NULL) around the XLOGD_WARN

dwolaver · 2025-09-17T17:42:25Z

src/voice/ipc/ctrlm_voice_ipc_iarm_thunder.cpp

+                                XLOGD_WARN("invalid name parameter, but this is optional");
+                            } else {
+                                str_name_of_source = std::string(json_string_value(obj_name_of_source));
+                                if(str_name_of_source.empty()) {


I don't see a need to print a warning if the user specifies an empty string.

Reason for change: Adding optional name param for voiceSessionRequest which is needed to track metadata about voice sessions from various ipcontrol clients Test Procedure: Use VoiceControl voiceSessionRequest method with name param Risks: Low Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

Reason for change: remove the required conditional of name Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

Reason for change: Clean up log messaging Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

dwolaver · 2025-09-17T20:05:26Z

src/voice/ipc/ctrlm_voice_ipc_iarm_thunder.cpp

                            }
+                            json_t *obj_name_of_source = json_object_get(obj, "name");
+                            if(obj_name_of_source == NULL || !json_is_string(obj_name_of_source)) {
+                                XLOGD_WARN("invalid name parameter, but this is optional");


check for if(obj_name_of_source != NULL) around the XLOGD_WARN

Reason for change: Move obj != NULL block to prevent null dereference Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

dwolaver

approved for QA test

Has already been addressed and was approved by Dave in an earlier comment

* Deploy cla action * RDKEMW-7174 : update AMC APP key mapping (#98) * RDKEMW-3409 : move certselector logic into ctrlm-main repo (#102) * RDKEMW-6767: getNetStatus call time out due to SAT download retries (#97) Reason for change: During initial setup, the device is attempting to download the SAT token and timing out, which is holding up the ctrlm message queue processing, which allows BLE pairing to fail Test Procedure: boot and attempt to pair BLE remote, keeping an eye on ctrlm loggging. Look for "CTRLM : ERROR: call_plugin: Thunder call failed <getServiceAccessToken> <11>, attempt 1 of 1" We expect BLE pairing to succeed rather than timeout. If the call_plugin log error occurs and BLE pairing succeed then the code change has been exercised and the issue is resolved. Risks: Low Priority: P1 Signed-off-by: Jason Thomson <jason_thomson@comcast.com> * RDKEMW-7333: remove device from bluez during factory reset (#100) When an RDK device is factory reset, controlMgr will send a message to the remote to also factory reset itself. Once controlMgr gets notified from the remote of a successful RCU factory reset, it needs to be requested to bluez to remove the device. This prevents a connection attempt from happening to the just factory-reset RCU before the RDK device reboots. This connection attempt will prevent the RCU from autopairing during the activation flow after the RDK factory reset. * RDKEMW-7573 : remove ctrlm compile flags (#104) * RDKEMW-7694 : remove ctrlm build flags - CPC, DUNFELL (#105) * RDKEMW-7834 : remove ctrlm build flags - RF4CE_PACKET_ANALYSIS (#107) * RDKEMW-7772 : remove ctrlm build flags - DISABLE_BLE_VOICE (#106) * RDKEMW-7122 : Missing Thunder cflags in ctrlm implemenation (#103) * RDKEMW-7979 : use version/branch from recipe (#109) * RDKEMW-8349 : ctrlm release v1.1.4 (#113) * RDKEMW-8133: Optional param name for voiceSessionRequest (#108) * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: Adding optional name param for voiceSessionRequest which is needed to track metadata about voice sessions from various ipcontrol clients Test Procedure: Use VoiceControl voiceSessionRequest method with name param Risks: Low Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: remove the required conditional of name Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: Clean up log messaging Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: Move obj != NULL block to prevent null dereference Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> --------- Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8354: ctrlm-main crash while holding standby during OTA (#115) * RDKEMW-8354: ctrlm-main crash while holding standby during OTA Reason for change: crash due to null reference on repeating timer event Test Procedure: see ticket Risks: low Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8354: ctrlm-main crash on timer Reason for change: add comment for clarity Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> --------- Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * Deploy cla action * Deploy fossid_integration_stateless_diffscan_target_repo action --------- Signed-off-by: Jason Thomson <jason_thomson@comcast.com> Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> Co-authored-by: rdkcmf <github@code.rdkcentral.com> Co-authored-by: dwolaver <44593664+dwolaver@users.noreply.github.com> Co-authored-by: jthomp007c <jason_thomson@cable.comcast.com> Co-authored-by: Kelvin Lu <119349872+klu339@users.noreply.github.com> Co-authored-by: Alan Ryan <20208488+Alan-Ryan@users.noreply.github.com>

* Deploy cla action * RDKEMW-7174 : update AMC APP key mapping (#98) * RDKEMW-3409 : move certselector logic into ctrlm-main repo (#102) * RDKEMW-6767: getNetStatus call time out due to SAT download retries (#97) Reason for change: During initial setup, the device is attempting to download the SAT token and timing out, which is holding up the ctrlm message queue processing, which allows BLE pairing to fail Test Procedure: boot and attempt to pair BLE remote, keeping an eye on ctrlm loggging. Look for "CTRLM : ERROR: call_plugin: Thunder call failed <getServiceAccessToken> <11>, attempt 1 of 1" We expect BLE pairing to succeed rather than timeout. If the call_plugin log error occurs and BLE pairing succeed then the code change has been exercised and the issue is resolved. Risks: Low Priority: P1 Signed-off-by: Jason Thomson <jason_thomson@comcast.com> * RDKEMW-7333: remove device from bluez during factory reset (#100) When an RDK device is factory reset, controlMgr will send a message to the remote to also factory reset itself. Once controlMgr gets notified from the remote of a successful RCU factory reset, it needs to be requested to bluez to remove the device. This prevents a connection attempt from happening to the just factory-reset RCU before the RDK device reboots. This connection attempt will prevent the RCU from autopairing during the activation flow after the RDK factory reset. * RDKEMW-7573 : remove ctrlm compile flags (#104) * RDKEMW-7694 : remove ctrlm build flags - CPC, DUNFELL (#105) * RDKEMW-7834 : remove ctrlm build flags - RF4CE_PACKET_ANALYSIS (#107) * RDKEMW-7772 : remove ctrlm build flags - DISABLE_BLE_VOICE (#106) * RDKEMW-7122 : Missing Thunder cflags in ctrlm implemenation (#103) * RDKEMW-7979 : use version/branch from recipe (#109) * RDKEMW-8349 : ctrlm release v1.1.4 (#113) * RDKEMW-8133: Optional param name for voiceSessionRequest (#108) * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: Adding optional name param for voiceSessionRequest which is needed to track metadata about voice sessions from various ipcontrol clients Test Procedure: Use VoiceControl voiceSessionRequest method with name param Risks: Low Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: remove the required conditional of name Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: Clean up log messaging Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8133: Optional param name for voiceSessionRequest Reason for change: Move obj != NULL block to prevent null dereference Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> --------- Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8354: ctrlm-main crash while holding standby during OTA (#115) * RDKEMW-8354: ctrlm-main crash while holding standby during OTA Reason for change: crash due to null reference on repeating timer event Test Procedure: see ticket Risks: low Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-8354: ctrlm-main crash on timer Reason for change: add comment for clarity Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> --------- Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * Deploy cla action * Deploy fossid_integration_stateless_diffscan_target_repo action * RDKEMW-8815: only return SUCCESS for autolookup if it found at least 1 code. * Revert "RDKEMW-8815: only return SUCCESS for autolookup if it found at least 1 code." (#124) * RDKEMW-8815: only return SUCCESS for autolookup if it found at least 1 code. (#125) It can often happen that the IRDB returns success but no codes are returned, this leads to confusing UI screens. So even if the IR database returned successfully, only return success to the plugin API if there is at least 1 code is present * Update CODEOWNERS (#130) * RDKEMW-8929 (#129) * RDKEMW-8929: Refactor ctrlm_voice_ipc_t to inherit ctrlm_ipc_iarm_t Reason for change: Inherit ctrlm_ipc_iarm_t Test Procedure: Verify behavior of events before & after no diff Risks: Low Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * RDKEMW-7225: BLE pairing retries (#126) if BLE pairing fails, retry 3 times or up to the pairing timeout (currently configured at 20 seconds), whichever comes first. --------- Signed-off-by: Jason Thomson <jason_thomson@comcast.com> Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> Co-authored-by: rdkcmf <github@code.rdkcentral.com> Co-authored-by: dwolaver <44593664+dwolaver@users.noreply.github.com> Co-authored-by: jthomp007c <jason_thomson@cable.comcast.com> Co-authored-by: Kelvin Lu <119349872+klu339@users.noreply.github.com> Co-authored-by: Alan Ryan <20208488+Alan-Ryan@users.noreply.github.com> Co-authored-by: Stephen Barrett <sbarre01@users.noreply.github.com>

klu339 requested review from a team September 11, 2025 18:34

dwolaver requested changes Sep 12, 2025

View reviewed changes

klu339 force-pushed the feature/RDKEMW-8133 branch 2 times, most recently from 7e485ca to 548bbd3 Compare September 17, 2025 14:49

dwolaver requested changes Sep 17, 2025

View reviewed changes

klu339 added 3 commits September 17, 2025 17:48

RDKEMW-8133: Optional param name for voiceSessionRequest

57d8b78

Reason for change: remove the required conditional of name Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

RDKEMW-8133: Optional param name for voiceSessionRequest

23bee2c

Reason for change: Clean up log messaging Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

klu339 force-pushed the feature/RDKEMW-8133 branch from 548bbd3 to 23bee2c Compare September 17, 2025 17:51

dwolaver previously requested changes Sep 17, 2025

View reviewed changes

RDKEMW-8133: Optional param name for voiceSessionRequest

dcfefdf

Reason for change: Move obj != NULL block to prevent null dereference Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

dwolaver reviewed Sep 17, 2025

View reviewed changes

egalla204 approved these changes Sep 22, 2025

View reviewed changes

klu339 merged commit 507a5a4 into develop Sep 22, 2025
8 checks passed

klu339 deleted the feature/RDKEMW-8133 branch September 22, 2025 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RDKEMW-8133: Optional param name for voiceSessionRequest #108

RDKEMW-8133: Optional param name for voiceSessionRequest #108

Uh oh!

klu339 commented Sep 11, 2025

Uh oh!

dwolaver Sep 12, 2025

Uh oh!

dwolaver Sep 12, 2025

Uh oh!

dwolaver Sep 17, 2025

Uh oh!

dwolaver Sep 17, 2025

Uh oh!

dwolaver Sep 17, 2025

Uh oh!

dwolaver Sep 17, 2025

Uh oh!

dwolaver left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RDKEMW-8133: Optional param name for voiceSessionRequest #108

RDKEMW-8133: Optional param name for voiceSessionRequest #108

Uh oh!

Conversation

klu339 commented Sep 11, 2025

Uh oh!

dwolaver Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

dwolaver Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

dwolaver Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

dwolaver Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

dwolaver Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

dwolaver Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

dwolaver left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants