RDKEMW-16000: Add refreshHandles in libds. by santoshcomcast · Pull Request #235 · rdkcentral/devicesettings

santoshcomcast · 2026-03-26T14:03:44Z

Reason for change: These refreshHandles() api calls when dsmgr restart.
Test Procedure: refer RDKEMW-16000
Risks: High
Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com

Reason for change: These refreshHandles() api calls when dsmgr restart. Test Procedure: refer RDKEMW-16000 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

Copilot

Pull request overview

This PR adds a DSMgr-restart recovery mechanism to libds by introducing “refresh handles” APIs for audio/video ports and video devices, plus an IARM event-driven auto-refresh path to clear stale intptr_t handles after DSMgr crashes/restarts. It also adds a large standalone sample client to reproduce and validate stale-handle behavior and recovery options.

Changes:

Add refreshHandle() / refreshAllHandles() APIs for AudioOutputPort, VideoOutputPort (incl. inner Display), and VideoDevice objects.
Register an IARM handler for IARM_BUS_DSMGR_EVENT_RESTARTED in Manager::Initialize() to auto-refresh handles asynchronously with retries (and a force re-init fallback).
Add sample/dsMenuClient.cpp as a menu-driven test app demonstrating stale-handle failures and recovery flows.

Reviewed changes

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

Show a summary per file

File	Description
sample/dsMenuClient.cpp	New menu-driven client to reproduce stale-handle issues and exercise refresh/reinit options.
rpc/include/dsMgr.h	Adds `IARM_BUS_DSMGR_EVENT_RESTARTED` event ID for restart notification.
ds/videoOutputPortConfig.hpp	Declares `VideoOutputPortConfig::refreshAllHandles()`.
ds/videoOutputPortConfig.cpp	Implements refresh loop calling `VideoOutputPort::refreshHandle()` for all ports.
ds/videoOutputPort.cpp	Adds `VideoOutputPort::refreshHandle()` to re-fetch port + display handles and refresh cached flags.
ds/videoDeviceConfig.hpp	Declares `VideoDeviceConfig::refreshAllHandles()`.
ds/videoDeviceConfig.cpp	Implements refresh loop calling `VideoDevice::refreshHandle()` for all devices.
ds/videoDevice.cpp	Adds `VideoDevice::refreshHandle()` to re-fetch device handle from DSMgr.
ds/manager.cpp	Registers/unregisters restart event handler; async retry thread refreshes all handles on DSMgr restart (with fallback reinit).
ds/include/videoOutputPort.hpp	Exposes `VideoOutputPort::refreshHandle()` in public header.
ds/include/videoDevice.hpp	Exposes `VideoDevice::refreshHandle()` in public header.
ds/include/hdmiIn.hpp	Adds missing `<string>` include.
ds/include/audioOutputPort.hpp	Exposes `AudioOutputPort::refreshHandle()` in public header.
ds/audioOutputPortConfig.hpp	Declares `AudioOutputPortConfig::refreshAllHandles()`.
ds/audioOutputPortConfig.cpp	Implements refresh loop calling `AudioOutputPort::refreshHandle()` for all ports.
ds/audioOutputPort.cpp	Adds `AudioOutputPort::refreshHandle()` to re-fetch handle and refresh cached port fields.

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

Copilot · 2026-03-26T14:12:58Z

ds/videoOutputPort.cpp

+    intptr_t old_handle = _handle;
+    dsError_t ret = dsGetVideoPort((dsVideoPortType_t)_type, _index, &_handle);
+    printf("\n[refreshHandle] VideoOutputPort type:%d index:%d old_handle:0x%lX new_handle:0x%lX ret:%d\n",
+           _type, _index, (long)old_handle, (long)_handle, ret);


This new refreshHandle() adds unconditional printf() logging to stdout (including pointer-like handles formatted via %lX). Since handle refresh is now triggered automatically from Manager on DSMgr restart, this can spam stdout in production and the formatting isn’t portable for intptr_t. Prefer routing through the existing INT_INFO/INT_ERROR logger (and use PRIxPTR/uintptr_t formatting for handles) or gate the logging behind a debug flag.

Copilot · 2026-03-26T14:12:59Z

ds/audioOutputPort.cpp

+    intptr_t old_handle = _handle;
+    dsError_t ret = dsGetAudioPort((dsAudioPortType_t)_type, _index, &_handle);
+
+    printf("\n[refreshHandle] AudioOutputPort type:%d index:%d "
+           "old_handle:0x%lX new_handle:0x%lX ret:%d\n",
+           _type, _index, (long)old_handle, (long)_handle, ret);
+


AudioOutputPort::refreshHandle() introduces unconditional printf()s to stdout and prints intptr_t handles via %lX casts. With the new automatic DSMgr-restart refresh, this can generate noisy output in normal operation and the formatting isn’t portable. Use INT_INFO/INT_ERROR (or similar project logging) and PRIxPTR/uintptr_t formatting for handle values, or gate this behind a debug switch.

Copilot · 2026-03-26T14:12:59Z

ds/videoDevice.cpp

+    intptr_t old_handle = _handle;
+    dsError_t ret = dsGetVideoDevice(_id, &_handle);
+
+    printf("\n[refreshHandle] VideoDevice id:%d "
+           "old_handle:0x%lX new_handle:0x%lX ret:%d\n",
+           _id, (long)old_handle, (long)_handle, ret);
+
+    if (ret != dsERR_NONE) {
+        printf("[refreshHandle] VideoDevice FAILED – dsmgr not running? (ret=%d)\n", ret);
+    }


VideoDevice::refreshHandle() adds unconditional printf() output to stdout and prints intptr_t via %lX casts. Since this method is now used by the automatic DSMgr-restart refresh path, this can create noisy output and isn’t portable formatting. Prefer INT_INFO/INT_ERROR with PRIxPTR/uintptr_t formatting (or guard the logging behind a debug flag).

Copilot · 2026-03-26T14:13:00Z

ds/manager.cpp

+            /* Register the dsmgr-restart handler so that libds handles are
+             * refreshed automatically whenever dsmgr crashes and restarts.
+             * Registered here (after all HAL inits) so handles are valid
+             * before the first event can arrive.  Unregistered in
+             * DeInitialize() to match the Manager lifetime exactly. */
+            IARM_Result_t iarmRet = IARM_Bus_RegisterEventHandler(
+                IARM_BUS_DSMGR_NAME,
+                IARM_BUS_DSMGR_EVENT_RESTARTED,
+                dsMgrRestartedHandler);
+            if (IARM_RESULT_SUCCESS != iarmRet) {
+                INT_ERROR("[Manager] Failed to register IARM_BUS_DSMGR_EVENT_RESTARTED handler (ret=%d)",
+                          iarmRet);
+            } else {
+                INT_INFO("[Manager] IARM_BUS_DSMGR_EVENT_RESTARTED handler registered OK");
+            }


This change adds new, high-risk behavior (auto handle refresh + retry thread + last-resort DeInitialize/Initialize) but there’s no corresponding unit/integration coverage. Since the repo already has Boost-based tests for Audio/Video config and RPC clients, please add tests that exercise refreshAllHandles()/refreshHandle() (and ideally the restart-handler path) using the existing IARM stubs, so regressions in restart recovery are caught automatically.

Copilot · 2026-03-26T14:13:00Z

ds/audioOutputPortConfig.hpp

 	void load(audioConfigs_t* dynamicAudioConfigs);
 	void release();
+        dsError_t refreshAllHandles();  /*!< Re-fetch _handle for every port in _aPorts[].
+                                         *   Mirrors the load() port loop but calls
+                                         *   AudioOutputPort::refreshHandle() on each
+                                         *   existing object instead of constructing new ones.
+                                         *   Call after dsmgr crash+restart to clear stale
+                                         *   handles for ALL audio ports in one shot. */



AudioOutputPortConfig::refreshAllHandles() is declared to return dsError_t, but this header doesn’t include dsError.h (or otherwise guarantee dsError_t is visible). This can break compilation for translation units that include audioOutputPortConfig.hpp without including dsError.h first (e.g. ds/manager.cpp). Add the proper include (or change the return type to something already declared here).

Copilot · 2026-03-26T14:13:00Z

sample/dsMenuClient.cpp

+            /* Quick liveness check */
+            bool connected = false;
+            dsError_t chk = dsIsDisplayConnected(nh, &connected);


dsIsDisplayConnected() is a video-port RPC API (takes a video-port handle), but here it’s called with nh returned from dsGetDisplay() (a display handle). This will return incorrect results or fail (often dsERR_INVALID_PARAM). Use dsIsDisplayConnected(g_hdl_vport, ...) (or perform a display-level liveness check using a dsDisplay API such as EDID, if the intent is to validate the display handle).

Suggested change

/* Quick liveness check */

bool connected = false;

dsError_t chk = dsIsDisplayConnected(nh, &connected);

/* Quick liveness check (use HDMI0 VideoPort handle as required by dsIsDisplayConnected) */

bool connected = false;

dsError_t chk = dsIsDisplayConnected(g_hdl_vport, &connected);

Copilot · 2026-03-26T14:13:01Z

sample/dsMenuClient.cpp

+        dsError_t r2 = dsIsDisplayConnected(g_hdl_display, &conn);
+        printf("  dsIsDisplayConnected(g_hdl_display): %s (ret=%d)\n",


dsIsDisplayConnected() expects a video-port handle, but g_hdl_display is populated via dsGetDisplay() (display handle). This verification call should use the video-port handle (e.g. g_hdl_vport) or switch to a dsDisplay API that takes a display handle.

Suggested change

dsError_t r2 = dsIsDisplayConnected(g_hdl_display, &conn);

printf(" dsIsDisplayConnected(g_hdl_display): %s (ret=%d)\n",

dsError_t r2 = dsIsDisplayConnected(g_hdl_vport, &conn);

printf(" dsIsDisplayConnected(g_hdl_vport) : %s (ret=%d)\n",

santoshcomcast added 4 commits March 25, 2026 07:53

RDKEMW-16000: Add refreshHandles in libds.

279e97c

Reason for change: These refreshHandles() api calls when dsmgr restart. Test Procedure: refer RDKEMW-16000 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>

update missing logic

d1b0449

Fix build issue.

55d4d3a

adding retry in client side

a8007ad

santoshcomcast requested a review from a team as a code owner March 26, 2026 14:03

Copilot AI review requested due to automatic review settings March 26, 2026 14:03

Copilot started reviewing on behalf of santoshcomcast March 26, 2026 14:04 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RDKEMW-16000: Add refreshHandles in libds.#235

RDKEMW-16000: Add refreshHandles in libds.#235
santoshcomcast wants to merge 4 commits intodevelopfrom
topic/RDKEMW-16000-refresh-handels

santoshcomcast commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		dsError_t r2 = dsIsDisplayConnected(g_hdl_display, &conn);
		printf(" dsIsDisplayConnected(g_hdl_display): %s (ret=%d)\n",

Conversation

santoshcomcast commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants