Skip to content

Conversation

@Dosakaya
Copy link
Contributor

@Dosakaya Dosakaya commented Aug 25, 2025

Reason for change: Update Display Settings with libds client library.
Test Procedure: test all end-to-end tests for all new events

Risks: NO
Priority: P1

@Dosakaya Dosakaya requested a review from a team as a code owner August 25, 2025 08:58
@CLAassistant
Copy link

CLAassistant commented Aug 25, 2025

CLA assistant check
All committers have signed the CLA.

@skamath skamath requested a review from Copilot September 3, 2025 08:51

This comment was marked as outdated.

Copy link
Contributor

@skamath skamath left a comment

Choose a reason for hiding this comment

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

A common observation, in all event notification callbacks, I see there is a check for being valid DisplaySettings::_instance. We should really try and understand what use case will we hit the issue, and please document the usecase.

Copy link
Contributor

@skamath skamath left a comment

Choose a reason for hiding this comment

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

I don't see deletion of old static methods (originally receiving notification over IARM).
Also make sure dsMgr.h and other unused headers are no more included.

@skamath skamath requested a review from Copilot September 5, 2025 11:29
Copy link
Contributor

Copilot AI left a comment

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 migrates the DisplaySettings plugin from IARM-based event handling to a device library-based event handling system. The change replaces legacy IARM event registration and handlers with modern C++ interface implementations for better type safety and maintainability.

Key changes:

  • Replaces IARM event handlers with device library interface implementations
  • Adds multiple interface inheritance for handling different types of display and audio events
  • Refactors initialization and deinitialization logic to use device::Manager instead of IARM

Reviewed Changes

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

File Description
DisplaySettings.h Adds interface inheritance and declares new event handler methods while removing IARM function declarations
DisplaySettings.cpp Implements new device library event handlers and removes IARM-based event handling code

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Dosakaya Dosakaya changed the title Feature/rdkemw 6167 pr RDKEMW-6167: Update Display Settings with libds client library Sep 13, 2025
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