Skip to content

Conversation

Prathyushakothuru
Copy link
Contributor

No description provided.

@Prathyushakothuru Prathyushakothuru requested a review from a team as a code owner September 9, 2025 04:19
@skamath skamath requested a review from Copilot September 9, 2025 05:43
Copy link

@Copilot 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 updates the AVInput plugin to replace IARM-based event handling with the libds client library notification system. This modernizes the event handling architecture to use direct device library callbacks instead of bus-based events.

  • Replaces IARM event registration with device library interface implementation
  • Implements callback methods for HDMI and Composite input events
  • Migrates from InitializeIARM/DeinitializeIARM to device::Manager initialization

Reviewed Changes

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

File Description
AVInput/AVInput.h Adds inheritance from device event interfaces and declares new callback methods
AVInput/AVInput.cpp Replaces IARM initialization with device manager and implements libds callbacks
.github/workflows/tests-trigger.yml Adds feature branch to CI workflow triggers

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

Copy link

@rababu2 rababu2 left a comment

Choose a reason for hiding this comment

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

Check if the additional parameter is required or not.

@Prathyushakothuru
Copy link
Contributor Author

Check if the additional parameter is required or not.

That is legacy code, which was already exist , so I added the same

void OnCompositeInHotPlug(dsCompositeInPort_t port, bool isConnected) override;
void OnCompositeInSignalStatus(dsCompositeInPort_t port, dsCompInSignalStatus_t signalStatus) override;
void OnCompositeInStatus(dsCompositeInPort_t activePort, bool isPresented) override;
void OnCompositeInVideoModeUpdate(dsCompositeInPort_t activePort, dsVideoPortResolution_t videoResolution) ;

Choose a reason for hiding this comment

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

still override is missing for OnHdmiInEventStatus and OnCompositeInVideoModeUpdate. pls correct it.

LOGINFO("device::Manager::Initialize success");
if (!_registeredDsEventHandlers) {
_registeredDsEventHandlers = true;
device::Host::getInstance().Register(baseInterface<device::Host::IHdmiInEvents>(), "WPE::HdmiInEvents");
Copy link

@karuppaiyak karuppaiyak Sep 18, 2025

Choose a reason for hiding this comment

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

this should be client name for example WPE::AVInputHdmi and WPE::AVInputComp for both hdmi and composite or shorter version of this.

{
LOGINFO("device::Manager::Initialize failed");
LOG_DEVICE_EXCEPTION0();
return std::string("AVInput: Initialization failed");

Choose a reason for hiding this comment

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

return will fail the plugin initialization. i think as per the latest discussion, we sshould continue plugin init though device::Manager::Initialize() fails. you can discuss with dixit for double confirmation if u want.

try
{
device::Manager::DeInitialize();
LOGINFO("device::Manager::DeInitialize success");

Choose a reason for hiding this comment

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

correct the try-catch block alignment wrt other places where it is already used.

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