RDKEMW-12082: Interface Changes for RDKAppManagers plugin#752
RDKEMW-12082: Interface Changes for RDKAppManagers plugin#752ABIRAMI-S wants to merge 1 commit intotopic/RDKEMW-13976from
Conversation
There was a problem hiding this comment.
Pull request overview
Adds new Thunder RPC/JSON-RPC interface definitions and ID allocations to support the RDKAppManagers “Application Service” plugin API surface.
Changes:
- Allocates a new
ID_APPLICATION_SERVICE_*ID range inapis/Ids.h. - Introduces
apis/ApplicationManager/IApplicationService.hdefining request/config/listener/system-info/system-settings/test-preferences/diagnostics interfaces for Thunder codegen.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
apis/Ids.h |
Adds a new ID block for Application Service related interfaces. |
apis/ApplicationManager/IApplicationService.h |
New Thunder RPC interface header defining Application Service APIs and notifications. |
| // @return Error code: ERROR_NONE on success | ||
| // @text:request | ||
| virtual Core::hresult Request( |
There was a problem hiding this comment.
The @text tag syntax is inconsistent with the project’s interface-header guidelines/examples (which use // @text <name> with a space). Using // @text:<name> may not be recognized by the documentation/code generators. Please switch these to the documented form (e.g., // @text request, // @text getSystemInfo, etc.).
| // @brief Process an HTTP-style request | ||
| // @param flags Request type flags: GET=0x01, POST=0x02, DELETE=0x04 | ||
| // @param url Request URL path (e.g., "/as/network/ipconfig/settings") | ||
| // @param headers HTTP headers as JSON object string | ||
| // @param queryParams Query parameters as JSON object string | ||
| // @param body Request body (for POST requests) | ||
| // @param code HTTP status code (output) | ||
| // @param responseHeaders Response headers as JSON object string (output) | ||
| // @param responseBody Response body as JSON string (output) | ||
| // @return Error code: ERROR_NONE on success | ||
| // @text:request | ||
| virtual Core::hresult Request( | ||
| const uint32_t flags, |
There was a problem hiding this comment.
flags encodes a small, well-known set of request types (GET/POST/DELETE) but is exposed as a raw uint32_t. Per the interface guidelines, prefer an enum/bitmask enum for constrained values to make the API self-describing and type-safe (and to avoid undocumented flag combinations).
| // @brief Process an HTTP-style request | |
| // @param flags Request type flags: GET=0x01, POST=0x02, DELETE=0x04 | |
| // @param url Request URL path (e.g., "/as/network/ipconfig/settings") | |
| // @param headers HTTP headers as JSON object string | |
| // @param queryParams Query parameters as JSON object string | |
| // @param body Request body (for POST requests) | |
| // @param code HTTP status code (output) | |
| // @param responseHeaders Response headers as JSON object string (output) | |
| // @param responseBody Response body as JSON string (output) | |
| // @return Error code: ERROR_NONE on success | |
| // @text:request | |
| virtual Core::hresult Request( | |
| const uint32_t flags, | |
| enum REQUEST_FLAGS : uint32_t { | |
| REQUEST_GET = 0x01 /* @text REQUEST_GET */, | |
| REQUEST_POST = 0x02 /* @text REQUEST_POST */, | |
| REQUEST_DELETE = 0x04 /* @text REQUEST_DELETE */ | |
| }; | |
| // @brief Process an HTTP-style request | |
| // @param flags Request type flags. Bitmask of REQUEST_FLAGS (e.g., REQUEST_GET, REQUEST_POST, REQUEST_DELETE) | |
| // @param url Request URL path (e.g., "/as/network/ipconfig/settings") | |
| // @param headers HTTP headers as JSON object string | |
| // @param queryParams Query parameters as JSON object string | |
| // @param body Request body as JSON string (typically used for POST requests) | |
| // @param code HTTP status code (output) | |
| // @param responseHeaders Response headers as JSON object string (output) | |
| // @param responseBody Response body as JSON string (output) | |
| // @retval Core::ERROR_NONE: Request processed successfully | |
| // @retval Core::ERROR_GENERAL: Request processing failed | |
| // @text:request | |
| virtual Core::hresult Request( | |
| const REQUEST_FLAGS flags, |
| // @brief Process an HTTP-style request | ||
| // @param flags Request type flags: GET=0x01, POST=0x02, DELETE=0x04 | ||
| // @param url Request URL path (e.g., "/as/network/ipconfig/settings") | ||
| // @param headers HTTP headers as JSON object string | ||
| // @param queryParams Query parameters as JSON object string | ||
| // @param body Request body (for POST requests) | ||
| // @param code HTTP status code (output) | ||
| // @param responseHeaders Response headers as JSON object string (output) | ||
| // @param responseBody Response body as JSON string (output) | ||
| // @return Error code: ERROR_NONE on success | ||
| // @text:request |
There was a problem hiding this comment.
Method documentation uses @return prose but is missing the required @retval <ErrorCode>: <Description> annotations. Please add explicit @retval entries for each possible return value for these APIs (e.g., success and common error cases), as required for generated docs.
| // @brief WebSocket update notification | ||
| // @param url WebSocket URL that sent update | ||
| // @param message Update message content | ||
| // @text:onNotifyWebSocketUpdate | ||
| virtual void OnNotifyWebSocketUpdate(const string &url, const string &message) = 0; | ||
|
|
There was a problem hiding this comment.
IApplicationServiceListener::INotification defines notification methods as pure virtual (= 0). Notification interfaces in these API headers must provide default (empty) implementations and must not use pure virtual methods, to keep implementation and binary compatibility expectations consistent.
| // @brief WebSocket update notification | ||
| // @param url WebSocket URL that sent update | ||
| // @param message Update message content | ||
| // @text:onNotifyWebSocketUpdate | ||
| virtual void OnNotifyWebSocketUpdate(const string &url, const string &message) = 0; | ||
|
|
||
| // @brief HTTP update notification | ||
| // @param url HTTP endpoint that sent update | ||
| // @param code HTTP status code | ||
| // @text:onNotifyHttpUpdate | ||
| virtual void OnNotifyHttpUpdate(const string &url, const uint32_t code) = 0; |
There was a problem hiding this comment.
Notification naming doesn’t follow the documented on[Object][Action] pattern. For example, OnNotifyWebSocketUpdate / @text:onNotifyWebSocketUpdate uses an extra “Notify” prefix that makes the object/action unclear. Please rename notifications (and their @text names) to match the pattern (e.g., OnWebSocketUpdate with // @text onWebSocketUpdate).
| }; | ||
| virtual Core::hresult Register(IApplicationServiceListener::INotification *notification) = 0; |
There was a problem hiding this comment.
Register / Unregister are missing the required interface documentation tags (@brief, @param, and (where applicable) @retval). Please document these methods consistently so the generated API docs are complete.
| }; | |
| virtual Core::hresult Register(IApplicationServiceListener::INotification *notification) = 0; | |
| }; | |
| // @brief Register a notification sink for application service listener events. | |
| // @param notification Pointer to the notification interface instance to be registered. | |
| // @retval Core::ERROR_NONE: Registration succeeded. | |
| // @retval Core::ERROR_BAD_REQUEST: The notification parameter is null or invalid. | |
| // @retval Core::ERROR_ALREADY_CONNECTED: The notification interface is already registered. | |
| // @text register | |
| virtual Core::hresult Register(IApplicationServiceListener::INotification *notification) = 0; | |
| // @brief Unregister a previously registered notification sink. | |
| // @param notification Pointer to the notification interface instance to be unregistered. | |
| // @retval Core::ERROR_NONE: Unregistration succeeded. | |
| // @retval Core::ERROR_BAD_REQUEST: The notification parameter is null or invalid. | |
| // @retval Core::ERROR_UNKNOWN_KEY: The specified notification interface is not currently registered. | |
| // @text unregister |
|
|
||
| ID_RESOURCEMANAGER = ID_ENTOS_OFFSET + 0x480, | ||
|
|
||
| ID_APPLICATION_SERVICE_REQUEST = ID_ENTOS_OFFSET + 0x490, |
There was a problem hiding this comment.
Indentation within the newly added ID block is inconsistent (ID_APPLICATION_SERVICE_REQUEST is tab-indented while the following lines are space-indented). Please normalize indentation in this block to match the surrounding style to keep diffs clean.
| ID_APPLICATION_SERVICE_REQUEST = ID_ENTOS_OFFSET + 0x490, | |
| ID_APPLICATION_SERVICE_REQUEST = ID_ENTOS_OFFSET + 0x490, |
| // | ||
| // IApplicationService.h | ||
| // Thunder RPC interface definitions for AS Services | ||
| // Uses Thunder header-based code generation approach | ||
| // | ||
| // USAGE: | ||
| // Run Thunder ProxyStubGenerator or JsonGenerator on this file to generate: | ||
| // - JSON-RPC proxy classes (client-side for asproxy) | ||
| // - JSON-RPC stub registration (server-side for services) | ||
| // - Marshalling/unmarshalling code | ||
| // | ||
|
|
||
| #pragma once | ||
|
|
||
| #include "Module.h" |
There was a problem hiding this comment.
The file header is missing the standard Apache 2.0 copyright/license block that other apis/**/I*.h interface headers include. Please add the repository-standard license header at the top of this new API file for consistency and compliance.
madanagopalt
left a comment
There was a problem hiding this comment.
Rename ApplicationManager name to RDKAppManagerService
No description provided.