RDKEMW-6311: Devicesettings plugin interface.#703
RDKEMW-6311: Devicesettings plugin interface.#703santoshcomcast wants to merge 4 commits intodevelopfrom
Conversation
Documentation Auto-GeneratedDocumentation has been automatically generated for the changed plugins and committed to this PR branch. The generated documentation files have been added to the PR for review. |
Reason for change: Add Devicesettings plugin interface. Test Procedure: refer RDKEMW-6311 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>
6d8adb2 to
67f016a
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new UserPlugin API and a comprehensive DeviceSettingsManager interface set, along with corresponding documentation and ID registrations, to expose device power, volume, and low‑level device settings via COM/JSON-RPC.
Changes:
- Added
IUserPlugininterface (with power state and volume level APIs) and its generated UserPlugin API documentation, and linked it into the docs sidebar. - Extended
Ids.hwith IDs forID_USER_PLUGINand multipleID_DEVICESETTINGS_MANAGER_*interfaces and iterators. - Introduced
IDeviceSettingsManager.hdefining audio, composite-in, display, front panel display, HDMI-in, host, video device, and video port manager interfaces, plus their notification sub‑interfaces and data structures.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/apis/UserPlugin.md | New auto-generated API reference for the UserPlugin plugin (methods getDevicePowerState and getVolumeLevel). |
| docs/_sidebar.md | Adds a sidebar entry for the new UserPlugin documentation so it appears in the API reference navigation. |
| apis/UserPlugin/IUserPlugin.h | Defines the IUserPlugin COM/JSON-RPC interface for device power state and volume level retrieval. |
| apis/Ids.h | Allocates unique IDs for ID_USER_PLUGIN and the various IDeviceSettingsManager* interfaces and iterators. |
| apis/DeviceSettingsManager/IDeviceSettingsManager.h | Introduces the IDeviceSettingsManager* family of interfaces and notifications covering audio, composite-in, display, FPD, HDMI-in, host, video device, and video port settings. |
| - [TextTrack](apis/TextTrack.md) | ||
| - [USBDevice](apis/USBDevice.md) | ||
| - [USBMassStorage](apis/USBMassStorage.md) | ||
| - [User<sup>@</sup>](apis/UserPlugin.md) |
There was a problem hiding this comment.
The sidebar entry label User does not match the actual plugin name UserPlugin used in the generated API documentation (UserPlugin Plugin) and the callsign (org.rdk.UserPlugin). To avoid confusion and to stay consistent with other plugin entries (e.g. DisplaySettings, VoiceControl), this entry should be renamed to UserPlugin<sup>@</sup> while keeping the same link target.
| - [User<sup>@</sup>](apis/UserPlugin.md) | |
| - [UserPlugin<sup>@</sup>](apis/UserPlugin.md) |
| // @brief Audio mode for the respective audio port - raised for every type of port | ||
| // @text onAudioModeEvent | ||
| // @param audioPortType: audio port type see AudioPortType | ||
| // @param audioMode: audio mode - see StereoMode | ||
| virtual void OnAudioModeEvent(AudioPortType audioPortType, StereoMode audioMode) { }; | ||
|
|
||
| // @brief Audio mode for the respective audio port - raised for every type of port | ||
| // @text onAudioModeEvent | ||
| // @param audioPortType: audio port type see AudioPortType | ||
| // @param audioMode: audio mode - see StereoMode | ||
| virtual void OnAudioLevelChangedEvent(int32_t audioLevel) { }; |
There was a problem hiding this comment.
The notification OnAudioLevelChangedEvent has @text and @param documentation copied from OnAudioModeEvent (referring to onAudioModeEvent, audioPortType, and audioMode), which no longer match the method signature that only takes an audioLevel. This will generate an incorrect JSON-RPC event name and misleading documentation; please update the @text tag to a distinct camelCase name (for example onAudioLevelChangedEvent) and adjust the @brief/@param tags to describe the actual audioLevel parameter.
| /** Get Audio Level */ | ||
| // @text GetAudioDucking | ||
| // @brief Get Audio Level | ||
| // @param handle: handle returned in GetAudioPort() | ||
| // @param audioLevel: Audio level | ||
| virtual Core::hresult GetAudioLevel(const int32_t handle , float &audioLevel /* @out */) = 0; |
There was a problem hiding this comment.
For the GetAudioLevel API, the JSON-RPC name tag // @text GetAudioDucking is both misnamed (it refers to ducking rather than level) and not in the required camelCase form. This should be updated to a camelCase name that reflects the method, such as getAudioLevel, so that the generated JSON-RPC API surface matches the C++ method purpose.
| /** Select HDMIIn Port. */ | ||
| // @text selectHDMIInPort | ||
| // @brief Get HDMIIn Status | ||
| // @param port: Port to select | ||
| // @param requestAudioMix: Should audio be mixed on this port | ||
| // @param topMostPlane: Should this be on top for display | ||
| // @param videoPlaneType: Primary or secondary | ||
| virtual Core::hresult SelectHDMIInPort(const HDMIInPort port , const bool requestAudioMix , const bool topMostPlane , const HDMIVideoPlaneType videoPlaneType ) = 0; |
There was a problem hiding this comment.
The SelectHDMIInPort API has a @brief Get HDMIIn Status description that is inconsistent with the method name and parameters, which clearly indicate that it selects an HDMI input and configures audio mixing and plane placement. Please update the brief to accurately describe that this method selects an HDMI input port and its presentation options.
| // @brief Display HDCP Status | ||
| // @text OnDisplayHDCPStatus | ||
| virtual void OnDisplayHDCPStatus() {}; |
There was a problem hiding this comment.
The Display notification OnDisplayHDCPStatus uses a JSON-RPC name tag @text OnDisplayHDCPStatus starting with an uppercase O, which violates the requirement that JSON-RPC names be camelCase. Please change this to a camelCase event name (for example onDisplayHDCPStatus) to align with the other notifications in this interface.
| // @json @text:keep | ||
| struct EXTERNAL IUserPlugin : virtual public Core::IUnknown { |
There was a problem hiding this comment.
This interface is marked as JSON-RPC but the @json annotation here is missing a version and uses line comment syntax, which violates the JSON-RPC tagging guidelines and is inconsistent with newer headers like IUserSettings that use /* @json 1.0.0 @text:keep */. Please switch this to a block comment with an explicit version (for example /* @json 1.0.0 @text:keep */) so tooling and docs can correctly version this API.
| /** Set Audio Surroud Virtualizer */ | ||
| // @text setAudioSurroudVirtualizer | ||
| // @brief Set Audio Surroud Virtualizer | ||
| // @param handle: handle returned in GetAudioPort() | ||
| // @param surroundVirtualizer: virtualizer | ||
| virtual Core::hresult SetAudioSurroudVirtualizer(const int32_t handle , const SurroundVirtualizer surroundVirtualizer ) = 0; | ||
|
|
||
| /** Get Audio Surroud Virtualizer */ | ||
| // @text getAudioSurroudVirtualizer | ||
| // @brief Get Audio Surroud Virtualizer | ||
| // @param handle: handle returned in GetAudioPort() | ||
| // @param surroundVirtualizer: virtualizer | ||
| virtual Core::hresult GetAudioSurroudVirtualizer(const int32_t handle , SurroundVirtualizer &surroundVirtualizer /* @out */) = 0; |
There was a problem hiding this comment.
"Surroud" is misspelled in several related APIs and their JSON-RPC names (EnableAudioSurroudDecoder, IsAudioSurroudDecoderEnabled, setAudioSurroudVirtualizer, getAudioSurroudVirtualizer), which will expose the typo in both C++ and JSON-RPC interfaces and in the generated documentation. Consider correcting these to use the proper spelling "Surround" in both method names and @text tags (for example EnableAudioSurroundDecoder, setAudioSurroundVirtualizer, etc.) to keep the public API surface professional and self-explanatory.
| /** Set Audio Surroud Virtualizer */ | |
| // @text setAudioSurroudVirtualizer | |
| // @brief Set Audio Surroud Virtualizer | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param surroundVirtualizer: virtualizer | |
| virtual Core::hresult SetAudioSurroudVirtualizer(const int32_t handle , const SurroundVirtualizer surroundVirtualizer ) = 0; | |
| /** Get Audio Surroud Virtualizer */ | |
| // @text getAudioSurroudVirtualizer | |
| // @brief Get Audio Surroud Virtualizer | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param surroundVirtualizer: virtualizer | |
| virtual Core::hresult GetAudioSurroudVirtualizer(const int32_t handle , SurroundVirtualizer &surroundVirtualizer /* @out */) = 0; | |
| /** Set Audio Surround Virtualizer */ | |
| // @text setAudioSurroundVirtualizer | |
| // @brief Set Audio Surround Virtualizer | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param surroundVirtualizer: virtualizer | |
| virtual Core::hresult SetAudioSurroundVirtualizer(const int32_t handle , const SurroundVirtualizer surroundVirtualizer ) = 0; | |
| /** Get Audio Surround Virtualizer */ | |
| // @text getAudioSurroundVirtualizer | |
| // @brief Get Audio Surround Virtualizer | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param surroundVirtualizer: virtualizer | |
| virtual Core::hresult GetAudioSurroundVirtualizer(const int32_t handle , SurroundVirtualizer &surroundVirtualizer /* @out */) = 0; |
| // @text getHDMIInNumbefOfInputs | ||
| // @brief Get Number of HDMI Inputs in the platform | ||
| // @param count: number of inputs | ||
| virtual Core::hresult GetHDMIInNumbefOfInputs(int32_t &count /* @out */) = 0; |
There was a problem hiding this comment.
Both the method name and JSON-RPC tag for GetHDMIInNumbefOfInputs contain a typo (Numbef instead of Number), which will leak into the public API surface and documentation. Please correct the spelling to GetHDMIInNumberOfInputs (and @text getHDMIInNumberOfInputs) so the API name is clear and professional.
| // @text getHDMIInNumbefOfInputs | |
| // @brief Get Number of HDMI Inputs in the platform | |
| // @param count: number of inputs | |
| virtual Core::hresult GetHDMIInNumbefOfInputs(int32_t &count /* @out */) = 0; | |
| // @text getHDMIInNumberOfInputs | |
| // @brief Get Number of HDMI Inputs in the platform | |
| // @param count: number of inputs | |
| virtual Core::hresult GetHDMIInNumberOfInputs(int32_t &count /* @out */) = 0; |
| // @brief On Resolution Pre changed | ||
| // @text OnResolutionPreChange | ||
| // @param resolution: resolution | ||
| virtual void OnResolutionPreChange(const ResolutionChange resolution) {}; | ||
|
|
||
| // @brief On HDCP Status change | ||
| // @text OnHDCPStatusChange | ||
| // @param hdcpStatus: HDCP Status | ||
| virtual void OnHDCPStatusChange(const HDCPStatus hdcpStatus) {}; | ||
|
|
||
| // @brief On Video Format update | ||
| // @text OnVideoFormatUpdate | ||
| // @param videoFormatHDR: Video format HDR standard | ||
| virtual void OnVideoFormatUpdate(const HDRStandard videoFormatHDR) {}; |
There was a problem hiding this comment.
Several VideoPort notifications use @text tags that are not camelCase (for example OnResolutionPreChange, OnHDCPStatusChange, OnVideoFormatUpdate), even though the JSON-RPC convention is to start names with a lowercase letter. Please update these JSON-RPC event names to camelCase (e.g. onResolutionPreChange, onHDCPStatusChange, onVideoFormatUpdate) so they are consistent with the rest of the API surface.
| // @brief Set Audio Bass Enhancer | ||
| // @param handle: handle returned in GetAudioPort() | ||
| // @param drcMode: mode | ||
| virtual Core::hresult SetAudioDRCMode(const int32_t handle , const int32_t drcMode ) = 0; | ||
|
|
||
| /** Get Audio DRC mode */ | ||
| // @text getAudioDRCMode | ||
| // @brief Get Audio Bass Enhancer | ||
| // @param handle: handle returned in GetAudioPort() | ||
| // @param drcMode: mode |
There was a problem hiding this comment.
The SetAudioDRCMode and GetAudioDRCMode APIs have @brief descriptions that still refer to "Audio Bass Enhancer", which does not match the method names or parameters and will confuse consumers. Please update both brief descriptions to explain that these APIs set and get the Dynamic Range Control (DRC) mode rather than the bass enhancer.
| // @brief Set Audio Bass Enhancer | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param drcMode: mode | |
| virtual Core::hresult SetAudioDRCMode(const int32_t handle , const int32_t drcMode ) = 0; | |
| /** Get Audio DRC mode */ | |
| // @text getAudioDRCMode | |
| // @brief Get Audio Bass Enhancer | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param drcMode: mode | |
| // @brief Set the audio Dynamic Range Control (DRC) mode for the specified port | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param drcMode: Dynamic Range Control (DRC) mode value to be applied | |
| virtual Core::hresult SetAudioDRCMode(const int32_t handle , const int32_t drcMode ) = 0; | |
| /** Get Audio DRC mode */ | |
| // @text getAudioDRCMode | |
| // @brief Get the current audio Dynamic Range Control (DRC) mode for the specified port | |
| // @param handle: handle returned in GetAudioPort() | |
| // @param drcMode: Dynamic Range Control (DRC) mode value currently in use |
Reason for change: Add Devicesettings plugin interface. Test Procedure: refer RDKEMW-6311
Risks: High
Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com