Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adrianM27 wants to merge 2 commits into
base: feature/RDKEMW-9458
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
686890c
RDKEMW-9895: IAppNotifications: move Emit to callback
adrianM27 Oct 31, 2025
3a06e7a
Merge branch 'feature/RDKEMW-9458' into feature/RDKEMW-9458_Fb_claud_…
adrianM27 Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions apis/AppNotifications/IAppNotifications.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,33 @@ namespace WPEFramework
ID = ID_APP_NOTIFICATIONS_HANDLER_INTERNAL
};


struct EXTERNAL IEmitter : virtual public Core::IUnknown
{
enum { ID = ID_APP_NOTIFICATIONS_HANDLER_INTERNAL_EMITTER };
virtual ~IEmitter() override = default;

// @json:omit
// @text emit
// @brief Dispatch event for a given registration, if appId is provided the dispatch happens for a given App.
// @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 */,
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

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 */,

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@adrianM27 adrianM27 Oct 31, 2025

Choose a reason for hiding this comment

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

For notification we do not return a value

const string &payload /* @in @opaque */,
const string &appId /* @in */) = 0;

};

// @json:omit
// @text handleAppEventNotifier
// @brief Handle AppEvent Notfier expectations for a given event
// @brief Handle AppEvent Notifier expectations for a given event
// @param emitCb: the emit callback interface
// @param event: the event for registration
// @param listen: whether to listen
// @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;
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@adrianM27 adrianM27 Oct 31, 2025

Choose a reason for hiding this comment

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

Thats a THunder way of passing pointers

Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

};
} // namespace Exchange
Expand Down
3 changes: 2 additions & 1 deletion apis/Ids.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ namespace Exchange {
ID_APP_GATEWAY_REQUEST_HANDLER = ID_APP_GATEWAY + 4,

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_EMITTER = ID_APP_NOTIFICATIONS + 2


}; // enum IDS
Expand Down
Loading