Feature/rdkemw 9659 device settings client library by ManimaranRenganathan · Pull Request #211 · rdkcentral/devicesettings

ManimaranRenganathan · 2026-02-04T08:12:13Z

No description provided.

… and Interface changes

…Thunder is enabled.

Copilot

Pull request overview

Adds a Thunder COM-RPC based Front Panel Display (FPD) client implementation and introduces build-time selection between the existing IARM implementation and the new Thunder path.

Changes:

Added dsFPD-com.cpp implementing the dsFPD C API via Thunder Exchange::IDeviceSettingsFPD.
Added conditional build selection for FPD source (dsFPD.c vs dsFPD-com.cpp) and Thunder linking in rpc/cli/Makefile.am and rpc/cli/Makefile.
Added --enable-thunder-plugin configure option and automake conditional in configure.ac.

Reviewed changes

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

File	Description
rpc/cli/dsFPD-com.cpp	New Thunder COM-RPC backed implementation of dsFPD APIs.
rpc/cli/Makefile.am	Automake conditional to choose Thunder vs IARM FPD source and link Thunder libs.
rpc/cli/Makefile	Manual build conditional for Thunder vs IARM and conditional linking.
configure.ac	Adds `--enable-thunder-plugin` flag + conditional and macro definition.

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

Copilot · 2026-02-04T08:19:53Z

rpc/cli/dsFPD-com.cpp

+        _apiLock.Lock();
+        if (nullptr == _instance) {
+            _instance = new DeviceSettingsFPD();
+        }
+        _apiLock.Unlock();


DeviceSettingsFPD::Init() holds _apiLock while constructing DeviceSettingsFPD. The constructor calls Connect(), which also locks _apiLock, causing a self-deadlock (non-recursive CriticalSection) on the first Init(). Refactor so construction/Connect happen outside the locked section (or remove Connect from ctor and call it after releasing the lock, or use std::call_once/static-local singleton).

Suggested change

_apiLock.Lock();

if (nullptr == _instance) {

_instance = new DeviceSettingsFPD();

}

_apiLock.Unlock();

// Fast path without locking

if (_instance != nullptr) {

return;

}

// Construct outside of _apiLock to avoid self-deadlock when the

// constructor calls Connect(), which also locks _apiLock.

DeviceSettingsFPD* newInstance = new DeviceSettingsFPD();

_apiLock.Lock();

if (_instance == nullptr) {

_instance = newInstance;

newInstance = nullptr;

}

_apiLock.Unlock();

// If another thread initialized _instance first, delete the extra instance.

if (newInstance != nullptr) {

delete newInstance;

}

Copilot · 2026-02-04T08:19:54Z

rpc/cli/dsFPD-com.cpp

+        _apiLock.Lock();
+        if (nullptr != _instance) {
+            delete _instance;
+            _instance = nullptr;
+        }
+        _apiLock.Unlock();


DeviceSettingsFPD::Term() deletes the singleton while holding _apiLock. ~DeviceSettingsFPD() calls BaseClass::Close(), and the Operational() callback also locks _apiLock, which can deadlock during shutdown. Avoid holding _apiLock across delete/Close; e.g., move the pointer to a local under lock, clear _instance, unlock, then close/delete outside the lock.

Suggested change

_apiLock.Lock();

if (nullptr != _instance) {

delete _instance;

_instance = nullptr;

}

_apiLock.Unlock();

DeviceSettingsFPD* instance = nullptr;

_apiLock.Lock();

if (nullptr != _instance) {

instance = _instance;

_instance = nullptr;

}

_apiLock.Unlock();

if (instance != nullptr) {

delete instance;

}

Copilot · 2026-02-04T08:19:54Z

rpc/cli/dsFPD-com.cpp

+    // Note: Interface expects minutes and seconds, but API provides hour and minutes
+    // Converting: treating uMinutes as seconds for interface compatibility
+    uint32_t result = instance->SetFPDTime(timeFormat, uHour, uMinutes);


dsSetFPTime() passes (uHour, uMinutes) directly into SetFPDTime(minutes, seconds) while the DS API semantics are hours/minutes (see ds/frontPanelTextDisplay.cpp where setTime(14,35) calls dsSetFPTime). This will render incorrect time values on the panel. Align the Thunder call with DS semantics (prefer an hours/minutes API if available, or convert hours/minutes into the expected parameters correctly instead of treating minutes as seconds).

Suggested change

// Note: Interface expects minutes and seconds, but API provides hour and minutes

// Converting: treating uMinutes as seconds for interface compatibility

uint32_t result = instance->SetFPDTime(timeFormat, uHour, uMinutes);

// The Thunder interface expects (minutes, seconds), while the DS API provides (hours, minutes).

// Convert DS (hour, minute) into total minutes for the Thunder "minutes" parameter and use 0 seconds.

const unsigned int thunderMinutes = (uHour * 60u) + uMinutes;

const unsigned int thunderSeconds = 0u;

uint32_t result = instance->SetFPDTime(timeFormat, thunderMinutes, thunderSeconds);

Copilot · 2026-02-04T08:19:54Z

rpc/cli/dsFPD-com.cpp

+    // Wait for plugin to become operational
+    if (!instance->WaitForOperational(5000)) {
+        fprintf(stderr, "[dsFPD-com] DeviceSettings plugin not operational after 5 seconds\n");
+        return dsERR_GENERAL;
+    }


dsFPInit() blocks up to 5 seconds waiting for WaitForOperational(). Callers already implement retry loops (e.g., ds/frontPanelConfig.cpp retries dsFPInit up to 20 times), so this can amplify startup latency to ~100 seconds. Consider making dsFPInit non-blocking/shorter (or detect repeated init attempts and avoid the full wait) so higher-level retry logic controls timing.

Copilot · 2026-02-04T08:19:55Z

rpc/cli/Makefile

+# Conditional compilation: Thunder vs IARM
+ifdef USE_WPE_THUNDER_PLUGIN
+    # Thunder mode - use dsFPD-com.cpp, exclude dsFPD.c
+    OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
+    OBJS += $(patsubst %.c,%.o,$(filter-out dsFPD.c,$(wildcard *.c)))
+else
+    # IARM mode - use dsFPD.c, exclude dsFPD-com.cpp
+    OBJS := $(patsubst %.cpp,%.o,$(filter-out dsFPD-com.cpp,$(wildcard *.cpp)))
+    OBJS += $(patsubst %.c,%.o,$(wildcard *.c))
+endif


When USE_WPE_THUNDER_PLUGIN is set, the build switches to dsFPD-com.cpp but does not add -DUSE_WPE_THUNDER_PLUGIN to the compiler flags. Because dsFPD-com.cpp wraps all symbols in #ifdef USE_WPE_THUNDER_PLUGIN, it will compile to an empty object and the FPD C API symbols will be missing (and dsFPD.c is excluded). Add the define to CFLAGS/CPPFLAGS in this branch (or remove the #ifdef from dsFPD-com.cpp and rely on build selection).

Copilot · 2026-02-04T08:19:55Z

rpc/cli/Makefile


+# Conditional linking flags
+ifdef USE_WPE_THUNDER_PLUGIN
+    LDLIBS := -lWPEFrameworkCore -lWPEFrameworkCOM


In Thunder mode, LDLIBS is set to only -lWPEFrameworkCore/-lWPEFrameworkCOM, but most of the CLI objects (e.g., dsAudio.c/dsDisplay.c/dsHost.cpp) still call IARM_Bus_* APIs. Dropping -lIARMBus can lead to undefined-symbol failures at link/load time. Include -lIARMBus in Thunder mode as well (or gate the other IARM-based sources behind the same feature flag).

Suggested change

LDLIBS := -lWPEFrameworkCore -lWPEFrameworkCOM

LDLIBS := -lWPEFrameworkCore -lWPEFrameworkCOM -lIARMBus

Copilot · 2026-02-04T08:19:55Z

rpc/cli/Makefile.am

+# Conditional compilation for Thunder COM-RPC
+if USE_THUNDER_PLUGIN
+    FPD_SOURCE = dsFPD-com.cpp
+    THUNDER_LIBS = -lWPEFrameworkCore -lWPEFrameworkCOM
+else
+    FPD_SOURCE = dsFPD.c
+    THUNDER_LIBS =
+endif
+
+libdshalcli_la_SOURCES = dsAudio.c dsclientlogger.c dsDisplay.c $(FPD_SOURCE) dsHost.cpp dsVideoDevice.c dsVideoPort.c


The automake conditional selects dsFPD-com.cpp when USE_THUNDER_PLUGIN is enabled, but nothing in this Makefile.am passes -DUSE_WPE_THUNDER_PLUGIN (and the sources do not include cfg/config.h). As written, dsFPD-com.cpp will compile with all symbols excluded by its #ifdef. Add the define to libdshalcli_la_CPPFLAGS under this conditional (or include config.h in sources and define HAVE_CONFIG_H appropriately).

Copilot · 2026-02-04T08:19:55Z

configure.ac

+    [AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])
+     AC_MSG_NOTICE([Thunder COM-RPC plugin support enabled])],


--enable-thunder-plugin only AC_DEFINEs USE_WPE_THUNDER_PLUGIN into cfg/config.h, but no source in this repo includes config.h and the build files don’t add -DUSE_WPE_THUNDER_PLUGIN, so the option will not actually enable the Thunder codepaths. Also, enabling this feature doesn’t check for required Thunder headers/libs (WPEFrameworkCore/COM), so configure can succeed and fail later at build/link. Consider adding proper detection (PKG_CHECK_MODULES/AC_CHECK_LIB+AC_CHECK_HEADERS) and exporting the needed CPPFLAGS/LIBS when enabled.

Suggested change

[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])

AC_MSG_NOTICE([Thunder COM-RPC plugin support enabled])],

[PKG_CHECK_MODULES([THUNDER],

[WPEFrameworkCore WPEFrameworkCOM],

[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])

AC_MSG_NOTICE([Thunder COM-RPC plugin support enabled])],

[AC_MSG_ERROR([Thunder COM-RPC plugin support requested, but WPEFrameworkCore and/or WPEFrameworkCOM were not found])])],

1. Created the dsConnectionManager.cpp, dsConnectionManager.h and dsHdmiIn-com.cpp files 2. Modified the dsFPD-com.cpp file with SmartInterfaceType to get the common interface from FPD and using QueryInterface to get the interface handle for other component like HdmiIn, CompositeIn, etc.,

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 16 comments.

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

Copilot · 2026-02-05T05:18:17Z

rpc/cli/dsFPD-com.cpp

+
+dsError_t dsSetFPDBrightness(dsFPDIndicator_t eIndicator, dsFPDBrightness_t eBrightness, bool toPersist)
+{
+    if (eIndicator >= dsFPD_INDICATOR_MAX || eBrightness > 100) {


The brightness parameter is validated as eBrightness > 100, but dsFPDBrightness_t is likely an enum type, not a percentage value. This validation may be incorrect. Check the definition of dsFPDBrightness_t - if it's an enum with values like dsFPD_BRIGHTNESS_MIN, dsFPD_BRIGHTNESS_MAX, etc., the validation should check against the enum max value, not 100.

Suggested change

if (eIndicator >= dsFPD_INDICATOR_MAX || eBrightness > 100) {

if (eIndicator >= dsFPD_INDICATOR_MAX || eBrightness >= dsFPD_BRIGHTNESS_MAX) {

Copilot · 2026-02-05T05:18:17Z

rpc/cli/dsHdmiIn-com.cpp

+dsError_t dsHdmiInRegisterConnectCB(dsHdmiInConnectCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterConnectCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterSignalChangeCB(dsHdmiInSignalChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterSignalChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterStatusChangeCB(dsHdmiInStatusChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterStatusChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterVideoModeUpdateCB(dsHdmiInVideoModeUpdateCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterVideoModeUpdateCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAllmChangeCB(dsHdmiInAllmChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAllmChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAVLatencyChangeCB(dsAVLatencyChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAVLatencyChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAviContentTypeChangeCB(dsHdmiInAviContentTypeChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAviContentTypeChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}


All callback registration functions return dsERR_OPERATION_NOT_SUPPORTED, indicating that event notifications are not implemented in Thunder mode. This is a significant functional difference from the IARM implementation. Applications relying on these callbacks (connection events, signal changes, status changes, video mode updates, ALLM changes, AV latency changes, AVI content type changes) will not receive notifications in Thunder mode. This limitation should be clearly documented in release notes or migration guides, as it may break existing functionality for applications that depend on these events.

Copilot · 2026-02-05T05:18:17Z

rpc/cli/dsHdmiIn-com.cpp

+
+        // TODO: Handle portConnectionStatus iterator if needed
+        if (portConnectionStatus) {
+            portConnectionStatus->Release();


The portConnectionStatus iterator may be nullptr even when the result is ERROR_NONE (depending on the Thunder interface implementation). The code only checks if portConnectionStatus is not null before releasing it, but doesn't handle the case where it might be null. While this check prevents a crash, if portConnectionStatus is expected to contain data, the absence of an error log or handling when it's unexpectedly null could hide interface implementation issues. Consider adding a warning log if portConnectionStatus is null when result is ERROR_NONE.

Suggested change

portConnectionStatus->Release();

portConnectionStatus->Release();

} else {

fprintf(stderr, "[dsHdmiIn-com] WARNING: GetHDMIInStatus returned ERROR_NONE but portConnectionStatus is NULL\n");

Copilot · 2026-02-05T05:18:17Z

rpc/cli/dsHdmiIn-com.cpp

+
+    int32_t count = 0;
+    ConnectionManager::Lock();
+    uint32_t result = hdmiInInterface->GetHDMIInNumbefOfInputs(count);


The method name "GetHDMIInNumbefOfInputs" contains a typo. It should be "GetHDMIInNumberOfInputs" (with "Number" instead of "Numbef"). This appears to be calling an interface method defined elsewhere, so verify that the interface definition matches this spelling. If the interface has the correct spelling, this call will fail.

Suggested change

uint32_t result = hdmiInInterface->GetHDMIInNumbefOfInputs(count);

uint32_t result = hdmiInInterface->GetHDMIInNumberOfInputs(count);

Copilot · 2026-02-05T05:18:18Z

rpc/cli/dsConnectionManager.cpp

+ConnectionManager::~ConnectionManager()
+{
+    printf("[dsConnectionManager] Destroying connection manager\n");
+    _shutdown = true;
+
+    // Release all component interfaces before closing base connection
+    if (_hdmiInInterface) {
+        _hdmiInInterface->Release();
+        _hdmiInInterface = nullptr;
+    }
+
+    if (_fpdInterface) {
+        _fpdInterface->Release();
+        _fpdInterface = nullptr;
+    }
+
+    BaseClass::Close(Core::infinite);
+}


There is a potential deadlock issue in the destructor. The destructor calls Release() on interfaces which may trigger callbacks or other operations that could attempt to acquire _dsConnectionManagerlock, but the lock is not held. However, if other threads are still using GetFPDInterface() or GetHDMIInInterface(), they will hold the lock while using these interfaces. If those operations are in progress when the destructor releases the interfaces, it could cause race conditions. Consider ensuring all API calls are completed before destroying the singleton, or document that Term() must only be called when no other threads are using the interfaces.

Copilot · 2026-02-05T05:18:19Z

cov_build.sh

 export STANDALONE_BUILD_ENABLED=y
 export DS_MGRS=$WORKDIR

+export USE_WPE_THUNDER_PLUGIN=y


The cov_build.sh script exports USE_WPE_THUNDER_PLUGIN=y but doesn't pass it as a compiler flag. The Makefile checks for USE_WPE_THUNDER_PLUGIN using ifdef, which checks if the variable is defined in the make environment or as a make variable, not as a shell environment variable. Either export it as a make variable using "export USE_WPE_THUNDER_PLUGIN" in the Makefile, or pass it to make using "make USE_WPE_THUNDER_PLUGIN=y", or add -DUSE_WPE_THUNDER_PLUGIN to CFLAGS.

Copilot · 2026-02-05T05:18:19Z

rpc/cli/dsFPD-com.cpp

+ * If not stated otherwise in this file or this component's LICENSE file the
+ * following copyright and licenses apply:
+ *
+ * Copyright 2016 RDK Management


The copyright year is listed as 2016, but this is a new file being added in 2025. The copyright year should be 2025 to reflect when the file was created, as seen in dsHdmiIn-com.cpp (line 5) and dsConnectionManager files.

Suggested change

* Copyright 2016 RDK Management

* Copyright 2025 RDK Management

Copilot · 2026-02-05T05:18:20Z

rpc/cli/dsFPD-com.cpp

+    // Note: Interface expects minutes and seconds, but API provides hour and minutes
+    // Converting: treating uMinutes as seconds for interface compatibility


The comment states "treating uMinutes as seconds for interface compatibility" which suggests a potential parameter mapping issue. The function signature uses uHour and uMinutes, but the comment indicates uMinutes is being treated as seconds when passed to SetFPDTime. This could lead to incorrect time display. Verify that the Thunder interface expects (hours, minutes) as stated in the function parameters, not (minutes, seconds) as the comment suggests.

Suggested change

// Note: Interface expects minutes and seconds, but API provides hour and minutes

// Converting: treating uMinutes as seconds for interface compatibility

// Note: Thunder interface expects hour and minutes, matching the dsSetFPTime API

// Parameters are passed through directly without conversion

Copilot · 2026-02-05T05:18:20Z

rpc/cli/dsFPD-com.cpp

+    Exchange::IDeviceSettingsFPD::FPDIndicator indicator = 
+        static_cast<Exchange::IDeviceSettingsFPD::FPDIndicator>(eIndicator);
+
+    // Thunder interface doesn't support persist flag - ignore it


The persist/toPersist flag is being ignored in the Thunder implementation. The comment on line 356 states "Thunder interface doesn't support persist flag - ignore it", but this means the behavior differs from the original IARM implementation where persist affects whether settings are saved permanently. Consider documenting this limitation more prominently or implementing persistence through an alternative mechanism if it's critical functionality.

Suggested change

// Thunder interface doesn't support persist flag - ignore it

// NOTE: Thunder IDeviceSettingsFPD interface does not support a persist flag.

// The toPersist parameter is kept for API compatibility with the IARM

// implementation but is not honored here: settings are always applied in

// the same way regardless of the flag, and persistence behavior (if any)

// is determined solely by the Thunder implementation.

if (!toPersist) {

fprintf(stderr,

"[dsFPD-com] Warning: persist flag is not supported in Thunder "

"implementation; applying FPD color without persistence control\n");

}

Copilot · 2026-02-05T05:18:20Z

rpc/cli/dsHdmiIn-com.cpp

+    ConnectionManager::Lock();
+    uint32_t result = hdmiInInterface->GetEdidBytes(port, static_cast<uint16_t>(*length), edid);
+    ConnectionManager::Unlock();
+
+    return ConvertThunderError(result);


The length parameter is used as an input to specify the buffer size, but it's not updated with the actual number of bytes written by the GetEdidBytes call. In the original IARM implementation, the length parameter is typically both input and output. After the Thunder interface call, the length should be updated to reflect the actual EDID data size returned. Without this, the caller won't know how much valid data is in the buffer.

Suggested change

ConnectionManager::Lock();

uint32_t result = hdmiInInterface->GetEdidBytes(port, static_cast<uint16_t>(*length), edid);

ConnectionManager::Unlock();

return ConvertThunderError(result);

// Use a local variable for the in/out length parameter so that any

// updates made by GetEdidBytes are visible to the caller.

uint16_t edidLength = static_cast<uint16_t>(*length);

ConnectionManager::Lock();

uint32_t result = hdmiInInterface->GetEdidBytes(port, edidLength, edid);

ConnectionManager::Unlock();

dsError_t error = ConvertThunderError(result);

if (error == dsERR_NONE) {

*length = static_cast<int>(edidLength);

}

return error;

Copilot

Pull request overview

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

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

Copilot · 2026-02-05T07:49:50Z

rpc/cli/dsFPD-com.cpp

+dsError_t dsSetFPState(dsFPDIndicator_t eIndicator, dsFPDState_t state)
+{
+    ConnectionManager* connMgr = ConnectionManager::Instance();
+    if (!connMgr || !connMgr->IsOperational()) {
+        return dsERR_GENERAL;
+    }
+
+    Exchange::IDeviceSettingsFPD* fpdInterface = connMgr->GetFPDInterface();
+    if (!fpdInterface) {
+        return dsERR_GENERAL;
+    }
+
+    Exchange::IDeviceSettingsFPD::FPDIndicator indicator = 
+        static_cast<Exchange::IDeviceSettingsFPD::FPDIndicator>(eIndicator);
+    Exchange::IDeviceSettingsFPD::FPDState fpdState = 
+        static_cast<Exchange::IDeviceSettingsFPD::FPDState>(state);
+
+    ConnectionManager::Lock();
+    uint32_t result = fpdInterface->SetFPDState(indicator, fpdState);
+    ConnectionManager::Unlock();
+
+    return ConvertThunderError(result);
+}


Inconsistent input validation: dsSetFPDBrightness validates eIndicator and eBrightness parameters, but similar setter functions like dsSetFPState, dsSetFPDColor, and dsSetFPTextBrightness do not validate their input parameters before casting and using them. Consider adding consistent validation across all setter functions to prevent invalid enum values from being passed to the Thunder interface.

Copilot · 2026-02-05T07:49:51Z

rpc/cli/dsConnectionManager.cpp

+
+ConnectionManager* ConnectionManager::Instance()
+{
+    return _instance;


The Instance() method returns the raw pointer without any locking, which creates a race condition with Term(). If Instance() is called while Term() is deleting the instance in another thread, it could return a pointer to a partially destroyed object. Consider adding locking to Instance() or using atomic operations for thread-safe singleton access.

Suggested change

return _instance;

_dsConnectionManagerlock.Lock();

ConnectionManager* instance = _instance;

_dsConnectionManagerlock.Unlock();

return instance;

Copilot · 2026-02-05T07:49:51Z

rpc/cli/dsHdmiIn-com.cpp

+
+    int32_t count = 0;
+    ConnectionManager::Lock();
+    uint32_t result = hdmiInInterface->GetHDMIInNumbefOfInputs(count);


The method name appears to have a typo: "GetHDMIInNumbefOfInputs" should likely be "GetHDMIInNumberOfInputs" (Numbef -> NumberOf).

Suggested change

uint32_t result = hdmiInInterface->GetHDMIInNumbefOfInputs(count);

uint32_t result = hdmiInInterface->GetHDMIInNumberOfInputs(count);

Copilot · 2026-02-05T07:49:51Z

rpc/cli/Makefile

+#OBJS        := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
+#OBJS        += $(patsubst %.c,%.o,$(wildcard *.c))


These commented-out lines should be removed rather than left as comments. Dead code in makefiles can cause confusion about the actual build logic.

Suggested change

#OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))

#OBJS += $(patsubst %.c,%.o,$(wildcard *.c))

Copilot · 2026-02-05T07:49:51Z

rpc/cli/dsHdmiIn-com.cpp

+    uint32_t result = hdmiInInterface->GetEdidBytes(port, static_cast<uint16_t>(*length), edid);
+    ConnectionManager::Unlock();


The function casts *length to uint16_t for passing to GetEdidBytes, but doesn't update *length with the actual bytes retrieved. The caller may expect *length to be updated with the actual EDID data size returned. Verify the interface contract and update *length if needed.

Copilot · 2026-02-05T07:49:52Z

rpc/cli/Makefile

+ifdef USE_WPE_THUNDER_PLUGIN
+    # Thunder mode - use *-com.cpp files, exclude legacy .c implementations
+    OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
+    OBJS += $(patsubst %.c,%.o,$(filter-out dsFPD.c dsHdmiIn.c,$(wildcard *.c)))


In the Thunder mode (when USE_WPE_THUNDER_PLUGIN is defined), the Makefile excludes dsFPD.c and dsHdmiIn.c but includes dsCompositeIn.c. However, the Makefile.am sets COMPOSITEIN_SOURCE to empty when USE_THUNDER_PLUGIN is enabled. This creates an inconsistency between the two build systems - Makefile will include dsCompositeIn.c while Makefile.am won't. Both should handle dsCompositeIn.c consistently.

Suggested change

OBJS += $(patsubst %.c,%.o,$(filter-out dsFPD.c dsHdmiIn.c,$(wildcard *.c)))

OBJS += $(patsubst %.c,%.o,$(filter-out dsFPD.c dsHdmiIn.c dsCompositeIn.c,$(wildcard *.c)))

Copilot · 2026-02-05T07:49:52Z

rpc/cli/Makefile

+# Conditional linking flags
+ifdef USE_WPE_THUNDER_PLUGIN
+    LDLIBS := -lWPEFrameworkCore -lWPEFrameworkCOM
+else
+    LDLIBS := -lIARMBus
+endif


When USE_WPE_THUNDER_PLUGIN is set as an environment variable or make variable, the Makefile correctly adjusts OBJS and LDLIBS. However, the C/C++ preprocessor directives in the source code also check for USE_WPE_THUNDER_PLUGIN. The Makefile should add -DUSE_WPE_THUNDER_PLUGIN to CFLAGS when this variable is set, otherwise the source files won't be compiled with the correct preprocessor definition.

Copilot · 2026-02-05T07:49:52Z

rpc/cli/dsFPD-com.cpp

+    // Thunder interface doesn't support persist flag - ignore it
+    ConnectionManager::Lock();
+    uint32_t result = fpdInterface->SetFPDColor(indicator, static_cast<uint32_t>(eColor));


The persist flag parameter is accepted but silently ignored, which could lead to unexpected behavior where callers expect persistence but it's not implemented. Consider either implementing persistence support in the Thunder interface or returning dsERR_OPERATION_NOT_SUPPORTED when toPersist is true.

This reverts commit 70a09eb.

…/rpc/cli" This reverts commit 402f9bf.

This reverts commit 94475ec.

This reverts commit d220f98.

This reverts commit 59ccd08.

This reverts commit 8c15bc0.

…ettings /rpc/cli"" This reverts commit 7e25fcc.

…p file"" This reverts commit 3944baf.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.

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

Copilot · 2026-02-05T10:07:39Z

rpc/cli/dsHdmiIn-com.cpp

+
+        // TODO: Handle portConnectionStatus iterator if needed
+        if (portConnectionStatus) {
+            portConnectionStatus->Release();
+        }
+    }


Potential memory leak: if the interface method GetHDMIInStatus sets portConnectionStatus to a non-null value but returns an error, the iterator will not be released. The Release() call should be moved outside the result check to ensure cleanup in all cases, or there should be an else branch to handle the error case.

Suggested change

// TODO: Handle portConnectionStatus iterator if needed

if (portConnectionStatus) {

portConnectionStatus->Release();

}

}

}

// TODO: Handle portConnectionStatus iterator if needed

if (portConnectionStatus) {

portConnectionStatus->Release();

}

Copilot · 2026-02-05T10:07:39Z

rpc/cli/dsHdmiIn-com.cpp

+    ConnectionManager* connMgr = ConnectionManager::Instance();
+    if (!connMgr) {
+        ConnectionManager::Init();
+        connMgr = ConnectionManager::Instance();
+    }


Potential race condition in ConnectionManager initialization. Lines 71-75 check if the instance exists, then call Init() if it doesn't, then get the instance again. However, there's no locking between the first Instance() check and the Init() call. If two threads call dsHdmiInInit() simultaneously, both could see a null instance and both could call Init(), potentially creating the instance twice. While ConnectionManager::Init() has internal locking, the pattern here should either be: (1) always call Init() which is idempotent, or (2) use a lock around this entire sequence.

Suggested change

ConnectionManager* connMgr = ConnectionManager::Instance();

if (!connMgr) {

ConnectionManager::Init();

connMgr = ConnectionManager::Instance();

}

ConnectionManager::Init();

ConnectionManager* connMgr = ConnectionManager::Instance();

Copilot · 2026-02-05T10:07:39Z

rpc/cli/dsHdmiIn-com.cpp

+dsError_t dsHdmiInRegisterSignalChangeCB(dsHdmiInSignalChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterSignalChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterStatusChangeCB(dsHdmiInStatusChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterStatusChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterVideoModeUpdateCB(dsHdmiInVideoModeUpdateCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterVideoModeUpdateCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAllmChangeCB(dsHdmiInAllmChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAllmChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAVLatencyChangeCB(dsAVLatencyChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAVLatencyChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAviContentTypeChangeCB(dsHdmiInAviContentTypeChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAviContentTypeChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+


Unused parameter warning suppression missing. The CBFunc parameters in all these callback registration stub functions (lines 625-660) are not used. To avoid compiler warnings, each unused parameter should be marked with (void)CBFunc or use an attribute like [[maybe_unused]] or attribute((unused)).

Copilot · 2026-02-05T10:07:39Z

cov_build.sh


 echo "##### Triggering make"
-make CFLAGS+='-fPIC -DDSMGR_LOGGER_ENABLED=ON -DRDK_DSHAL_NAME=\"libdshal.so\" -I${DS_IF_PATH}/include -I${DS_HAL_PATH} -I${DS_MGRS}/stubs  -I${IARMBUS_PATH}/core -I${IARMBUS_PATH}/core/include -I${IARM_MGRS}/sysmgr/include -I${DS_MGRS}/ds/include -I${DS_MGRS}/rpc/include -I${POWER_IF_PATH}/include/ -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I${IARM_MGRS}/mfr/include/ -I${IARM_MGRS}/mfr/common -I${DEEPSLEEP_IF_PATH}/include -I${IARM_MGRS}/hal/include -I${IARM_MGRS}/power -I${IARM_MGRS}/power/include' LDFLAGS="-L/usr/lib/x86_64-linux-gnu/ -L/usr/local/include -lglib-2.0 -lIARMBus -lWPEFrameworkPowerController -ldshal"
+make CFLAGS+='-fPIC -DDSMGR_LOGGER_ENABLED=ON -DRDK_DSHAL_NAME=\"libdshal.so\" -I${DS_IF_PATH}/include -I${DS_HAL_PATH} -I${DS_MGRS}/stubs  -I${IARMBUS_PATH}/core -I${IARMBUS_PATH}/core/include -I${IARM_MGRS}/sysmgr/include -I${DS_MGRS}/ds/include -I${DS_MGRS}/rpc/include -I${POWER_IF_PATH}/include/ -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I${IARM_MGRS}/mfr/include/ -I${IARM_MGRS}/mfr/common -I${DEEPSLEEP_IF_PATH}/include -I${IARM_MGRS}/hal/include -I${IARM_MGRS}/power -I${IARM_MGRS}/power/include' LDFLAGS="-L/usr/lib/x86_64-linux-gnu/ -L/usr/local/include -lglib-2.0 -lIARMBus -lWPEFrameworkPowerController -ldshal"


The USE_WPE_THUNDER_PLUGIN environment variable is set on line 69, but the macro definition is not passed to the compiler in CFLAGS on line 75. The CFLAGS should include -DUSE_WPE_THUNDER_PLUGIN to ensure the conditional compilation directives (#ifdef USE_WPE_THUNDER_PLUGIN) in the source files work correctly. Without this, the Thunder code paths won't be compiled even though USE_WPE_THUNDER_PLUGIN is exported.

Suggested change

make CFLAGS+='-fPIC -DDSMGR_LOGGER_ENABLED=ON -DRDK_DSHAL_NAME=\"libdshal.so\" -I${DS_IF_PATH}/include -I${DS_HAL_PATH} -I${DS_MGRS}/stubs -I${IARMBUS_PATH}/core -I${IARMBUS_PATH}/core/include -I${IARM_MGRS}/sysmgr/include -I${DS_MGRS}/ds/include -I${DS_MGRS}/rpc/include -I${POWER_IF_PATH}/include/ -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I${IARM_MGRS}/mfr/include/ -I${IARM_MGRS}/mfr/common -I${DEEPSLEEP_IF_PATH}/include -I${IARM_MGRS}/hal/include -I${IARM_MGRS}/power -I${IARM_MGRS}/power/include' LDFLAGS="-L/usr/lib/x86_64-linux-gnu/ -L/usr/local/include -lglib-2.0 -lIARMBus -lWPEFrameworkPowerController -ldshal"

make CFLAGS+='-fPIC -DDSMGR_LOGGER_ENABLED=ON -DUSE_WPE_THUNDER_PLUGIN -DRDK_DSHAL_NAME=\"libdshal.so\" -I${DS_IF_PATH}/include -I${DS_HAL_PATH} -I${DS_MGRS}/stubs -I${IARMBUS_PATH}/core -I${IARMBUS_PATH}/core/include -I${IARM_MGRS}/sysmgr/include -I${DS_MGRS}/ds/include -I${DS_MGRS}/rpc/include -I${POWER_IF_PATH}/include/ -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I${IARM_MGRS}/mfr/include/ -I${IARM_MGRS}/mfr/common -I${DEEPSLEEP_IF_PATH}/include -I${IARM_MGRS}/hal/include -I${IARM_MGRS}/power -I${IARM_MGRS}/power/include' LDFLAGS="-L/usr/lib/x86_64-linux-gnu/ -L/usr/local/include -lglib-2.0 -lIARMBus -lWPEFrameworkPowerController -ldshal"

Copilot · 2026-02-05T10:07:40Z

rpc/cli/dsHdmiIn-com.cpp

+
+    int32_t count = 0;
+    ConnectionManager::Lock();
+    uint32_t result = hdmiInInterface->GetHDMIInNumbefOfInputs(count);


The method name "GetHDMIInNumbefOfInputs" contains a typo. "Numbef" should be "NumberOf". This appears to be calling an interface method, so if this typo exists in the interface definition, it should be fixed there as well.

Suggested change

uint32_t result = hdmiInInterface->GetHDMIInNumbefOfInputs(count);

uint32_t result = hdmiInInterface->GetHDMIInNumberOfInputs(count);

Copilot · 2026-02-05T10:07:40Z

rpc/cli/dsFPD-com.cpp

+    // Note: Interface expects minutes and seconds, but API provides hour and minutes
+    // Converting: treating uMinutes as seconds for interface compatibility
+    uint32_t result = fpdInterface->SetFPDTime(timeFormat, uHour, uMinutes);


The comment indicates a potential parameter mismatch. The API receives hour and minutes (uHour, uMinutes), but the comment states the interface expects minutes and seconds. If the interface truly expects minutes and seconds, then passing uMinutes as the third parameter would be incorrect - it should be 0 or a calculated seconds value. This needs verification against the IDeviceSettingsFPD interface definition to ensure the parameters are correctly mapped.

Copilot · 2026-02-05T10:07:40Z

rpc/cli/dsFPD-com.cpp

+    ConnectionManager* connMgr = ConnectionManager::Instance();
+    if (!connMgr) {
+        ConnectionManager::Init();
+        connMgr = ConnectionManager::Instance();
+    }


Potential race condition in ConnectionManager initialization. Lines 75-79 check if the instance exists, then call Init() if it doesn't, then get the instance again. However, there's no locking between the first Instance() check and the Init() call. If two threads call dsFPInit() simultaneously, both could see a null instance and both could call Init(), potentially creating the instance twice. While ConnectionManager::Init() has internal locking, the pattern here should either be: (1) always call Init() which is idempotent, or (2) use a lock around this entire sequence.

Suggested change

ConnectionManager* connMgr = ConnectionManager::Instance();

if (!connMgr) {

ConnectionManager::Init();

connMgr = ConnectionManager::Instance();

}

// Initialize ConnectionManager (safe to call multiple times; handles its own locking)

ConnectionManager::Init();

ConnectionManager* connMgr = ConnectionManager::Instance();

Copilot · 2026-02-05T10:07:40Z

rpc/cli/dsFPD-com.cpp

+dsError_t dsGetFPBrightness(dsFPDIndicator_t eIndicator, dsFPDBrightness_t *pBrightness)
+{
+    if (pBrightness == NULL) {
+        fprintf(stderr, "[dsFPD-com] Invalid parameter: pBrightness is NULL\n");
+        return dsERR_INVALID_PARAM;
+    }
+
+    ConnectionManager* connMgr = ConnectionManager::Instance();
+    if (!connMgr || !connMgr->IsOperational()) {
+        return dsERR_GENERAL;
+    }
+
+    Exchange::IDeviceSettingsFPD* fpdInterface = connMgr->GetFPDInterface();
+    if (!fpdInterface) {
+        return dsERR_GENERAL;
+    }
+
+    Exchange::IDeviceSettingsFPD::FPDIndicator indicator = 
+        static_cast<Exchange::IDeviceSettingsFPD::FPDIndicator>(eIndicator);


Missing input validation for eIndicator parameter. The dsSetFPDBrightness function validates that eIndicator is less than dsFPD_INDICATOR_MAX (line 225), but dsGetFPBrightness does not perform this validation. For consistency and safety, the eIndicator parameter should be validated in all functions that use it, including getter functions, to prevent invalid enum values from being cast and passed to the interface.

Copilot · 2026-02-05T10:07:41Z

rpc/cli/dsHdmiIn-com.cpp

+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterConnectCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterSignalChangeCB(dsHdmiInSignalChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterSignalChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterStatusChangeCB(dsHdmiInStatusChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterStatusChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterVideoModeUpdateCB(dsHdmiInVideoModeUpdateCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterVideoModeUpdateCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAllmChangeCB(dsHdmiInAllmChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAllmChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAVLatencyChangeCB(dsAVLatencyChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAVLatencyChangeCB not supported in Thunder mode\n");
+    return dsERR_OPERATION_NOT_SUPPORTED;
+}
+
+dsError_t dsHdmiInRegisterAviContentTypeChangeCB(dsHdmiInAviContentTypeChangeCB_t CBFunc)
+{
+    fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAviContentTypeChangeCB not supported in Thunder mode\n");


Unused parameter warning suppression missing. The CBFunc parameter is not used in this function (it's a stub that returns OPERATION_NOT_SUPPORTED). To avoid compiler warnings, the parameter should either be marked with (void)CBFunc or use an attribute like [[maybe_unused]] or attribute((unused)).

Suggested change

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterConnectCB not supported in Thunder mode\n");

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterSignalChangeCB(dsHdmiInSignalChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterSignalChangeCB not supported in Thunder mode\n");

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterStatusChangeCB(dsHdmiInStatusChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterStatusChangeCB not supported in Thunder mode\n");

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterVideoModeUpdateCB(dsHdmiInVideoModeUpdateCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterVideoModeUpdateCB not supported in Thunder mode\n");

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterAllmChangeCB(dsHdmiInAllmChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAllmChangeCB not supported in Thunder mode\n");

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterAVLatencyChangeCB(dsAVLatencyChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAVLatencyChangeCB not supported in Thunder mode\n");

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterAviContentTypeChangeCB(dsHdmiInAviContentTypeChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAviContentTypeChangeCB not supported in Thunder mode\n");

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterConnectCB not supported in Thunder mode\n");

(void)CBFunc;

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterSignalChangeCB(dsHdmiInSignalChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterSignalChangeCB not supported in Thunder mode\n");

(void)CBFunc;

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterStatusChangeCB(dsHdmiInStatusChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterStatusChangeCB not supported in Thunder mode\n");

(void)CBFunc;

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterVideoModeUpdateCB(dsHdmiInVideoModeUpdateCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterVideoModeUpdateCB not supported in Thunder mode\n");

(void)CBFunc;

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterAllmChangeCB(dsHdmiInAllmChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAllmChangeCB not supported in Thunder mode\n");

(void)CBFunc;

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterAVLatencyChangeCB(dsAVLatencyChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAVLatencyChangeCB not supported in Thunder mode\n");

(void)CBFunc;

return dsERR_OPERATION_NOT_SUPPORTED;

}

dsError_t dsHdmiInRegisterAviContentTypeChangeCB(dsHdmiInAviContentTypeChangeCB_t CBFunc)

{

fprintf(stderr, "[dsHdmiIn-com] dsHdmiInRegisterAviContentTypeChangeCB not supported in Thunder mode\n");

(void)CBFunc;

Manimaran Renganathan added 3 commits January 27, 2026 18:23

RDKEMW-9659 Implement DeviceSettings client library for FPD in rpc/cli

3324427

Modified the dsFPD-com.cpp code based on latest DeviceSettings plugin…

b6e760c

… and Interface changes

Modified the rpc/cliMakefile with conditionally exclude dsFPD.c when …

fdc4105

…Thunder is enabled.

ManimaranRenganathan requested a review from a team as a code owner February 4, 2026 08:12

Copilot AI review requested due to automatic review settings February 4, 2026 08:12

Copilot started reviewing on behalf of ManimaranRenganathan February 4, 2026 08:12 View session

Added export USE_WPE_THUNDER_PLUGIN=y in cov_build.sh

034e5db

Copilot AI reviewed Feb 4, 2026

View reviewed changes

Manimaran Renganathan added 2 commits February 4, 2026 16:39

Resolved compilation errors

94475ec

Copilot AI review requested due to automatic review settings February 5, 2026 05:11

Copilot started reviewing on behalf of ManimaranRenganathan February 5, 2026 05:11 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

Manimaran Renganathan added 2 commits February 5, 2026 05:49

Added the Makefile and Makefile.am changes in devicesettings /rpc/cli

402f9bf

Resolved undefined reference error in dsHdmiIn-com.cpp file

70a09eb

Copilot AI review requested due to automatic review settings February 5, 2026 07:44

Copilot started reviewing on behalf of ManimaranRenganathan February 5, 2026 07:44 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

MadhusuthananRavi and others added 8 commits February 5, 2026 08:23

Revert "Resolved undefined reference error in dsHdmiIn-com.cpp file"

3944baf

This reverts commit 70a09eb.

Revert "Added the Makefile and Makefile.am changes in devicesettings …

7e25fcc

…/rpc/cli" This reverts commit 402f9bf.

Revert "Resolved compilation errors"

8c15bc0

This reverts commit 94475ec.

Revert "Modified the code"

59ccd08

This reverts commit d220f98.

Revert "Revert "Modified the code""

69ef6ab

This reverts commit 59ccd08.

Revert "Revert "Resolved compilation errors""

2823278

This reverts commit 8c15bc0.

Revert "Revert "Added the Makefile and Makefile.am changes in devices…

29e9c5e

…ettings /rpc/cli"" This reverts commit 7e25fcc.

Revert "Revert "Resolved undefined reference error in dsHdmiIn-com.cp…

13a8e6d

…p file"" This reverts commit 3944baf.

Copilot AI review requested due to automatic review settings February 5, 2026 09:59

Copilot started reviewing on behalf of ManimaranRenganathan February 5, 2026 09:59 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

-        _apiLock.Lock();
-        if (nullptr == _instance) {
-            _instance = new DeviceSettingsFPD();
-        }
-        _apiLock.Unlock();
+        // Fast path without locking
+        if (_instance != nullptr) {
+            return;
+        }
+        // Construct outside of _apiLock to avoid self-deadlock when the
+        // constructor calls Connect(), which also locks _apiLock.
+        DeviceSettingsFPD* newInstance = new DeviceSettingsFPD();
+        _apiLock.Lock();
+        if (_instance == nullptr) {
+            _instance = newInstance;
+            newInstance = nullptr;
+        }
+        _apiLock.Unlock();
+        // If another thread initialized _instance first, delete the extra instance.
+        if (newInstance != nullptr) {
+            delete newInstance;
+        }

-    // Note: Interface expects minutes and seconds, but API provides hour and minutes
-    // Converting: treating uMinutes as seconds for interface compatibility
-    uint32_t result = instance->SetFPDTime(timeFormat, uHour, uMinutes);
+    // The Thunder interface expects (minutes, seconds), while the DS API provides (hours, minutes).
+    // Convert DS (hour, minute) into total minutes for the Thunder "minutes" parameter and use 0 seconds.
+    const unsigned int thunderMinutes = (uHour * 60u) + uMinutes;
+    const unsigned int thunderSeconds = 0u;
+    uint32_t result = instance->SetFPDTime(timeFormat, thunderMinutes, thunderSeconds);

	LDLIBS := -lWPEFrameworkCore -lWPEFrameworkCOM
	LDLIBS := -lWPEFrameworkCore -lWPEFrameworkCOM -lIARMBus

		[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])
		AC_MSG_NOTICE([Thunder COM-RPC plugin support enabled])],

	if (eIndicator >= dsFPD_INDICATOR_MAX \|\| eBrightness > 100) {
	if (eIndicator >= dsFPD_INDICATOR_MAX \|\| eBrightness >= dsFPD_BRIGHTNESS_MAX) {

	uint32_t result = hdmiInInterface->GetHDMIInNumbefOfInputs(count);
	uint32_t result = hdmiInInterface->GetHDMIInNumberOfInputs(count);

	* Copyright 2016 RDK Management
	* Copyright 2025 RDK Management

		// Note: Interface expects minutes and seconds, but API provides hour and minutes
		// Converting: treating uMinutes as seconds for interface compatibility

-    // Thunder interface doesn't support persist flag - ignore it
+    // NOTE: Thunder IDeviceSettingsFPD interface does not support a persist flag.
+    // The toPersist parameter is kept for API compatibility with the IARM
+    // implementation but is not honored here: settings are always applied in
+    // the same way regardless of the flag, and persistence behavior (if any)
+    // is determined solely by the Thunder implementation.
+    if (!toPersist) {
+        fprintf(stderr,
+                "[dsFPD-com] Warning: persist flag is not supported in Thunder "
+                "implementation; applying FPD color without persistence control\n");
+    }

-    ConnectionManager::Lock();
-    uint32_t result = hdmiInInterface->GetEdidBytes(port, static_cast<uint16_t>(*length), edid);
-    ConnectionManager::Unlock();
-    return ConvertThunderError(result);
+    // Use a local variable for the in/out length parameter so that any
+    // updates made by GetEdidBytes are visible to the caller.
+    uint16_t edidLength = static_cast<uint16_t>(*length);
+    ConnectionManager::Lock();
+    uint32_t result = hdmiInInterface->GetEdidBytes(port, edidLength, edid);
+    ConnectionManager::Unlock();
+    dsError_t error = ConvertThunderError(result);
+    if (error == dsERR_NONE) {
+        *length = static_cast<int>(edidLength);
+    }
+    return error;

-    return _instance;
+    _dsConnectionManagerlock.Lock();
+    ConnectionManager* instance = _instance;
+    _dsConnectionManagerlock.Unlock();
+    return instance;

		#OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
		#OBJS += $(patsubst %.c,%.o,$(wildcard *.c))

		uint32_t result = hdmiInInterface->GetEdidBytes(port, static_cast<uint16_t>(*length), edid);
		ConnectionManager::Unlock();

	OBJS += $(patsubst %.c,%.o,$(filter-out dsFPD.c dsHdmiIn.c,$(wildcard *.c)))
	OBJS += $(patsubst %.c,%.o,$(filter-out dsFPD.c dsHdmiIn.c dsCompositeIn.c,$(wildcard *.c)))

Conversation

ManimaranRenganathan commented Feb 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 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 Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 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 Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026