Conversation
-SetZOrder, GetZOrder, StartVncServer, StopVncServer Signed-off-by: sajilal711 <shreyas.sajilal@gmail.com>
… topic/RDKEMW-12410
There was a problem hiding this comment.
Pull request overview
This PR updates the RDKWindowManager API surface (and its generated markdown) to document and expose z-order control plus VNC server start/stop, and adjusts PackageManager generated documentation/annotations.
Changes:
- Add
SetZOrder/GetZOrderand VNC server control methods toIRDKWindowManager. - Update
docs/apis/RDKWindowManager.mdto include the new methods and re-number example request IDs. - Adjust PackageManager docs ordering and remove the JSON-RPC tagging line for
IPackageHandler.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| docs/apis/RDKWindowManager.md | Adds documentation entries for z-order and VNC server APIs and shifts example IDs. |
| docs/apis/PackageManager.md | Reorders method list/sections and removes getLockedInfo documentation section. |
| apis/RDKWindowManager/IRDKWindowManager.h | Adds new z-order + VNC server API methods. |
| apis/PackageManager/IAppPackageManager.h | Removes the @json ... @text:keep marker above IPackageHandler. |
| | [startVncServer](#startVncServer) | Starts the VNC server @retval Core::ERROR_NONE on success | | ||
| | [stopVncServer](#stopVncServer) | Stops the VNC server @retval Core::ERROR_NONE on success | |
There was a problem hiding this comment.
The methods table includes @retval ... text in the short descriptions for startVncServer/stopVncServer, which reads like a Doxygen tag leaking into the generated markdown. Keep the table description user-focused (what it does) and put return-code details in the appropriate results/error section instead.
| | [startVncServer](#startVncServer) | Starts the VNC server @retval Core::ERROR_NONE on success | | |
| | [stopVncServer](#stopVncServer) | Stops the VNC server @retval Core::ERROR_NONE on success | | |
| | [startVncServer](#startVncServer) | Starts the VNC server | | |
| | [stopVncServer](#stopVncServer) | Stops the VNC server | |
| | Name | Type | Description | | ||
| | :-------- | :-------- | :-------- | | ||
| | result | object | | | ||
| | result.zOrder | integer | integer value indicating the zOrder @retval Core::ERROR_NONE on success | |
There was a problem hiding this comment.
In this markdown, the @retval Core::ERROR_NONE on success fragment appears inside the field description (result.zOrder). That looks like a documentation tag leaking into the generated output; it should be expressed as a return code description rather than part of a data field description.
| | result.zOrder | integer | integer value indicating the zOrder @retval Core::ERROR_NONE on success | | |
| | result.zOrder | integer | integer value indicating the zOrder | |
| | :-------- | :-------- | :-------- | | ||
| | params | object | | | ||
| | params.appInstanceId | string | the identifier of the connected application | | ||
| | params.zOrder | integer | integer value indicating the zOrder @retval Core::ERROR_NONE on success | |
There was a problem hiding this comment.
The parameter description for params.zOrder includes @retval Core::ERROR_NONE on success, which is a return-code detail and doesn't belong in a parameter description. Move/express return codes separately so the parameter docs stay purely about inputs.
| | params.zOrder | integer | integer value indicating the zOrder @retval Core::ERROR_NONE on success | | |
| | params.zOrder | integer | integer value indicating the zOrder | |
| // @param appInstanceId: client name or application instance ID | ||
| // @param zOrder: integer value indicating the zOrder of the client | ||
| // @retval Core::ERROR_NONE on success | ||
| virtual Core::hresult GetZOrder(const string& appInstanceId, int32_t &zOrder /* @out */) = 0; |
There was a problem hiding this comment.
The @retval line for GetZOrder is missing the required : <Description> portion (e.g., @retval Core::ERROR_NONE: Successfully retrieved z-order).
| <a id="startVncServer"></a> | ||
| ## *startVncServer* | ||
|
|
||
| Starts the VNC server @retval Core::ERROR_NONE on success |
There was a problem hiding this comment.
The short description line includes an @retval ... fragment (Starts the VNC server @retval ...), which looks like a Doxygen tag leaking into the generated markdown. Keep the description as plain text and document return codes separately.
| Starts the VNC server @retval Core::ERROR_NONE on success | |
| Starts the VNC server. |
| /** Starts the VNC server */ | ||
| // @text startVncServer | ||
| // @brief Starts the VNC server | ||
| // @retval Core::ERROR_NONE on success | ||
| virtual Core::hresult StartVncServer() = 0; | ||
|
|
||
| /** Stops the VNC server */ | ||
| // @text stopVncServer | ||
| // @brief Stops the VNC server | ||
| // @retval Core::ERROR_NONE on success | ||
| virtual Core::hresult StopVncServer() = 0; |
There was a problem hiding this comment.
StartVncServer/StopVncServer use mixed-case for the acronym ("Vnc"). Per the repo guideline, acronyms should be fully capitalized; consider renaming to StartVNCServer/StopVNCServer (and updating the corresponding @text JSON-RPC names/docs accordingly) while the API is still new.
| // @brief Sets the zOrder of the given client or appInstanceId | ||
| // @param appInstanceId: client name or application instance ID | ||
| // @param zOrder: integer value indicating the zOrder | ||
| // @retval Core::ERROR_NONE on success | ||
| virtual Core::hresult SetZOrder(const string& appInstanceId, const int32_t zOrder) = 0; |
There was a problem hiding this comment.
The new @retval tag format is inconsistent with the documented requirement (@retval <ErrorCode>: <Description>). Here it is missing the colon + description (e.g., Core::ERROR_NONE: ...), which can also leak into generated markdown.
| // @param appInstanceId: client name or application instance ID | ||
| // @param zOrder: integer value indicating the zOrder of the client | ||
| // @retval Core::ERROR_NONE on success | ||
| virtual Core::hresult GetZOrder(const string& appInstanceId, int32_t &zOrder /* @out */) = 0; |
There was a problem hiding this comment.
GetZOrder is a getter but is not marked const, unlike other getters in this interface (e.g., GetApps, GetKeyRepeatsEnabled, GetLastKeyInfo). If it doesn't mutate state, make it const for API consistency and usability on const instances.
| virtual Core::hresult GetZOrder(const string& appInstanceId, int32_t &zOrder /* @out */) = 0; | |
| virtual Core::hresult GetZOrder(const string& appInstanceId, int32_t &zOrder /* @out */) const = 0; |
| // @text startVncServer | ||
| // @brief Starts the VNC server | ||
| // @retval Core::ERROR_NONE on success | ||
| virtual Core::hresult StartVncServer() = 0; |
There was a problem hiding this comment.
The new @retval tag format is missing the required : <Description> portion (e.g., @retval Core::ERROR_NONE: <...>).
| // @text stopVncServer | ||
| // @brief Stops the VNC server | ||
| // @retval Core::ERROR_NONE on success | ||
| virtual Core::hresult StopVncServer() = 0; |
There was a problem hiding this comment.
The new @retval tag format is missing the required : <Description> portion (e.g., @retval Core::ERROR_NONE: <...>).
No description provided.