Rebase by muraliselvaraj2020 · Pull Request #155 · rdkcentral/control

muraliselvaraj2020 · 2025-12-02T16:38:17Z

No description provided.

This reverts commit b47d6f8.

…HREAD (#118)

…C_DISABLE_VIA_PRIVACY (#120)

…CURTAIL (#132)

* RDKEMW-8929: Refactor base ipc class Reason for change: Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * Updating based on Copilot review comments --------- Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

Copilot

Pull request overview

This pull request represents a major "rebase" effort that refactors the codebase to remove numerous compile-time flags and replace them with runtime configuration. The changes focus on voice control features, power management, and device management infrastructure.

Changes:

Removes compile-time flags (CTRLM_LOCAL_MIC, CTRLM_LOCAL_MIC_TAP, BEEP_ON_KWD_ENABLED, NETWORKED_STANDBY_MODE_ENABLED, etc.) and replaces them with runtime checks
Refactors IPC/IARM infrastructure to use a common base class with API revision management
Updates logging to use "AUTOMATION" variants of logging macros throughout the codebase
Adds controller ID range management for RF4CE and BLE networks
Introduces callback mechanism for audio streaming initiation
Updates factory test infrastructure to support dynamic audio analysis plugins

Reviewed changes

Copilot reviewed 65 out of 66 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/voice/ipc/ctrlm_voice_ipc_iarm_thunder.cpp	Refactors voice IPC to use runtime feature detection, adds API revision setting
src/voice/ctrlm_voice_obj.cpp	Replaces compile flags with runtime configuration for local mic and beep features
src/voice/ctrlm_voice_obj.h	Adds new member variables for runtime feature flags and audio start callback support
src/rf4ce/ctrlm_rf4ce_network.cpp	Major refactoring to extract audio streaming logic, add controller ID range management
src/ble/ctrlm_ble_network.cpp	Similar refactoring for BLE audio streaming, adds controller ID migration logic
src/ipc/ctrlm_rcp_ipc_iarm_thunder.cpp	Updates to use new broadcast_iarm_event signatures and API revision management
src/ipc/ctrlm_ipc_iarm.h/cpp	Adds API revision management and refactors event broadcasting
src/factory/.c/.cpp	Updates factory test infrastructure for dynamic plugin loading
CMakeLists.txt	Removes numerous build options and simplifies configuration

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

Copilot · 2026-01-13T16:05:11Z


         if(it->response != CTRLM_THREAD_MONITOR_RESPONSE_ALIVE) {
-            XLOGD_TELEMETRY("Thread %s is unresponsive", it->name);
+            XLOGD_AUTOMATION_TELEMETRY("Thread %s is unresponsive", it->name);


The logging macro XLOGD_AUTOMATION_TELEMETRY is being used but may not be defined. This could cause compilation errors. Verify that this logging macro is properly defined in the logging infrastructure.

Copilot · 2026-01-13T16:05:11Z

+                if (it.second == CTRLM_IARM_CALL_RESULT_SUCCESS) {
+                    return true;
                }
            }
-            return true;
+            return false;


The logic change for result checking appears to be inverted. The original code returned true if all results were SUCCESS and false if any failed. The new code returns true if ANY result is SUCCESS and false if all failed. This could cause incorrect behavior when processing multiple network results.

Copilot · 2026-01-13T16:05:12Z

+                            false, NULL, NULL, NULL, (fd >= 0) ? true : false, true, NULL, NULL,
+                            str_transcription.empty() ? NULL : str_transcription.c_str(), str_audio_file.empty() ? NULL : str_audio_file.c_str(), &request_uuid, request_config.low_latency, request_config.low_cpu_util, fd);


The function ctrlm_voice_ipc_iarm_thunder_t::voice_session_request has a complex function call signature with many parameters. Lines 770-771 add two NULL parameters before the transcription and audio file parameters. This changes the function signature but it's not clear if all call sites have been updated correctly. Verify that the function parameter order matches the declaration in the header file.

Copilot

Pull request overview

Copilot reviewed 143 out of 143 changed files in this pull request and generated 2 comments.

 bool ctrlm_telemetry_event_t<std::string>::event() const {
-    XLOGD_TELEMETRY("telemetry event <%s, %s>", this->marker.c_str(), this->value.c_str());
-    return(t2_event_s((char *)this->marker.c_str(), (char *)this->value.c_str()) == T2ERROR_SUCCESS);
+    if (value.length() > CTRLM_TELEMETRY_MAX_EVENT_SIZE_BYTES) {


+    m_end_reason_rcu       = 0;
+    m_end_reason_session   = 0;
    m_end_reason_server    = 0;
    m_result               = false;
+    m_ret_code_protocol    = 0;


Copilot

Pull request overview

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

+            this->local_config = local_config;
            ret = true;
        } else {
-            XLOGD_ERROR("JSON ERROR: Line <%u> Column <%u> Text <%s>", json_error.line, json_error.column, json_error.text);
+            XLOGD_ERROR("JSON ERROR: Line <%u> Column <%u> Text <%s> Contents <%s>", json_error.line, json_error.column, json_error.text, contents.c_str());
        }


+        json_t *append_root = json_loads(contents.c_str(), JSON_REJECT_DUPLICATES, &json_error);
+        if(append_root == NULL) {
+            XLOGD_ERROR("JSON ERROR: Line <%u> Column <%u> Text <%s> Contents <%s>", json_error.line, json_error.column, json_error.text, contents.c_str());
+        } else {


-#ifdef ANSI_CODES_DISABLED
-#define XLOG_OPTS_DEFAULT (XLOG_OPTS_DATE | XLOG_OPTS_TIME | XLOG_OPTS_LF | XLOG_OPTS_MOD_NAME | XLOG_OPTS_LEVEL)
-#else
 #define XLOG_OPTS_DEFAULT (XLOG_OPTS_DATE | XLOG_OPTS_TIME | XLOG_OPTS_LF | XLOG_OPTS_MOD_NAME | XLOG_OPTS_LEVEL | XLOG_OPTS_COLOR)


+   for (uint8_t i = 0; i < outcome->maxBluezRetries; i++) {
+      if (i >= outcome->bluezRetries || i >= outcome->bluezError.size()) {
+          ss << "\"null\"";
+      } else {


+   ss         << MARKER_RCU_PAIRING_ATTEMPT_VERSION << ",";
+   ss << "\"" << name_get()                         << "\",";
+   ss         << outcome->method                    << ",";
+   ss         << outcome->result                    << ",";
+   ss         << outcome->discovered                << ",";
+   ss << "\"" << outcome->name                      << "\",";


Copilot

Pull request overview

Copilot reviewed 143 out of 143 changed files in this pull request and generated 3 comments.

+#include <cstddef>
 #include "ctrlm_powermanager.h"
-#include "ctrlm_ipc_iarm_powermanager.h"
 #include "ctrlm_thunder_powermanager.h"



                    }
                    ret = true;
-                    XLOGD_DEBUG("EDID data <%d> bytes: \n<%s>", edid_size, ss.str().c_str());
+                    XLOGD_INFO("EDID data <%d> bytes: \n<%s>", edid_size, ss.str().c_str());


-#ifdef ANSI_CODES_DISABLED
-#define XLOG_OPTS_DEFAULT (XLOG_OPTS_DATE | XLOG_OPTS_TIME | XLOG_OPTS_LF | XLOG_OPTS_MOD_NAME | XLOG_OPTS_LEVEL)
-#else
 #define XLOG_OPTS_DEFAULT (XLOG_OPTS_DATE | XLOG_OPTS_TIME | XLOG_OPTS_LF | XLOG_OPTS_MOD_NAME | XLOG_OPTS_LEVEL | XLOG_OPTS_COLOR)


Copilot

Pull request overview

Copilot reviewed 146 out of 146 changed files in this pull request and generated 3 comments.

    bool get_result(void) {
        if (network_id_ != CTRLM_MAIN_NETWORK_ID_ALL) {
            return (result_map_[network_id_] == CTRLM_IARM_CALL_RESULT_SUCCESS);
        } else {


                    }
                    ret = true;
-                    XLOGD_DEBUG("EDID data <%d> bytes: \n<%s>", edid_size, ss.str().c_str());
+                    XLOGD_INFO("EDID data <%d> bytes: \n<%s>", edid_size, ss.str().c_str());


 using namespace std;

-#define OTA_MAX_RETRIES (2)
+#define OTA_MAX_RETRIES (0)


Copilot

Pull request overview

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

        } else {
-            XLOGD_ERROR("JSON ERROR: Line <%u> Column <%u> Text <%s>", json_error.line, json_error.column, json_error.text);
+            XLOGD_ERROR("JSON ERROR: Line <%u> Column <%u> Text <%s> Contents <%s>", json_error.line, json_error.column, json_error.text, contents.c_str());
        }


+        if(append_root == NULL) {
+            XLOGD_ERROR("JSON ERROR: Line <%u> Column <%u> Text <%s> Contents <%s>", json_error.line, json_error.column, json_error.text, contents.c_str());
+        } else {


                    }
                    ret = true;
-                    XLOGD_DEBUG("EDID data <%d> bytes: \n<%s>", edid_size, ss.str().c_str());
+                    XLOGD_INFO("EDID data <%d> bytes: \n<%s>", edid_size, ss.str().c_str());


 #include <stdio.h>

-#ifdef ANSI_CODES_DISABLED
-#define XLOG_OPTS_DEFAULT (XLOG_OPTS_DATE | XLOG_OPTS_TIME | XLOG_OPTS_LF | XLOG_OPTS_MOD_NAME | XLOG_OPTS_LEVEL)
-#else
 #define XLOG_OPTS_DEFAULT (XLOG_OPTS_DATE | XLOG_OPTS_TIME | XLOG_OPTS_LF | XLOG_OPTS_MOD_NAME | XLOG_OPTS_LEVEL | XLOG_OPTS_COLOR)
-#endif



 bool ctrlm_input_event_writer::get_meta_data(struct stat &file_meta_data) {
-    int ret = fstat(fd_, &file_meta_data);
+    std::ostringstream oss;
+    oss << "/sys/devices/virtual/input/" << sysfs_name_;
+    std::string dir_path = oss.str();
+    XLOGD_DEBUG("virtual input path = %s", dir_path.c_str());


Copilot

Pull request overview

Copilot reviewed 147 out of 147 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (7)

src/ipc/ctrlm_rcp_ipc_event.cpp:158

to_string() currently calls json_dumps(to_json(), ...) without checking for a NULL to_json() result, and it never decref’s the JSON object returned by to_json(). This can crash if to_json() returns NULL and also leaks the JSON object on success. Consider dumping the JSON into a string, json_decref()-ing the object, and returning the dumped buffer (caller still must free() it).
src/ipc/ctrlm_rcp_ipc_event.cpp:193
Same issue as net status: to_string() can crash if to_json() returns NULL and it leaks the JSON object returned by to_json() because it’s never json_decref()’d.
src/ipc/ctrlm_rcp_ipc_event.cpp:240
Same issue as net status: to_string() can crash if to_json() returns NULL and it leaks the JSON object returned by to_json() because it’s never json_decref()’d.
src/server/ctrlms_main.c:50
The SIGTERM handler calls ctrlms_ws_term() directly. Signal handlers must only call async-signal-safe functions; calling into websocket teardown (and likely malloc/logging) is undefined and can deadlock/crash. Prefer setting a volatile sig_atomic_t flag in the handler and perform shutdown from the main thread/event loop.
src/ipc/ctrlm_rcp_ipc_event.h:115
to_string() returns a raw char* (allocated by Jansson). The caller must free() it, and it may be NULL on error. Adding an explicit contract comment here would help prevent leaks/misuse at call sites.
src/ipc/ctrlm_rcp_ipc_event.h:147
to_string() returns a raw char* (allocated by Jansson). The caller must free() it, and it may be NULL on error. Adding an explicit contract comment here would help prevent leaks/misuse at call sites.
src/ipc/ctrlm_rcp_ipc_event.h:172
to_string() returns a raw char* (allocated by Jansson). The caller must free() it, and it may be NULL on error. Adding an explicit contract comment here would help prevent leaks/misuse at call sites.

   XLOGD_INFO("Broadcasting IARM message %s RCU %<%s> Upgrade Status....", rcu.ieee_address_get().to_string().c_str(), name_get());
   XLOGD_DEBUG("%s", msg.to_string());

   if (!rcp_ipc->on_firmware_update_progress(msg)) {
       XLOGD_ERROR("Error broadcasting IARM message");
   }


Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

egalla204 and others added 13 commits November 5, 2025 18:00

RDKEMW-10164: update CHANGELOG for release v1.1.5 (#139)

6342f27

RDKEMW-10311 : RF4CE update key mapping (#140)

4705590

Revert "RDKEMW-8929 (#129)" (#145)

07c0c95

This reverts commit b47d6f8.

RDKEMW-7905 : remove ctrlm build flags - ANSI_CODES_DISABLED (#110)

e652641

RDKEMW-8295 : remove ctrlm build flags - MEMORY_LOCK (#111)

fc5045c

RDKEMW-8296 : remove ctrlm build flags - DEEPSLEEP_CLOSE_DB (#112)

284efc8

RDKEMW-8668 : modify ctrlm build flag - BREAKPAD (#119)

d327435

RDKEMW-8297 : remove ctrlm build flags - A5000_ENABLE (#117)

cd566c4

RDKEMW-8664 : remove ctrlm build flags - MEM_DEBUG, ASSERT_ON_WRONG_T…

97ea739

…HREAD (#118)

RDKEMW-8676 : remove ctrlm build flags - MIC_TAP, LOCAL_MIC, LOCAL_MI…

84e8ce8

…C_DISABLE_VIA_PRIVACY (#120)

RDKEMW-9124 : remove xr-voice-sdk build flags - XRAUDIO_CURTAIL XLOG_…

bcc3be8

…CURTAIL (#132)

RDKEMW-10480 : ctrlm release v1.1.6, xr-voice-sdk v1.0.7 (#149)

a5109c8

RDKEMW-9600: FIRST_PACKET_TIMEOUTs (#135)

e1fadf0

muraliselvaraj2020 requested a review from a team as a code owner December 2, 2025 16:38

klu339 and others added 9 commits December 2, 2025 12:09

RDKEMW-10425: Automation Logging (#144)

baebbf2

RDKEMW-11159: new_certselector_type (#157)

d72398a

RDKEMW-11249: update CHANGELOG for ctrlm release v1.1.7 (#158)

4f688c6

RDKEMW-8930 : remove ctrlm build flags - FACTORY_AUDIO_PLAYBACK (#143)

2f1050b

RDKEMW-11283: add product description docs (#159)

f7ef989

RDKEMW-8929: Refactor base ipc class (#162)

3ab9e92

* RDKEMW-8929: Refactor base ipc class Reason for change: Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com> * Updating based on Copilot review comments --------- Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>

RDKEMW-12151 : ctrlm release v1.1.8, xr-voice-sdk v1.0.9 (#166)

d66eb1d

RDKEMW-11471: Remove netType param (#164)

e502de1

RDKEMW-10631: Filter out unpaired devices when reconnecting all devices

fc1c94a

Copilot AI review requested due to automatic review settings January 13, 2026 16:01

Copilot started reviewing on behalf of msmith113 January 13, 2026 16:02 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

klu339 and others added 2 commits January 13, 2026 14:43

RDKEMW-12512: ctrlm release v1.1.9 (#169)

084f519

RDKEMW-9843: report loaded and unloaded voltage from RCU (#163)

932b09e

Copilot AI review requested due to automatic review settings January 21, 2026 14:51

Copilot started reviewing on behalf of msmith113 January 21, 2026 14:51 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

RDKEMW-17936 : remove ctrlm build flags - USE_IARM_POWER_MANAGER (#251)

51f7a1e

Copilot AI review requested due to automatic review settings July 13, 2026 19:08

Copilot started reviewing on behalf of dwolaver July 13, 2026 19:08 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

RDKEMW-20544: Add timestamp to T2 accum markers (#261)

c650ac2

Copilot AI review requested due to automatic review settings July 13, 2026 19:21

Copilot started reviewing on behalf of klu339 July 13, 2026 19:21 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

RDKEMW-19785 : control manager - remove deprecated RFCs (#243)

4fa7429

Copilot AI review requested due to automatic review settings July 15, 2026 00:49

Copilot started reviewing on behalf of dwolaver July 15, 2026 00:50 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

RDKEMW-4056 : RFC overrides for BLE network config (#52)

1fdb0b1

Copilot AI review requested due to automatic review settings July 16, 2026 17:07

Copilot started reviewing on behalf of dwolaver July 16, 2026 17:08 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

RDKEMW-21599 : Control manager crashed on executing tr181 command (#262)

8e1c5e7

Copilot AI review requested due to automatic review settings July 20, 2026 16:32

Copilot started reviewing on behalf of dwolaver July 20, 2026 16:33 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

RDKEMW-20831 : remove ctrlm unused IARM calls/events (#258)

e0a75fd

Copilot AI review requested due to automatic review settings July 22, 2026 21:27

Copilot AI reviewed Jul 22, 2026

RDKEMW-21962 : ctrlm-main - Use of potentially dangerous function (#263)

5906429

Copilot AI review requested due to automatic review settings July 23, 2026 19:16

Copilot AI reviewed Jul 23, 2026

RDKEMW-17937 : remove ctrlm build flags - IP_ENABLED (#253)

c9bc6c0

Copilot AI review requested due to automatic review settings July 23, 2026 19:39

Copilot AI reviewed Jul 23, 2026

		false, NULL, NULL, NULL, (fd >= 0) ? true : false, true, NULL, NULL,
		str_transcription.empty() ? NULL : str_transcription.c_str(), str_audio_file.empty() ? NULL : str_audio_file.c_str(), &request_uuid, request_config.low_latency, request_config.low_cpu_util, fd);

Uh oh!

Conversation

muraliselvaraj2020 commented Dec 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants