Skip to content

RDKEMW-19653 : Deprecated DataModel Removal for XRE and XCaliber code from RDKE - #520

Open
madhubabutt wants to merge 2 commits into
developfrom
feature/RDKEMW-19653
Open

RDKEMW-19653 : Deprecated DataModel Removal for XRE and XCaliber code from RDKE#520
madhubabutt wants to merge 2 commits into
developfrom
feature/RDKEMW-19653

Conversation

@madhubabutt

@madhubabutt madhubabutt commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Reason for change: Removal of deprecated XRE/Xcalibur profile infrastructure (USE_XRESRC, XRELIB_FLAG, and all Xcalibur parameters except xconfCheckNow) while preserving minimal xconfCheckNow parameter support.
Test Procedure: Build RDKE with the change, validate Device.X_COMCAST-COM_Xcalibur.Client.xconfCheckNow functionality, perform stability/reboot tests ensuring no crashes.
Risks: Medium
Priority: P1
Signed-off-by: Tirumala, Madhubabu (Contractor) Madhubabu_Tirumala@comcast.com

Copilot AI lite review requested due to automatic review settings August 3, 2026 06:39
@madhubabutt
madhubabutt requested a review from a team as a code owner August 3, 2026 06:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes deprecated XRE/Xcalibur data model and build/config wiring from the tr69hostif codebase, aligning RDKE builds and runtime configuration away from the legacy XRE/Xcalibur profile.

Changes:

  • Removes XRE/Xcalibur-specific build conditionals and linkage (XRELIB_FLAG, --enable-xre, -ltr69ProfileXcaliber).
  • Drops Xcalibur/XRE manager mappings from runtime and integration-test configuration.
  • Prunes XRE/Xcalibur-related WALDB data model objects from the TV and generic data model XMLs.

Reviewed changes

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

Show a summary per file
File Description
src/Makefile.am Removes XRELIB_FLAG-based linking and Xcaliber profile library linkage.
configure.ac Removes --enable-xre option, conditional, and XRE substitution variables.
conf/tr69hostIf.conf Removes XRE/Xcalibur object-to-manager mappings.
conf/mgrlist.conf Removes Xcalibur manager entry from the manager list.
src/integrationtest/conf/mgrlist.conf Mirrors mgrlist removal for integration-test configuration.
src/hostif/parodusClient/waldb/data-model/data-model-tv.xml Removes Xcalibur CoPilot object definitions from the TV WALDB model.
src/hostif/parodusClient/waldb/data-model/data-model-generic.xml Removes XRPairing RFC object and large Xcalibur/XRE/TRM sections from the generic WALDB model.

Comment thread src/hostif/parodusClient/waldb/data-model/data-model-generic.xml
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|74.2%  9594|90.4% 659|    -    0

@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from 3a7de3d to d90824e Compare August 3, 2026 12:50
Copilot AI review requested due to automatic review settings August 3, 2026 12:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (3)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:133

  • The XRE client handler is documented as “xconfCheckNow only”, but handleSetMsg still accepts additional Xcalibur parameters under RDKV_TR69 (xreReceiverRestart/devAppRestartRequest). Since the XRE/Xcalibur data model and manager mappings are being removed, these extra branches are now inconsistent and can leave partially-supported/undocumented behavior. Consider removing the RDKV_TR69-only branches so this handler truly only supports xconfCheckNow.
    #ifdef RDKV_TR69
    else if(strcasecmp(stMsgData->paramName,"Device.X_COMCAST-COM_Xcalibur.Client.XRE.xreReceiverRestart") == 0)
    {
        ret = setXreReceiverRestart(stMsgData);
    }

src/hostif/profiles/DeviceInfo/Makefile.am:27

  • This Makefile now has the same MoCA include path listed twice, which is redundant and makes the include list harder to maintain. Remove the duplicate entry.
-I$(top_srcdir)/src/hostif/profiles/moca \
-I$(top_srcdir)/src/hostif/profiles/moca \

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:214

  • The xconfCheckNow input validation accepts prefix matches because it uses strncasecmp(..., strlen("TRUE")) / strlen("CANARY") (e.g., "TRUE123" passes). If the intent is to only accept the exact tokens TRUE or CANARY (case-insensitive), use strcasecmp for full-string comparison.
/**
 * @brief Set xconfCheckNow parameter to trigger firmware update check.
 * @param[in] stMsgData Message data containing the parameter value.
 * @return OK on success, NOK on failure.
 */
int set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow(HOSTIF_MsgData_t *stMsgData)

Comment thread src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|68.6%  8203|86.0% 578|    -    0

@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from d90824e to 3678902 Compare August 4, 2026 06:15
Copilot AI review requested due to automatic review settings August 4, 2026 06:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:140

  • handleSetMsg() still references setXreReceiverRestart() and setDevAppRestartRequest() under #ifdef RDKV_TR69, but those functions are not defined anywhere after the XRE/Xcalibur removal. This will fail to compile when RDKV_TR69 is enabled. Since the PR scope says only xconfCheckNow remains, drop these legacy branches (or reintroduce their implementations).
    #ifdef RDKV_TR69
    else if(strcasecmp(stMsgData->paramName,"Device.X_COMCAST-COM_Xcalibur.Client.XRE.xreReceiverRestart") == 0)
    {
        ret = setXreReceiverRestart(stMsgData);
    }
    else if(strcasecmp(stMsgData->paramName,"Device.X_COMCAST-COM_Xcalibur.DevApp.devAppRestartRequest") == 0)
    {
        // xreReceiverRestart uses /lib/rdk/restartReceiver.sh, which sometimes doesn't start receiver back 
        ret = setDevAppRestartRequest(stMsgData);
    }
    #endif

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:259

  • get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() reads at most 9 characters and doesn't populate paramLen/paramtype, which can lead to truncated values and inconsistent TR-069 responses. Prefer reading up to TR69HOSTIFMGR_MAX_PARAM_LEN-1, stripping newline, and setting paramLen + paramtype.
    fscanf(file, "%9s", stMsgData->paramValue);
    fclose(file);
    return OK;

src/hostif/profiles/DeviceInfo/Makefile.am:28

  • AM_CXXFLAGS contains the same MoCA include path twice, which is redundant and makes future maintenance noisier.
-I$(top_srcdir)/src/hostif/profiles/moca \
-I$(top_srcdir)/src/hostif/profiles/moca \
$(GLIB_CFLAGS) $(G_THREAD_CFLAGS) $(MOCAMGR_FLAGS) $(PROCPS_CFLAGS) $(YOCTO_FLAG) \

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:159

  • Doc comment has a double period in "device..", which looks accidental and will show up in generated documentation.
 * @return Returns the status of the operation.
 * @retval OK if it is successfully.
 * @retval ERR_INTERNAL_ERROR if not able to get data from the device..
 * @ingroup TR-069HOSTIF_XRECLIENT_REQHANDLER_CLASSES

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|68.6%  8203|86.0% 578|    -    0

Copilot AI review requested due to automatic review settings August 4, 2026 10:00
@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from 3678902 to bbef606 Compare August 4, 2026 10:00
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|68.6%  8203|86.0% 578|    -    0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:133

  • The file header/note says only Device.X_COMCAST-COM_Xcalibur.Client.xconfCheckNow remains, but handleSetMsg() still accepts other XRE/DevApp control parameters under #ifdef RDKV_TR69 (e.g., ...Client.XRE.xreReceiverRestart, ...DevApp.devAppRestartRequest). If those parameters are deprecated/removed from the data model, this branch should be removed so the handler behavior matches the documented scope.
    #ifdef RDKV_TR69
    else if(strcasecmp(stMsgData->paramName,"Device.X_COMCAST-COM_Xcalibur.Client.XRE.xreReceiverRestart") == 0)
    {
        ret = setXreReceiverRestart(stMsgData);
    }

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:255

  • get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() returns OK without setting paramtype / paramLen, and doesn't clear paramValue before reading. Callers can observe stale buffer contents or a zero length even on success; set hostIf_StringType and paramLen when returning a value.
    if (fscanf(file, "%9s", stMsgData->paramValue) != 1) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error reading file.\n",__FILE__,__FUNCTION__,__LINE__);
        fclose(file);
        return NOK;
    }

src/hostif/profiles/DeviceInfo/Makefile.am:28

  • src/hostif/profiles/DeviceInfo/Makefile.am now has the MoCA include path listed twice consecutively, which is redundant and makes the include list harder to maintain.
-I$(top_srcdir)/src/hostif/profiles/moca \
-I$(top_srcdir)/src/hostif/profiles/moca \
$(GLIB_CFLAGS) $(G_THREAD_CFLAGS) $(MOCAMGR_FLAGS) $(PROCPS_CFLAGS) $(YOCTO_FLAG) \

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:211

  • set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() currently uses strncasecmp(..., strlen("TRUE")) / strlen("CANARY"), which accepts prefix values like TRUEjunk. It also writes the raw value to /tmp/xconfchecknow_val even when the input is invalid (write happens before validation). Validate an exact value first and only then persist a canonical value.
int set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow(HOSTIF_MsgData_t *stMsgData)
{
    FILE *file = fopen("/tmp/xconfchecknow_val", "w");

@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from bbef606 to 5f28316 Compare August 4, 2026 10:29
Copilot AI review requested due to automatic review settings August 4, 2026 10:29
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|63.1%  9593|79.7% 659|    -    0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (6)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:215

  • set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() writes the requested value to /tmp before validating it, so invalid inputs still get persisted. It also matches only a prefix (e.g., "TRUE123" passes) and does not set a faultCode for invalid values. Validate first (exact match), set fcInvalidParameterValue on invalid input, then persist the value only on success.
    FILE *file = fopen("/tmp/xconfchecknow_val", "w");
    if (file == NULL) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error opening file for write.\n",__FILE__,__FUNCTION__,__LINE__);
        return NOK;
    }

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:255

  • get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() populates paramValue but never sets paramLen. Many other handlers set paramLen after filling paramValue, and callers may rely on it for transport/serialization.
    if (fscanf(file, "%9s", stMsgData->paramValue) != 1) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error reading file.\n",__FILE__,__FUNCTION__,__LINE__);
        fclose(file);
        return NOK;
    }

src/hostif/profiles/DeviceInfo/Makefile.am:27

  • Duplicate include path: DeviceInfo/Makefile.am adds the MoCA include directory twice consecutively. This is redundant and makes flag lists harder to maintain.
-I$(top_srcdir)/src/hostif/profiles/moca \
-I$(top_srcdir)/src/hostif/profiles/moca \

docs/api/dml_parameter_list.md:9

  • The access-count summary table is internally inconsistent: 306 readOnly + 543 readWrite = 849, but the Total row says 848.
| readOnly  | 306 | GET only - assert non-empty or expected value |
| readWrite | 543 | GET + SET - verify set/get round-trip |
| **Total** | **848** | |

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:23

  • The file header says only xconfCheckNow remains, but handleSetMsg() still supports additional legacy parameters under RDKV_TR69 (xreReceiverRestart/devAppRestartRequest). Either remove those code paths or update this note so the documented scope matches the actual supported parameters.

This issue also appears in the following locations of the same file:

  • line 211
  • line 251
 * @file hostIf_XREClient_ReqHandler.cpp
 * @brief Minimal handler for Device.X_COMCAST-COM_Xcalibur.Client.xconfCheckNow parameter.
 * @note All XRE/Xcalibur profile features except xconfCheckNow have been removed.

src/hostif/handlers/include/hostIf_XREClient_ReqHandler.h:23

  • The header note claims only xconfCheckNow is supported, but the implementation still has additional legacy SET parameters under RDKV_TR69. Update the note (or remove the legacy paths) so API docs match behavior.
 * @file hostIf_XREClient_ReqHandler.h
 * @brief Minimal handler for Device.X_COMCAST-COM_Xcalibur.Client.xconfCheckNow parameter.
 * @note All XRE/Xcalibur profile features except xconfCheckNow have been removed.
 */

Copilot AI review requested due to automatic review settings August 5, 2026 07:10
@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from 5f28316 to 4f2b8db Compare August 5, 2026 07:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:204

  • set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() persists the raw input to /tmp/xconfchecknow_val before validating it, and the current strncasecmp(..., strlen("TRUE")) / strncasecmp(..., strlen("CANARY")) checks accept prefixes (e.g. "TRUE123") even though the error message/documentation says only "TRUE" or "CANARY" are valid. This can leave an invalid value persisted even when returning NOK, and it can trigger the updater for unintended strings.
    FILE *file = fopen("/tmp/xconfchecknow_val", "w");
    if (file == NULL) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error opening file for write.\n",__FILE__,__FUNCTION__,__LINE__);
        return NOK;
    }

src/Makefile.am:58

  • WiFi profile support is being removed from the build here (the WITH_WIFI_PROFILE block that defined USE_WIFI_PROFILE and added the WiFi include path was deleted), but configure.ac still exposes --enable-wifi / WITH_WIFI_PROFILE and the default runtime config still maps Device.WiFi to wifiMgr (e.g. conf/tr69hostIf.conf:10, conf/mgrlist.conf:9). As-is, --enable-wifi becomes misleading (WiFi libs may build, but the main binary no longer compiles/links WiFi handler support) and the shipped configs reference a manager that will never be registered.
if WITH_DHCP_PROFILE
AM_CXXFLAGS += -DUSE_DHCPv4_PROFILE
AM_CXXFLAGS += -I$(top_srcdir)/src/hostif/profiles/DHCPv4
endif

docs/integration/build-setup.md:33

  • The build documentation still states that --enable-wifi enables WiFi profile support, but this PR removes WiFi handler/profile wiring from the build (e.g. WITH_WIFI_PROFILE blocks removed from src/Makefile.am and src/hostif/handlers/Makefile.am). This flag description should be updated to match the new behavior (removed/unsupported, or clarify what it still builds).
The top-level `configure.ac` currently exposes feature toggles including:

| Flag | Effect |
|------|--------|
| `--enable-moca` / `--enable-moca2` | Enable MoCA profile support |
| `--enable-DHCPv4` | Enable DHCPv4 profile support |
| `--enable-StorageService` | Enable StorageService profile support |
| `--enable-InterfaceStack` | Enable InterfaceStack profile support |
| `--enable-notification` | Enable value-change notification support |

src/hostif/profiles/DeviceInfo/Device_DeviceInfo.cpp:3766

  • set_xRDKCentralComRFC() no longer handles Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.LXC.XRE.Enable (XRE container RFC), but the DeviceInfo gtest suite still asserts this parameter returns OK (e.g. DeviceInfo/gtest/gtest_main.cpp:3608) and run_ut.sh still creates /opt/XRE_container_enable. This will cause the existing unit test to fail (and leaves stale test scaffolding).
    else if (strcasecmp(stMsgData->paramName,LE_RFC_ENABLE) == 0)
    {
        ret = set_xRDKCentralComRFCLoudnessEquivalenceEnable(stMsgData);
    }
    else if (strcasecmp(stMsgData->paramName,DAB_RFC_ENABLE) == 0)
    {
        ret = set_xRDKCentralComDABRFCEnable(stMsgData);
    }
    else if (strcasecmp(stMsgData->paramName,RFC_CTL_RETRIEVE_NOW) == 0)
    {
        ret = set_xRDKCentralComRFCRetrieveNow(stMsgData);

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:246

  • xconfCheckNow set/get behavior is now part of the always-built XREClient handler, and get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() has new error-handling (fscanf return check). There are gtest suites in the repo, but no tests currently exercise Device.X_COMCAST-COM_Xcalibur.Client.xconfCheckNow (set/get, invalid input, missing file, read failure), so regressions here will be untested.
int get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow(HOSTIF_MsgData_t *stMsgData)
{
    FILE *file = fopen("/tmp/xconfchecknow_val", "r");
    if (file == NULL) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error opening file for read.\n",__FILE__,__FUNCTION__,__LINE__);
        return NOK;
    }
    if (fscanf(file, "%9s", stMsgData->paramValue) != 1) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error reading file.\n",__FILE__,__FUNCTION__,__LINE__);
        fclose(file);
        return NOK;
    }
    fclose(file);
    return OK;

Copilot AI review requested due to automatic review settings August 5, 2026 07:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (6)

src/Makefile.am:55

  • WITH_WIFI_PROFILE support appears to have been removed from the build flags, but the project still has a WiFi profile (configure.ac defines WIFI_PROFILE_FLAG and conf files reference Device.WiFi=wifiMgr). As-is, enabling WiFi via configure will no longer define USE_WIFI_PROFILE or add the WiFi include path, so WiFi manager code won’t compile/route correctly.

This issue also appears on line 138 of the same file.

if WITH_DHCP_PROFILE

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:204

  • set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() writes the provided value to /tmp/xconfchecknow_val before validating it. If the input is invalid, the function returns NOK but leaves the persisted state set to the invalid value, which can break subsequent GETs and observability.

This issue also appears on line 242 of the same file.

int set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow(HOSTIF_MsgData_t *stMsgData)
{
    FILE *file = fopen("/tmp/xconfchecknow_val", "w");
    if (file == NULL) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error opening file for write.\n",__FILE__,__FUNCTION__,__LINE__);

src/Makefile.am:140

  • WITH_WIFI_PROFILE is no longer linked into the tr69hostif binary. Since the WiFi profile library still exists (profiles/wifi/libhostIfWiFi.la) and the default configs still map Device.WiFi to wifiMgr, the resulting binary will miss WiFi support even when configured.
AM_CXXFLAGS += -I$(top_srcdir)/src/hostif/parodusClient/pal 
AM_CXXFLAGS += -I$(top_srcdir)/src/hostif/parodusClient/waldb
tr69hostif_LDADD += $(top_builddir)/src/hostif/parodusClient/waldb/libwaldb.la

src/hostif/handlers/Makefile.am:39

  • WIFI profile include/define flags were removed from handler compilation. When WiFi is enabled, handler sources that include hostIf_WiFi_ReqHandler.h rely on USE_WIFI_PROFILE and the wifi include path; without these, WiFi handler support cannot be built even though configure.ac still exposes WITH_WIFI_PROFILE/WIFI_PROFILE_FLAG.
	-I./include $(GLIB_CFLAGS) $(G_THREAD_CFLAGS) \
	$(MOCAMGR_FLAGS) $(SOUP_CFLAGS) $(MOCA2_FLAG) \
	$(XRDK_SDCARD_PROFILE_FLAG) $(XRDK_EMMC_PROFILE_FLAG) \

src/hostif/handlers/Makefile.am:114

  • WiFi handler source is no longer added to libMsgHandlers when WITH_WIFI_PROFILE is enabled, so wifiMgr routing (Device.WiFi) will fail at runtime and/or link-time if other code expects WiFiReqHandler symbols.
if WITH_MOCA_PROFILE
libMsgHandlers_la_SOURCES +=    src/hostIf_MoCAClient_ReqHandler.cpp
endif

if WITH_DHCP_PROFILE
libMsgHandlers_la_SOURCES +=	src/hostIf_DHCPv4Client_ReqHandler.cpp
endif

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:248

  • get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() returns OK without populating paramtype/paramLen. Other handlers consistently set these fields, and leaving them stale can cause incorrect serialization/processing of the response.
    if (fscanf(file, "%9s", stMsgData->paramValue) != 1) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error reading file.\n",__FILE__,__FUNCTION__,__LINE__);
        fclose(file);
        return NOK;
    }
    fclose(file);
    return OK;

Copilot AI review requested due to automatic review settings August 6, 2026 07:23
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|73.9%  9566|90.4% 658|    -    0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/hostif/profiles/DeviceInfo/Makefile.am:27

  • The same MoCA include directory is listed twice in AM_CXXFLAGS, which is redundant and makes future edits noisier.
AM_CXXFLAGS = -I$(top_srcdir)/src/hostif/include \
-I$(top_srcdir)/src/hostif/handlers/include \
-I$(top_srcdir)/src/hostif/handlers/src \
-I$(top_srcdir)/src/hostif/profiles/moca \
-I$(top_srcdir)/src/hostif/profiles/moca \

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:202

  • set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() currently opens/writes /tmp/xconfchecknow_val before validating the input and uses prefix matching via strncasecmp+strlen (so values like "TRUEFOO" would be accepted). This can persist invalid/failed SET values and trigger the upgrader unexpectedly. Consider (1) validating exact allowed values, (2) running the script, and only then (3) persisting the value on success.
int set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow(HOSTIF_MsgData_t *stMsgData)
{
    FILE *file = fopen("/tmp/xconfchecknow_val", "w");

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:246

  • get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() returns OK/NOK but does not populate stMsgData->paramtype or stMsgData->paramLen, which other handlers rely on for correct serialization/logging. Also, fscanf("%9s") truncates the stored value and stops at whitespace; using fgets and trimming newline is safer and preserves the full stored string.
    if (fscanf(file, "%9s", stMsgData->paramValue) != 1) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error reading file.\n",__FILE__,__FUNCTION__,__LINE__);
        fclose(file);
        return NOK;
    }

Copilot AI review requested due to automatic review settings August 7, 2026 09:08
@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from 4c4a596 to 5ddc18e Compare August 7, 2026 09:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:206

  • set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() persists stMsgData->paramValue to /tmp/xconfchecknow_val before validating that the input is one of the supported tokens (TRUE/CANARY). If an invalid value is set, the function returns NOK but the invalid value still gets written, which can cause later GETs to return an unsupported value and makes the failure non-atomic. Validate first, then write the file only for valid inputs.
    FILE *file = fopen("/tmp/xconfchecknow_val", "w");
    if (file == NULL) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error opening file for write.\n",__FILE__,__FUNCTION__,__LINE__);
        return NOK;
    }

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|73.9%  9566|90.4% 658|    -    0

nhanasi
nhanasi previously approved these changes Aug 10, 2026
Copilot AI review requested due to automatic review settings August 11, 2026 05:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:227

  • The xconfCheckNow setter accepts prefix matches (e.g., "TRUE123" passes because it uses strncasecmp with strlen("TRUE")). Also, the value is persisted to /tmp/xconfchecknow_val before validation, so invalid inputs (or script execution failures) can leave an incorrect persisted value even though the SET returns NOK.
    return OK;

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:248

  • After successfully reading xconfCheckNow, the handler returns OK without updating stMsgData->paramtype and stMsgData->paramLen. Other GET handlers populate these fields, and leaving them stale can cause incorrect serialization of the response.
    fclose(file);
    return OK;

@github-actions

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|73.9%  9566|90.4% 658|    -    0

</syntax>
</parameter>
</object>
<object base="Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.XRPairing." access="readOnly" minEntries="0" maxEntries="1" >

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is Xfiniity Remote pairing actually need to be removed ?

Copilot AI review requested due to automatic review settings August 13, 2026 06:41
@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from 0e5aef1 to 36bf4ba Compare August 13, 2026 06:41
@madhubabutt
madhubabutt force-pushed the feature/RDKEMW-19653 branch from 36bf4ba to 4cf71f4 Compare August 13, 2026 06:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:206

  • set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() writes the requested value to /tmp/xconfchecknow_val before validating it, so invalid inputs (or a later script failure) still persist and can affect subsequent GETs. Also v_secure_system() is only checked for -1, which misses non-zero exit statuses.
    FILE *file = fopen("/tmp/xconfchecknow_val", "w");
    if (file == NULL) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error opening file for write.\n",__FILE__,__FUNCTION__,__LINE__);
        return NOK;
    }

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:88

  • getLock() calls g_mutex_init() every time it is invoked. Re-initializing a GMutex after it has been used is undefined behavior and can lead to races/crashes under concurrent access.
void XREClientReqHandler::reset()
{
    getLock();
    RDK_LOG(RDK_LOG_TRACE1,LOG_TR69HOSTIF,"[%s:%s] Entering..\n", __FUNCTION__, __FILE__);
    RDK_LOG(RDK_LOG_TRACE1,LOG_TR69HOSTIF,"[%s:%s] Exiting..\n", __FUNCTION__, __FILE__);

src/Makefile.am:58

  • This PR removes the WITH_WIFI_PROFILE wiring from the top-level build (no -DUSE_WIFI_PROFILE define, no WiFi include path, and no link of libhostIfWiFi.la), but configure.ac still exposes --enable-wifi and src/hostif/profiles/Makefile.am still builds the wifi/ subdir under WITH_WIFI_PROFILE. As-is, --enable-wifi becomes effectively a no-op and may mislead integrators.
if WITH_DHCP_PROFILE
AM_CXXFLAGS += -DUSE_DHCPv4_PROFILE
AM_CXXFLAGS += -I$(top_srcdir)/src/hostif/profiles/DHCPv4
endif

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:240

  • get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() returns OK without populating paramtype/paramLen, which other handlers rely on for correct responses. Using fscanf("%9s") also unnecessarily truncates the stored value (buffer is 4KB).
int get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow(HOSTIF_MsgData_t *stMsgData)
{
    FILE *file = fopen("/tmp/xconfchecknow_val", "r");
    if (file == NULL) {
        RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF, "[%s:%s:%d]Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow: Error opening file for read.\n",__FILE__,__FUNCTION__,__LINE__);
        return NOK;

docs/api/dml_parameter_list.md:9

  • The parameter-list documentation now drops from 904 → 747 total parameters, which includes removing the entire Device.WiFi.* section in addition to XRE/Xcalibur removals. Since the PR description only calls out XRE/Xcalibur deprecation removal, this doc update suggests broader DataModel scope than described.
| readOnly  | 254 | GET only - assert non-empty or expected value |
| readWrite | 493 | GET + SET - verify set/get round-trip |
| **Total** | **747** | |

Copilot AI review requested due to automatic review settings August 13, 2026 06:49
@github-actions

Copy link
Copy Markdown

Code Coverage Summary

                                        Total:|73.9%  9566|90.4% 658|    -    0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:134

  • In handleSetMsg(), the unsupported-parameter branch sets faultCode=fcInvalidParameterName but returns NOT_HANDLED. For a manager that has already been selected based on the prefix, returning NOT_HANDLED can cause the request to be treated as unhandled rather than an invalid parameter. This should return NOK (consistent with other handlers) when the parameter name is invalid.
    else
    {
        stMsgData->faultCode = fcInvalidParameterName;
        ret = NOT_HANDLED;
    }

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:248

  • get_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow() returns OK after reading the value, but it never sets stMsgData->paramLen (and leaves paramtype unchanged). Other handlers populate paramLen for string results; leaving it unset can produce incomplete/incorrect responses for consumers that rely on length.
    fclose(file);
    return OK;

src/hostif/handlers/src/hostIf_XREClient_ReqHandler.cpp:128

  • The PR’s main requirement is to preserve Device.X_COMCAST-COM_Xcalibur.Client.xconfCheckNow support, but there are no unit tests covering the XREClientReqHandler get/set flow (including invalid input handling). Adding a focused gtest would help prevent regressions now that the wider XRE/Xcalibur surface has been removed.
int XREClientReqHandler::handleSetMsg(HOSTIF_MsgData_t *stMsgData)
{
    int ret = NOT_HANDLED;

    stMsgData->instanceNum = 0;

    RDK_LOG(RDK_LOG_TRACE1,LOG_TR69HOSTIF,"[%s:%s:%d] Found string as %s\n", __FUNCTION__, __FILE__, __LINE__, stMsgData->paramName);

    getLock();

    if (strcasecmp (stMsgData->paramName, "Device.X_COMCAST-COM_Xcalibur.Client.xconfCheckNow") == 0)
    {
        ret = set_Device_X_COMCAST_COM_Xcalibur_Client_xconfCheckNow (stMsgData);

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.

4 participants