RDKEMW-11792 : FFV config file isolation #165

dwolaver · 2026-01-02T17:57:49Z

No description provided.

Copilot

Pull request overview

This PR removes OEM-specific configuration file integration from the build process as part of FFV (likely "Firmware Version") config file isolation. The change removes two parameters from the JSON configuration merge command that previously incorporated OEM customizations.

Removes OEM configuration substitution and addition parameters from the build-time JSON combine step

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

…ation

Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

src/voice/ctrlm_voice_obj.h:747

The function declaration for vsdk_keyword_sensitivity_limit_check remains in the header file but its implementation has been removed from ctrlm_voice_obj.cpp. This will cause a linker error if any code attempts to call this function. Remove this declaration from the header file.

    double               vsdk_keyword_sensitivity_limit_check(double sensitivity);

src/ctrlm_main.cpp:1631

The root JSON object is retrieved before the OEM configuration is appended (line 1646). While this works because json_from_path returns a reference to the internal root object which is modified in-place by append_config, it creates confusing code flow. Consider moving this call after the OEM config append operation for clarity, or add a comment explaining why this order is intentional.

   *json_obj_root = ctrlm_config->json_from_path("", true); // Get root AND add ref to it, since this code derefs it

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

CMakeLists.txt

…ub-rdkcentral:rdkcentral/control into feature/RDKEMW-11792_ffv_config_file_isolation

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/voice/ctrlm_voice_obj.h:747

The function declaration for vsdk_keyword_sensitivity_limit_check should be removed since its implementation has been deleted. This leftover declaration will cause linking errors if anyone attempts to use it.

    double               vsdk_keyword_sensitivity_limit_check(double sensitivity);

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

src/ctrlm_main.cpp

Copilot

Pull request overview

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

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

src/config/ctrlm_config.cpp

src/ctrlm_main.cpp

Co-authored-by: Copilot <[email protected]>

Copilot · 2026-01-21T23:16:35Z

@dwolaver I've opened a new pull request, #172, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

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

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

Copilot · 2026-01-21T23:22:27Z

src/config/ctrlm_config.cpp

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


The JSON parse error log now includes the full config file contents. This can be very large and may leak sensitive data into logs. Consider truncating the logged contents and/or only including it when verbose is enabled.

Copilot · 2026-01-21T23:22:27Z

src/ctrlm_main.cpp

   }

  //Extract the vsdk configuration object


In the vsdk extraction block immediately below this section, the code sets json_obj_vsdk = NULL when the JSON value is missing or not an object. That only clears the local pointer variable, not the out-param; if the value exists but is the wrong type, callers can still receive a non-object *json_obj_vsdk. Update that branch to clear *json_obj_vsdk instead.

Copilot · 2026-01-21T23:22:27Z

src/ctrlm_main.cpp

+   // Print the configuration since it was loaded from files
+   char *json_dump = json_dumps(*json_obj_root, JSON_INDENT(3) | JSON_SORT_KEYS);
+   if(json_dump == NULL) {
+      XLOGD_ERROR("unable to dump JSON object");
+      json_decref(*json_obj_root);
+      *json_obj_root = NULL;
+      return(false);
+   } else {
+      XLOGD_INFO_OPTS(XLOG_OPTS_DEFAULT, 20 * 1024, "Final configuration:\n%s", json_dump);
+      free(json_dump);
+   }


Dumping the entire merged configuration to logs can expose secrets/PII (e.g., device_id, endpoints, auth URLs) in production. Consider gating this behind a debug/verbose flag, masking sensitive fields before logging, or removing the full dump entirely.

Suggested change

// Print the configuration since it was loaded from files

char *json_dump = json_dumps(*json_obj_root, JSON_INDENT(3) | JSON_SORT_KEYS);

if(json_dump == NULL) {

XLOGD_ERROR("unable to dump JSON object");

json_decref(*json_obj_root);

*json_obj_root = NULL;

return(false);

} else {

XLOGD_INFO_OPTS(XLOG_OPTS_DEFAULT, 20 * 1024, "Final configuration:\n%s", json_dump);

free(json_dump);

}

// Configuration successfully loaded and validated at this point

Copilot · 2026-01-21T23:22:28Z

src/config/ctrlm_config.cpp

+        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 {
+            if(json_object_update(this->root, append_root) != 0) {
+                XLOGD_ERROR("Failed to merge JSON objects");
+            } else {
+                if(verbose) {
+                    XLOGD_INFO("config appended successfully as JSON for <%s>", file_path.c_str());
+                }
+                ret = true;
+            }
+            json_decref(append_root);


append_config() uses json_object_update(), which overwrites top-level keys and replaces nested objects wholesale. If the OPT file is intended to override only a subset of an OEM section (common for config overrides), this will drop the rest of the OEM subtree. Use a recursive merge (e.g., json_object_update_recursive() or equivalent custom deep-merge) to preserve existing nested keys while overriding only those provided.

Copilot · 2026-01-21T23:22:28Z

CMakeLists.txt

 install(TARGETS controlMgr RUNTIME DESTINATION bin)

-install(FILES ${CMAKE_BINARY_DIR}/ctrlm_config.json.template DESTINATION ${CMAKE_INSTALL_SYSCONFDIR} COMPONENT config )
+install(FILES ${CMAKE_BINARY_DIR}/ctrlm_config.json DESTINATION ${CMAKE_INSTALL_SYSCONFDIR} COMPONENT config )


ctrlm_load_config() now loads OEM config from /etc/vendor/input/ctrlm_config.json, but the build installs ctrlm_config.json to ${CMAKE_INSTALL_SYSCONFDIR} (typically /etc). This mismatch means the installed config may never be read at runtime. Align the install destination with the runtime search path (or update the runtime path to match the install).

RDKEMW-11792 : FFV config file isolation

d94e829

dwolaver requested a review from a team as a code owner January 2, 2026 17:57

Copilot AI review requested due to automatic review settings January 2, 2026 17:57

dwolaver marked this pull request as draft January 2, 2026 17:58

Copilot started reviewing on behalf of dwolaver January 2, 2026 17:58 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

dwolaver and others added 6 commits January 2, 2026 14:24

add append config file

0222245

apply oem config append

9453e90

Merge branch 'develop' into feature/RDKEMW-11792_ffv_config_file_isol…

05dd81b

…ation

remove keyword sensitivity from ctrlm

2e08dac

print final config

8596bd5

Merge branch 'develop' into feature/RDKEMW-11792_ffv_config_file_isol…

8f6e420

…ation

dwolaver marked this pull request as ready for review January 15, 2026 16:17

Copilot AI review requested due to automatic review settings January 15, 2026 16:17

Copilot started reviewing on behalf of dwolaver January 15, 2026 16:18 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

dwolaver added 3 commits January 16, 2026 13:36

remove vsdk defaults from config file

ee2927e

Merge branch 'feature/RDKEMW-11792_ffv_config_file_isolation' of gith…

fb60c5d

…ub-rdkcentral:rdkcentral/control into feature/RDKEMW-11792_ffv_config_file_isolation

remove template

2688e3d

Copilot AI review requested due to automatic review settings January 16, 2026 20:59

Copilot started reviewing on behalf of dwolaver January 16, 2026 21:00 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

src/ctrlm_main.cpp Outdated Show resolved Hide resolved

src/ctrlm_main.cpp Outdated Show resolved Hide resolved

dwolaver added 2 commits January 20, 2026 14:34

removing etc processing

55bb010

fix final config print

6b92eb4

Copilot AI review requested due to automatic review settings January 21, 2026 16:31

Copilot started reviewing on behalf of dwolaver January 21, 2026 16:32 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

src/config/ctrlm_config.cpp Outdated Show resolved Hide resolved

src/ctrlm_main.cpp Show resolved Hide resolved

src/ctrlm_main.cpp Show resolved Hide resolved

dwolaver and others added 2 commits January 21, 2026 11:58

removing unused config entries

6a2e3f8

Update src/config/ctrlm_config.cpp

70e9853

Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings January 21, 2026 23:15

Copilot started reviewing on behalf of dwolaver January 21, 2026 23:16 View session

Copilot AI mentioned this pull request Jan 21, 2026

Remove trailing whitespace in src/ctrlm_main.cpp #172

Closed

Copilot AI reviewed Jan 21, 2026

View reviewed changes

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

RDKEMW-11792 : FFV config file isolation #165

Are you sure you want to change the base?

RDKEMW-11792 : FFV config file isolation #165

Conversation

dwolaver commented Jan 2, 2026

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!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants