NO-MERGE: Pahearn test 20260217a by pahearn73 · Pull Request #377 · rdkcentral/entservices-inputoutput

pahearn73 · 2026-02-17T19:36:30Z

No description provided.

Reason for change: add unit tests to increase coverage for HdmiCecSink Test Procedure: see Jira ticket Risks: None Priority: P2 Signed-off-by: melhar098_comcast <Mahmoud_El-haron@comcast.com>

Copilot

Pull request overview

This PR adds extensive L1 test coverage for the HdmiCecSink plugin and enables verbose build output in the CI/CD workflow. The changes focus on testing internal implementation details including event handlers, device management, and physical address routing.

Changes:

Added 953 lines of new test cases covering event handlers (onPresentationLanguageChanged, onPowerModeChanged), device removal scenarios, device chain management (addChild/removeChild/getRoute), and direct HdmiPortMap method testing
Added -v (verbose) flag to all cmake build commands in the L1-tests.yml workflow for better build debugging output
Added two helper functions (CreateCecSettingsFile, CreateCecSettingsFileNoParams) that are currently unused

Reviewed changes

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

File	Description
Tests/L1Tests/tests/test_HdmiCecSink.cpp	Adds extensive L1 test coverage including helper functions, event handler tests, device management tests, and direct access tests to internal HdmiPortMap implementation
.github/workflows/L1-tests.yml	Enables verbose cmake build output by adding `-v` flag to all cmake build commands

Copilot · 2026-02-17T19:42:42Z

+TEST_F(HdmiCecSinkInitializedEventDsTest, onPresentationLanguageChanged_ValidLanguage)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    // Trigger the onPresentationLanguageChanged callback with a valid language
+    Plugin::HdmiCecSinkImplementation::_instance->onPresentationLanguageChanged("en-US");
+}


This test directly accesses internal implementation state (Plugin::HdmiCecSinkImplementation::_instance->onPresentationLanguageChanged) and validates that no exceptions are thrown, but does not verify that the expected behavior actually occurred (e.g., CEC messages were sent with the correct language). The test should verify the side effects of the language change to ensure the functionality is working correctly.

Copilot · 2026-02-17T19:42:42Z

+TEST_F(HdmiCecSinkInitializedEventDsTest, onPowerModeChanged_StandbyToOn)
+{
+    // Transition from STANDBY to ON
+    Plugin::HdmiCecSinkImplementation::_instance->onPowerModeChanged(
+        WPEFramework::Exchange::IPowerManager::POWER_STATE_STANDBY,
+        WPEFramework::Exchange::IPowerManager::POWER_STATE_ON
+    );
+}


This test directly calls internal notification handlers (onPowerModeChanged) but does not verify the expected behavior or side effects. The test should verify that the power mode change triggers the correct CEC messages or state changes, not just that the method doesn't throw an exception.

Copilot · 2026-02-17T19:42:43Z

+TEST_F(HdmiCecSinkFrameProcessingTest, SetOSDName_WithRequestRetry)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // Add device first
+    uint8_t reportPAFrame[] = { 0x4F, 0x84, 0x10, 0x00, 0x04 }; // LA=4
+    EXPECT_NO_THROW(InjectCECFrame(reportPAFrame, sizeof(reportPAFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set up retry state: m_isRequestRetry > 0 and m_isRequested == REQUEST_OSD_NAME
+    Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequestRetry = 2;
+    Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequested = Plugin::CECDeviceParams::REQUEST_OSD_NAME;
+
+    // Send SetOSDName frame - should reset retry count to 0
+    // From Playback (LA=4) to TV (LA=0) - 0x40 header, opcode 0x47 (Set OSD Name)
+    uint8_t setOSDNameFrame[] = { 0x40, 0x47, 'T', 'e', 's', 't' };
+    EXPECT_NO_THROW(InjectCECFrame(setOSDNameFrame, sizeof(setOSDNameFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Verify retry count was reset
+    EXPECT_EQ(Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequestRetry, 0);
+}


This test directly modifies internal implementation state (m_isRequestRetry and m_isRequested) which violates encapsulation and creates tight coupling between tests and implementation details. This makes the tests brittle and difficult to maintain. Consider testing through the public API or use proper test hooks/accessors instead of directly manipulating internal state.

Copilot · 2026-02-17T19:42:43Z

+// Test fixture description: ReportPhysicalAddress with request retry logic - covers lines 352-353
+TEST_F(HdmiCecSinkFrameProcessingTest, ReportPhysicalAddress_WithRequestRetry)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // Add device first
+    uint8_t vendorIDFrame[] = { 0x4F, 0x87, 0x00, 0x00, 0x00 }; // DeviceVendorID to add device
+    EXPECT_NO_THROW(InjectCECFrame(vendorIDFrame, sizeof(vendorIDFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set up retry state: m_isRequestRetry > 0 and m_isRequested == REQUEST_PHISICAL_ADDRESS
+    Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequestRetry = 3;
+    Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequested = Plugin::CECDeviceParams::REQUEST_PHISICAL_ADDRESS;
+
+    // Send ReportPhysicalAddress frame - should reset retry count to 0
+    uint8_t reportPAFrame[] = { 0x4F, 0x84, 0x10, 0x00, 0x04 }; // LA=4, PA=1.0.0.0
+    EXPECT_NO_THROW(InjectCECFrame(reportPAFrame, sizeof(reportPAFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Verify retry count was reset
+    EXPECT_EQ(Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequestRetry, 0);
+}


This test directly modifies internal implementation state (m_isRequestRetry and m_isRequested) which violates encapsulation. The same concern applies as with the SetOSDName_WithRequestRetry test - consider testing through the public API or proper test hooks instead of directly manipulating internal state.

Suggested change

// Test fixture description: ReportPhysicalAddress with request retry logic - covers lines 352-353

TEST_F(HdmiCecSinkFrameProcessingTest, ReportPhysicalAddress_WithRequestRetry)

{

EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))

.WillRepeatedly(::testing::Return());

std::this_thread::sleep_for(std::chrono::milliseconds(100));

// Add device first

uint8_t vendorIDFrame[] = { 0x4F, 0x87, 0x00, 0x00, 0x00 }; // DeviceVendorID to add device

EXPECT_NO_THROW(InjectCECFrame(vendorIDFrame, sizeof(vendorIDFrame)));

std::this_thread::sleep_for(std::chrono::milliseconds(50));

// Set up retry state: m_isRequestRetry > 0 and m_isRequested == REQUEST_PHISICAL_ADDRESS

Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequestRetry = 3;

Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequested = Plugin::CECDeviceParams::REQUEST_PHISICAL_ADDRESS;

// Send ReportPhysicalAddress frame - should reset retry count to 0

uint8_t reportPAFrame[] = { 0x4F, 0x84, 0x10, 0x00, 0x04 }; // LA=4, PA=1.0.0.0

EXPECT_NO_THROW(InjectCECFrame(reportPAFrame, sizeof(reportPAFrame)));

std::this_thread::sleep_for(std::chrono::milliseconds(50));

// Verify retry count was reset

EXPECT_EQ(Plugin::HdmiCecSinkImplementation::_instance->deviceList[4].m_isRequestRetry, 0);

}

// NOTE: A previous ReportPhysicalAddress_WithRequestRetry test directly modified

// internal HdmiCecSinkImplementation state (m_isRequestRetry / m_isRequested),

// which violates encapsulation and was flagged by static analysis. Any future

// retry-behavior tests should exercise this logic via public APIs or explicit

// test hooks rather than accessing implementation internals.

Copilot · 2026-02-17T19:42:43Z

+    Plugin::HdmiCecSinkImplementation::_instance->onPresentationLanguageChanged("en-US");
+}


The tests access Plugin::HdmiCecSinkImplementation::_instance which is a static singleton pointer, but there's no null check before dereferencing it in any of these tests. If the instance is not properly initialized by the test fixture setup, these tests will crash with a segmentation fault. Add null checks or assertions to verify _instance is valid before accessing it, or ensure the test fixture properly initializes it.

Copilot · 2026-02-17T19:42:44Z

+TEST_F(HdmiCecSinkFrameProcessingTest, removeDevice_ViaHotplugDisconnect_NonAudioDevice)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    // First, simulate a device connection by sending ReportPhysicalAddress
+    // This adds a device to deviceList with logical address 4 (Playback device)
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+


Using fixed-duration sleeps (std::this_thread::sleep_for) throughout these tests creates potential timing issues and makes tests unreliable in different execution environments. If the system is under load or slow, these delays may not be sufficient, leading to flaky tests. Consider using synchronization primitives, mock callbacks, or condition variables to ensure proper sequencing instead of relying on arbitrary time delays.

Copilot · 2026-02-17T19:42:44Z

+TEST_F(HdmiCecSinkInitializedEventDsTest, onPresentationLanguageChanged_ValidLanguage)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    // Trigger the onPresentationLanguageChanged callback with a valid language
+    Plugin::HdmiCecSinkImplementation::_instance->onPresentationLanguageChanged("en-US");
+}


The test name "onPresentationLanguageChanged_ValidLanguage" does not follow a clear Given-When-Then pattern or clearly describe what is being tested. The test comment says "tests UserSettings notification" but the test doesn't verify any notification was sent or received. Consider renaming to something like "onPresentationLanguageChanged_ValidLanguage_SendsCECMessage" and add assertions to verify the expected behavior.

Copilot · 2026-02-17T19:42:45Z

+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_AddChild_SingleLevelPhysicalAddress)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_0, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with PA=1.1.0.0 (single level depth - byte[1] != 0, byte[2,3] == 0)
+    // This tests the addChild branch for physical_addr.getByteValue(1) != 0
+    uint8_t device1[] = { 0x4F, 0x84, 0x11, 0x00, 0x04 }; // LA=4, PA=1.1.0.0
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set this device as active source to exercise getRoute
+    uint8_t activeSourceFrame[] = { 0x4F, 0x82, 0x11, 0x00 }; // LA=4, PA=1.1.0.0
+    EXPECT_NO_THROW(InjectCECFrame(activeSourceFrame, sizeof(activeSourceFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Call getActiveRoute which internally calls getRoute on HdmiPortMap
+    string response;
+    EXPECT_EQ(Core::ERROR_NONE, handler.Invoke(connection, _T("getActiveRoute"), _T("{}"), response));
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_AddChild_TwoLevelPhysicalAddress)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_1, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with PA=2.1.1.0 (two level depth - byte[2] != 0, byte[3] == 0)
+    // This tests the addChild branch for physical_addr.getByteValue(2) != 0
+    uint8_t device1[] = { 0x3F, 0x84, 0x21, 0x10, 0x04 }; // LA=3, PA=2.1.1.0
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set as active source
+    uint8_t activeSourceFrame[] = { 0x3F, 0x82, 0x21, 0x10 }; // LA=3, PA=2.1.1.0
+    EXPECT_NO_THROW(InjectCECFrame(activeSourceFrame, sizeof(activeSourceFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Call getActiveRoute to exercise getRoute with 2-level chain
+    string response;
+    EXPECT_EQ(Core::ERROR_NONE, handler.Invoke(connection, _T("getActiveRoute"), _T("{}"), response));
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_AddChild_ThreeLevelPhysicalAddress)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_0, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with PA=1.2.3.1 (three level depth - byte[3] != 0)
+    // This tests the addChild branch for physical_addr.getByteValue(3) != 0
+    uint8_t device1[] = { 0x4F, 0x84, 0x12, 0x31, 0x04 }; // LA=4, PA=1.2.3.1
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set as active source
+    uint8_t activeSourceFrame[] = { 0x4F, 0x82, 0x12, 0x31 }; // LA=4, PA=1.2.3.1
+    EXPECT_NO_THROW(InjectCECFrame(activeSourceFrame, sizeof(activeSourceFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Call getActiveRoute to exercise getRoute with 3-level chain
+    string response;
+    EXPECT_EQ(Core::ERROR_NONE, handler.Invoke(connection, _T("getActiveRoute"), _T("{}"), response));
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_AddChild_DirectPortConnection)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_1, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device connected directly to port (PA=2.0.0.0)
+    // This tests the else-if branch: physical_addr == m_physicalAddr
+    uint8_t device1[] = { 0x4F, 0x84, 0x20, 0x00, 0x04 }; // LA=4, PA=2.0.0.0
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set as active source
+    uint8_t activeSourceFrame[] = { 0x4F, 0x82, 0x20, 0x00 }; // LA=4, PA=2.0.0.0
+    EXPECT_NO_THROW(InjectCECFrame(activeSourceFrame, sizeof(activeSourceFrame)));
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_RemoveChild_SingleLevelPhysicalAddress)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_0, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with PA=1.1.0.0 first
+    uint8_t device1[] = { 0x4F, 0x84, 0x11, 0x00, 0x04 }; // LA=4, PA=1.1.0.0
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Remove device - tests removeChild branch for byte[1] != 0, byte[2,3] == 0
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_0, false);
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_RemoveChild_TwoLevelPhysicalAddress)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_1, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with PA=2.1.1.0 first
+    uint8_t device1[] = { 0x3F, 0x84, 0x21, 0x10, 0x04 }; // LA=3, PA=2.1.1.0
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Remove device - tests removeChild branch for byte[2] != 0, byte[3] == 0
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_1, false);
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_RemoveChild_ThreeLevelPhysicalAddress)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_0, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with PA=1.2.3.1 first
+    uint8_t device1[] = { 0x4F, 0x84, 0x12, 0x31, 0x04 }; // LA=4, PA=1.2.3.1
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Remove device - tests removeChild branch for byte[3] != 0
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_0, false);
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_GetRoute_NonMatchingPort)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection (use port that won't match PA=3.0.0.0)
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_0, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with PA=3.0.0.0 (port 3 - assumes only 2 ports configured)
+    // This tests the else branch in getRoute where physical address doesn't match port
+    uint8_t device1[] = { 0x4F, 0x84, 0x30, 0x00, 0x04 }; // LA=4, PA=3.0.0.0
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set as active source
+    uint8_t activeSourceFrame[] = { 0x4F, 0x82, 0x30, 0x00 }; // LA=4, PA=3.0.0.0
+    EXPECT_NO_THROW(InjectCECFrame(activeSourceFrame, sizeof(activeSourceFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Call getActiveRoute - should hit the else branch in getRoute
+    string response;
+    EXPECT_EQ(Core::ERROR_NONE, handler.Invoke(connection, _T("getActiveRoute"), _T("{}"), response));
+}
+
+TEST_F(HdmiCecSinkFrameProcessingTest, DeviceChain_GetRoute_AllDepthLevels)
+{
+    EXPECT_CALL(*p_connectionImplMock, sendTo(::testing::_, ::testing::_, ::testing::_))
+        .WillRepeatedly(::testing::Return());
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // First establish HDMI port connection
+    Plugin::HdmiCecSinkImplementation::_instance->OnHdmiInEventHotPlug(dsHDMI_IN_PORT_1, true);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Add device with all depth levels (PA=2.3.2.1)
+    // Tests getRoute pushing all three device chain levels plus root
+    uint8_t device1[] = { 0x4F, 0x84, 0x23, 0x21, 0x04 }; // LA=4, PA=2.3.2.1
+    EXPECT_NO_THROW(InjectCECFrame(device1, sizeof(device1)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Set as active source
+    uint8_t activeSourceFrame[] = { 0x4F, 0x82, 0x23, 0x21 }; // LA=4, PA=2.3.2.1
+    EXPECT_NO_THROW(InjectCECFrame(activeSourceFrame, sizeof(activeSourceFrame)));
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // Call getActiveRoute - exercises all push_back operations in getRoute
+    string response;
+    EXPECT_EQ(Core::ERROR_NONE, handler.Invoke(connection, _T("getActiveRoute"), _T("{}"), response));
+}


There is significant code duplication across multiple device chain management tests (lines 2898-3136). Each test follows nearly identical patterns: sleep, hotplug event, sleep, inject frame, sleep, inject active source, sleep, invoke API. Consider extracting common setup logic into helper methods to reduce duplication and improve maintainability. For example, create helpers like SetupDeviceAtPhysicalAddress() and SetActiveSource().

Copilot · 2026-02-17T19:42:45Z

+TEST_F(HdmiCecSinkFrameProcessingTest, HdmiPortMap_AddChild_TwoLevel_Direct)
+{
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    LogicalAddress testLA(4);
+    PhysicalAddress portPA(2, 0, 0, 0); // Port 1 base address
+    PhysicalAddress devicePA(2, 1, 2, 0); // Two level depth
+
+    Plugin::HdmiCecSinkImplementation::_instance->hdmiInputs[1].update(testLA);
+    Plugin::HdmiCecSinkImplementation::_instance->hdmiInputs[1].addChild(LogicalAddress(6), devicePA);
+
+    // Verify device was added to level 1 chain (byte[2] = 2, so index 1)
+    EXPECT_EQ(Plugin::HdmiCecSinkImplementation::_instance->hdmiInputs[1].m_deviceChain[1].m_childsLogicalAddr[1], 6);


The test directly accesses array indices in deviceList and hdmiInputs without boundary checks. For example, accessing deviceList[4], deviceList[3], deviceList[5], hdmiInputs[0], hdmiInputs[1] without verifying these indices are valid. If the arrays are smaller than expected or not properly initialized, this will cause out-of-bounds access and undefined behavior. Add assertions to verify array sizes before accessing specific indices.

Copilot · 2026-02-17T19:42:45Z

+	static void CreateCecSettingsFile(const std::string& filePath, bool cecEnabled = true, bool cecOTPEnabled = true, const std::string& osdName = "TV Box", unsigned int vendorId = 0x0019FB)
+	{
+		Core::File file(filePath);
+
+		if (file.Exists()) {
+			file.Destroy();
+		}
+
+		file.Create();
+
+		JsonObject parameters;
+		parameters["cecEnabled"] = cecEnabled;
+		parameters["cecOTPEnabled"] = cecOTPEnabled;
+		parameters["cecOSDName"] = osdName;
+		parameters["cecVendorId"] = vendorId;
+
+		parameters.IElement::ToFile(file);
+		file.Close();
+	}


The function CreateCecSettingsFile creates and writes to a file, but does not check if the Create() operation succeeded before attempting to write to it. If the file creation fails silently, subsequent operations will fail without clear error messages. Consider adding error checking after the file.Create() call to verify the file was created successfully.

melhar098 and others added 2 commits February 12, 2026 17:58

RDKEMW-12716: Improve L1 test coverage for HdmiCecSink plugin

4a16feb

Reason for change: add unit tests to increase coverage for HdmiCecSink Test Procedure: see Jira ticket Risks: None Priority: P2 Signed-off-by: melhar098_comcast <Mahmoud_El-haron@comcast.com>

NO-MERGE: verbose logging

d44a6e2

pahearn73 requested a review from a team as a code owner February 17, 2026 19:36

Copilot AI review requested due to automatic review settings February 17, 2026 19:36

Copilot started reviewing on behalf of pahearn73 February 17, 2026 19:37 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

NO-MERGE: Pahearn test 20260217a#377

NO-MERGE: Pahearn test 20260217a#377
pahearn73 wants to merge 2 commits into
developfrom
pahearn_test_20260217a

pahearn73 commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Plugin::HdmiCecSinkImplementation::_instance->onPresentationLanguageChanged("en-US");
		}

Uh oh!

Conversation

pahearn73 commented Feb 17, 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 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants