RDKEMW-9895: IAppNotifications: move Emit to callback #592

adrianM27 · 2025-10-31T10:31:01Z

No description provided.

Copilot

Pull Request Overview

This PR refactors the event emission mechanism in the AppNotifications API by extracting the Emit method into a separate callback interface. The changes enable a callback-based approach where event emitters are passed to handlers rather than being directly invoked on the main interface.

Key changes:

Extracted Emit method into a new nested IEmiter callback interface
Modified HandleAppEventNotifier to accept an emitter callback parameter
Changed Emit return type from Core::hresult to void

Reviewed Changes

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

File	Description
apis/Ids.h	Added new ID constant for the emitter callback interface
apis/AppNotifications/IAppNotifications.h	Refactored event emission into a callback interface and updated handler signature

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

Copilot · 2025-10-31T10:32:39Z

apis/Ids.h

        ID_APP_NOTIFICATIONS                         = ID_ENTOS_OFFSET + 0x450,
-	ID_APP_NOTIFICATIONS_HANDLER_INTERNAL        = ID_APP_NOTIFICATIONS + 1
+	ID_APP_NOTIFICATIONS_HANDLER_INTERNAL        = ID_APP_NOTIFICATIONS + 1,
+	ID_APP_NOTIFICATIONS_HANDLER_INTERNAL_EMITER    = ID_APP_NOTIFICATIONS + 2


Corrected spelling of 'EMITER' to 'EMITTER'.

Suggested change

ID_APP_NOTIFICATIONS_HANDLER_INTERNAL_EMITER = ID_APP_NOTIFICATIONS + 2

ID_APP_NOTIFICATIONS_HANDLER_INTERNAL_EMITTER = ID_APP_NOTIFICATIONS + 2

Copilot · 2025-10-31T10:32:40Z

apis/AppNotifications/IAppNotifications.h

            };

+
+            struct EXTERNAL IEmiter : virtual public Core::IUnknown


Corrected spelling of 'IEmiter' to 'IEmitter'.

Copilot · 2025-10-31T10:32:40Z

apis/AppNotifications/IAppNotifications.h

+
+            struct EXTERNAL IEmiter : virtual public Core::IUnknown
+            {
+                enum { ID = ID_APP_NOTIFICATIONS_HANDLER_INTERNAL_EMITER };


Corrected spelling of 'EMITER' to 'EMITTER' to match the corrected constant name.

Copilot · 2025-10-31T10:32:40Z

apis/AppNotifications/IAppNotifications.h

+
            // @json:omit
            // @text handleAppEventNotifier
            // @brief Handle AppEvent Notfier expectations for a given event


Corrected spelling of 'Notfier' to 'Notifier'.

Suggested change

// @brief Handle AppEvent Notfier expectations for a given event

// @brief Handle AppEvent Notifier expectations for a given event

Copilot · 2025-10-31T10:32:41Z

apis/AppNotifications/IAppNotifications.h

            // @param status: status to be filled in
            // @returns Core::hresult
-            virtual Core::hresult HandleAppEventNotifier(const string& event /* @in */, const bool& listen /* @in */, bool& status /* @out */) = 0;
+            virtual Core::hresult HandleAppEventNotifier(IEmiter *emitCb, const string& event /* @in */, const bool& listen /* @in */, bool& status /* @out */) = 0;


Corrected spelling of 'IEmiter' to 'IEmitter' to match the corrected interface name.

Copilot

Pull Request Overview

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

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

Copilot · 2025-10-31T10:59:59Z

apis/AppNotifications/IAppNotifications.h

+                // @param event: the event to emit
+                // @param payload: the payload to emit
+                // @param appId (optional): the appId to emit the event for, if empty the event is emitted for all Apps
+                virtual void Emit(const string &event /* @in */,


The return type changed from Core::hresult to void, removing the ability to report errors during emission. Consider keeping Core::hresult as the return type to allow error handling and propagation when emission fails.

Suggested change

virtual void Emit(const string &event /* @in */,

virtual Core::hresult Emit(const string &event /* @in */,

For notification we do not return a value

Copilot · 2025-10-31T10:59:59Z

apis/AppNotifications/IAppNotifications.h

            // @param status: status to be filled in
            // @returns Core::hresult
-            virtual Core::hresult HandleAppEventNotifier(const string& event /* @in */, const bool& listen /* @in */, bool& status /* @out */) = 0;
+            virtual Core::hresult HandleAppEventNotifier(IEmitter *emitCb, const string& event /* @in */, bool listen /* @in */, bool& status /* @out */) = 0;


The emitCb parameter should be a reference (IEmitter&) or a smart pointer rather than a raw pointer to clarify ownership semantics and prevent null pointer issues. If null is a valid value, this should be documented in the @param comment.

Thats a THunder way of passing pointers

…plugis_fixes

Copilot

Pull Request Overview

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

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

Copilot · 2025-11-03T14:13:54Z

apis/AppNotifications/IAppNotifications.h

            // @param status: status to be filled in
            // @returns Core::hresult
-            virtual Core::hresult HandleAppEventNotifier(const string& event , const bool& listen , bool& status /* @out */) = 0;
+            virtual Core::hresult HandleAppEventNotifier(IEmitter *emitCb, const string& event /* @in */, bool listen /* @in */, bool& status /* @out */) = 0;


The emitCb parameter should be marked with /* @in */ to be consistent with other parameters and clarify it's an input parameter. Additionally, consider whether this should be a non-nullable pointer or if it should use a reference instead.

Suggested change

virtual Core::hresult HandleAppEventNotifier(IEmitter *emitCb, const string& event /* @in */, bool listen /* @in */, bool& status /* @out */) = 0;

virtual Core::hresult HandleAppEventNotifier(IEmitter *emitCb /* @in */, const string& event /* @in */, bool listen /* @in */, bool& status /* @out */) = 0;

Copilot · 2025-11-03T14:13:54Z

apis/AppNotifications/IAppNotifications.h

            // @param status: status to be filled in
            // @returns Core::hresult
-            virtual Core::hresult HandleAppEventNotifier(const string& event , const bool& listen , bool& status /* @out */) = 0;
+            virtual Core::hresult HandleAppEventNotifier(IEmitter *emitCb, const string& event /* @in */, bool listen /* @in */, bool& status /* @out */) = 0;


The parameter listen is changed from const bool& to bool. For consistency with the existing codebase style where the original signature used const bool&, consider whether this change in parameter passing style is intentional. If this is a breaking API change, ensure it's documented.

Suggested change

virtual Core::hresult HandleAppEventNotifier(IEmitter *emitCb, const string& event /* @in */, bool listen /* @in */, bool& status /* @out */) = 0;

virtual Core::hresult HandleAppEventNotifier(IEmitter *emitCb, const string& event /* @in */, const bool& listen /* @in */, bool& status /* @out */) = 0;

Copilot AI review requested due to automatic review settings October 31, 2025 10:31

adrianM27 requested a review from a team as a code owner October 31, 2025 10:31

Copilot AI reviewed Oct 31, 2025

View reviewed changes

adrianM27 force-pushed the feature/RDKEMW-9458_Fb_claud_plugis_fixes branch from 6aa91d5 to 7517319 Compare October 31, 2025 10:51

adrianM27 requested a review from Copilot October 31, 2025 10:58

Copilot AI reviewed Oct 31, 2025

View reviewed changes

RDKEMW-9895: IAppNotifications: move Emit to callback

686890c

adrianM27 force-pushed the feature/RDKEMW-9458_Fb_claud_plugis_fixes branch from 7517319 to 686890c Compare November 3, 2025 08:51

Merge branch 'feature/RDKEMW-9458' into feature/RDKEMW-9458_Fb_claud_…

3a06e7a

…plugis_fixes

adrianM27 requested a review from Copilot November 3, 2025 14:12

Copilot AI reviewed Nov 3, 2025

View reviewed changes

	ID_APP_NOTIFICATIONS_HANDLER_INTERNAL_EMITER = ID_APP_NOTIFICATIONS + 2
	ID_APP_NOTIFICATIONS_HANDLER_INTERNAL_EMITTER = ID_APP_NOTIFICATIONS + 2

	// @brief Handle AppEvent Notfier expectations for a given event
	// @brief Handle AppEvent Notifier expectations for a given event

	virtual void Emit(const string &event /* @in */,
	virtual Core::hresult Emit(const string &event /* @in */,

	virtual Core::hresult HandleAppEventNotifier(IEmitter emitCb, const string& event / @in /, bool listen / @in /, bool& status / @out */) = 0;
	virtual Core::hresult HandleAppEventNotifier(IEmitter emitCb / @in /, const string& event / @in /, bool listen / @in /, bool& status / @out */) = 0;

RDKEMW-9895: IAppNotifications: move Emit to callback #592

Are you sure you want to change the base?

RDKEMW-9895: IAppNotifications: move Emit to callback #592

Uh oh!

Conversation

adrianM27 commented Oct 31, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

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 Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

adrianM27 Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

adrianM27 Oct 31, 2025

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 Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants