RDKDEV-1423 Add ThunderClientLibraries Documentation - #338
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a comprehensive component-level documentation page for ThunderClientLibraries, describing the role of the client libraries, their design patterns (COM-RPC usage, interface acquisition, lifecycle), and integration expectations for consumers.
Changes:
- Introduces a new
docs/README.mdwith architecture/design narrative and Mermaid diagrams. - Documents threading model, state/lifecycle flows, and example call flows.
- Adds module interaction matrices and configuration parameters.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
docs/README.md:7
- The intro’s Windows fallback example only mentions
127.0.0.1:63000, but in this repo ProvisionProxy defaults to127.0.0.1:7777on Windows whenPROVISION_PATHis unset (while SecurityAgent uses127.0.0.1:63000). Suggest either removing the specific port example or listing both so readers don’t assume a single default port.
At the device level, these libraries serve processes that sit outside the Thunder plugin host but require access to capabilities that are managed centrally inside it. Examples include media pipeline processes that require DRM session creation through OpenCDM, web runtimes that need display or device capability metadata, and Bluetooth audio pipelines that need to stream audio frames to or from a paired device. On Linux/RDK-V, inter-process calls use UNIX domain sockets via Thunder's COM-RPC transport; on Windows builds, some clients fall back to TCP loopback (e.g., `127.0.0.1:63000`).
docs/README.md:110
- The Mermaid diagram declares
subgraph MSec["Media & Security"]twice and has an extraend, which will likely break Mermaid rendering or create unexpected nesting. Remove the duplicatesubgraphand matchingendso the graph parses cleanly.
subgraph MSec["Media & Security"]
subgraph MSec["Media & Security"]
OA["OpenCDM\n(IAccessorOCDM)"]
SAL["SecurityAgent\n(PluginHost::IAuthenticate)"]
CGL["Cryptography\n(ICryptography)"]
docs/README.md:407
- The configuration table lists Linux socket-path defaults, but on Windows the code falls back to TCP endpoints when these env vars are unset (
SecurityAgent:127.0.0.1:63000,Provisioning:127.0.0.1:7777). Updating the defaults/description here keeps the table accurate across platforms.
| `SECURITYAGENT_PATH` | env string | `/tmp/SecurityAgent/token` | UNIX domain socket path used to connect to the SecurityAgent plugin. |
| `OPEN_CDM_SERVER` | env string | `/tmp/ocdm` | UNIX domain socket path used to connect to the OpenCDMImplementation plugin. |
| `PROVISION_PATH` | env string | `/tmp/provision` | UNIX domain socket path used to connect to the Provisioning plugin. |
ef6eaf3 to
b71c4fd
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
docs/README.md:407
- The configuration table lists Linux-only defaults and describes these values as "UNIX domain socket path". In the code, SecurityAgent and ProvisionProxy fall back to TCP loopback endpoints on Windows (
127.0.0.1:63000and127.0.0.1:7777). Please document the platform-specific defaults (or describe these as a generic COM-RPC endpoint) so the table remains accurate across supported platforms.
| `SECURITYAGENT_PATH` | env string | `/tmp/SecurityAgent/token` | UNIX domain socket path used to connect to the SecurityAgent plugin. |
| `OPEN_CDM_SERVER` | env string | `/tmp/ocdm` | UNIX domain socket path used to connect to the OpenCDMImplementation plugin. |
| `PROVISION_PATH` | env string | `/tmp/provision` | UNIX domain socket path used to connect to the Provisioning plugin. |
docs/README.md:107
- In the design diagram, several interface types are unqualified (
IAccessorOCDM,ICryptography,IProvisioning) while elsewhere in this document and in the codebase they are referenced asExchange::.... Using the fully qualified names here avoids ambiguity and keeps the documentation consistent.
OA["OpenCDM\n(IAccessorOCDM)"]
SAL["SecurityAgent\n(PluginHost::IAuthenticate)"]
CGL["Cryptography\n(ICryptography)"]
PPL["ProvisionProxy\n(IProvisioning)"]
docs/README.md:103
- The Mermaid diagram declares the same
MSecsubgraph twice (subgraph MSec[...]repeated). This is likely to break Mermaid parsing and/or produce an unexpected graph structure.
This issue also appears in the following locations of the same file:
- line 104
- line 405
subgraph MSec["Media & Security"]
subgraph MSec["Media & Security"]
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Updated README to clarify the purpose and functionality of ThunderClientLibraries, including key features and design principles.
f429531 to
8a701bb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
docs/README.md:59
- The IPC transport row claims all plugin communication uses UNIX domain sockets, but at least SecurityAgent and ProvisionProxy default to TCP loopback endpoints on Windows (e.g.
127.0.0.1:63000/127.0.0.1:7777). Please qualify this row so it stays accurate across platforms.
| COM-RPC / UNIX domain socket | All plugin communication uses Thunder COM-RPC over UNIX domain sockets | `RPC::CommunicatorClient`, `RPC::SmartInterfaceType`, `Core::NodeId` |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
docs/README.md:57
- The Compositor row lists
ISurface,IKeyboard, andIPointeras if they were top-levelThunder::Compositorinterfaces, but in this repo they are nested underThunder::Compositor::IDisplay(seeSource/compositorclient/include/compositor/Client.h:49-137). Using the correct qualified names here will prevent API consumers from looking for types that don’t exist.
| `Compositor` | Display surface creation and input event delivery | `Thunder::Compositor::IDisplay`, `ISurface`, `IKeyboard`, `IPointer` |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
Previously missed (1) — in code that hasn't changed since the last review.
docs/README.md:33
- This row refers to
Dolby::IOutput::INotification, but the code implements notifications viaExchange::Dolby::IOutput::INotification(e.g.,Source/playerinfo/PlayerInfo.cpp). The missingExchange::namespace makes the type name inaccurate.
This issue also appears on line 120 of the same file.
| `PlayerInfo` | Singleton wrapping `RPC::SmartInterfaceType<Exchange::IPlayerProperties>`. Queries `Exchange::Dolby::IOutput` and registers `Dolby::IOutput::INotification` for Dolby mode-change events. | `PlayerInfo.cpp`, `playerinfo.h` |
docs/README.md:38
- This CompositorClient row lists nested interfaces (
ISurface,IKeyboard, etc.) without their actual scope. InSource/compositorclient/include/compositor/Client.hthese are nested underThunder::Compositor::IDisplay, so the list should use fully qualified names to avoid ambiguity.
| `CompositorClient` | C++ abstraction layer (`IDisplay`, `ISurface`, `IKeyboard`, `IPointer`, `IWheel`, `ITouchPanel`). Implementation is selected at build time via `PLUGIN_COMPOSITOR_IMPLEMENTATION` and compiled from the corresponding subdirectory (e.g., `Wayland`, `Mesa`, `RPI`). | `Client.h`, `src/CMakeLists.txt` |
docs/README.md:127
- This CompositorClient HAL integration row lists
ISurfacewithout its scope.ISurfaceis nested underThunder::Compositor::IDisplayin the public header, so this should beThunder::Compositor::IDisplay::ISurfaceto be accurate.
| `Thunder::Compositor::IDisplay` / `ISurface` | `Create()`, `Destroy()`, keyboard/pointer/touch input event dispatch | CompositorClient |
docs/README.md:57
- The Compositor interaction row uses unqualified nested interface names (
ISurface,IKeyboard,IPointer). These types are nested underThunder::Compositor::IDisplayin the public header, so the table should useThunder::Compositor::IDisplay::<...>for accuracy (and to match the earlier CompositorClient description).
| `Compositor` | Display surface creation and input event delivery | `Thunder::Compositor::IDisplay`, `ISurface`, `IKeyboard`, `IPointer` |
docs/README.md:123
- In this HAL integration table, several Exchange interface names are missing their
Exchange::namespace (e.g.,IHDRProperties,IDeviceVideoCapabilities,Dolby::IOutput). In the implementation these areExchange::...types, so the current table entries are inaccurate.
| `Exchange::IDeviceInfo` / `IDeviceVideoCapabilities` / `IDeviceAudioCapabilities` | `SupportedVideoDisplays()`, `SupportedResolutions()`, `SupportedHdcp()`, `AudioCapabilities()` | DeviceInfo |
| `Exchange::IConnectionProperties` / `IHDRProperties` / `IGraphicsProperties` | `Width()`, `Height()`, `IsAudioPassthrough()`, `EDID()`, `HDRSetting()`, `TotalGpuRam()` | DisplayInfo |
| `Exchange::IPlayerProperties` / `Dolby::IOutput` | `VideoCodecs()`, `AudioCodecs()`, `Resolution()`, `SoundMode()`, `EnableAtmosOutput()`, `AudioModeChanged()` notification | PlayerInfo |
| `Exchange::ICryptography` / `IDiffieHellman` / `IVault` | `Hash()`, `Cipher()`, `DiffieHellman()`, `Vault()`, `Random()` | Cryptography |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
docs/README.md:57
- This Interaction Matrix entry mixes a fully qualified
Thunder::Compositor::IDisplaywith unqualified nested interfaces (ISurface,IKeyboard,IPointer). These interfaces are declared asThunder::Compositor::IDisplay::...inSource/compositorclient/include/compositor/Client.h, so the doc should use the fully qualified names for consistency and clarity.
| `Compositor` | Display surface creation and input event delivery | `Thunder::Compositor::IDisplay`, `ISurface`, `IKeyboard`, `IPointer` |
docs/README.md:127
- In this HAL API summary,
ISurfaceis listed as if it were a top-level type. In the compositor client public header it is nested (Thunder::Compositor::IDisplay::ISurface), so qualifying it avoids confusion when searching for the interface.
| `Thunder::Compositor::IDisplay` / `ISurface` | `Create()`, `Destroy()`, keyboard/pointer/touch input event dispatch | CompositorClient |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
docs/README.md:33
- The DisplayInfo internal module description uses unqualified
IHDRProperties,IGraphicsProperties, andIConnectionProperties::INotification, and the PlayerInfo row uses unqualifiedDolby::IOutput::INotification. In the implementation these areExchange::IHDRProperties,Exchange::IGraphicsProperties,Exchange::IConnectionProperties::INotification, andExchange::Dolby::IOutput::INotification(seeSource/displayinfo/DisplayInfo.cppandSource/playerinfo/PlayerInfo.cpp).
This issue also appears in the following locations of the same file:
- line 66
- line 121
| `DisplayInfo` | Singleton wrapping `RPC::SmartInterfaceType<Exchange::IConnectionProperties>`. Also queries `IHDRProperties` and `IGraphicsProperties`. Registers `IConnectionProperties::INotification` to deliver display-change events. | `DisplayInfo.cpp`, `displayinfo.h` |
| `PlayerInfo` | Singleton wrapping `RPC::SmartInterfaceType<Exchange::IPlayerProperties>`. Queries `Exchange::Dolby::IOutput` and registers `Dolby::IOutput::INotification` for Dolby mode-change events. | `PlayerInfo.cpp`, `playerinfo.h` |
docs/README.md:122
- In the HAL APIs table,
IHDRProperties,IGraphicsProperties, andDolby::IOutputare missing theirExchange::namespace, which is how they are referenced in the code (e.g.,Exchange::IHDRProperties,Exchange::IGraphicsProperties,Exchange::Dolby::IOutput).
| `Exchange::IConnectionProperties` / `IHDRProperties` / `IGraphicsProperties` | `Width()`, `Height()`, `IsAudioPassthrough()`, `EDID()`, `HDRSetting()`, `TotalGpuRam()` | DisplayInfo |
| `Exchange::IPlayerProperties` / `Dolby::IOutput` | `VideoCodecs()`, `AudioCodecs()`, `Resolution()`, `SoundMode()`, `EnableAtmosOutput()`, `AudioModeChanged()` notification | PlayerInfo |
docs/README.md:127
- In the HAL APIs table,
ISurfaceis listed unqualified. In the public compositor header it is nested underThunder::Compositor::IDisplay(Thunder::Compositor::IDisplay::ISurface).
| `Thunder::Compositor::IDisplay` / `ISurface` | `Create()`, `Destroy()`, keyboard/pointer/touch input event dispatch | CompositorClient |
docs/README.md:66
- The DisplayInfo display-output-change event references
IConnectionProperties::INotification::Updatedwithout theExchange::namespace. The notification interface isExchange::IConnectionProperties::INotificationin the implementation (Source/displayinfo/DisplayInfo.cpp).
| `displayinfo_display_output_change_cb` | Display connection properties change (e.g. HDMI cable insert/remove), delivered via `IConnectionProperties::INotification::Updated` | Callers registered with `displayinfo_register_display_output_change_callback()` |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
docs/README.md:33
- In the Internal Modules table, several interface names are missing the
Exchange::namespace (IHDRProperties,IGraphicsProperties,IConnectionProperties::INotification, andDolby::IOutput::INotification). The implementation uses fully qualifiedExchange::...types (e.g.,Exchange::IConnectionProperties::INotificationandExchange::Dolby::IOutput::INotification), so leaving these unqualified is inconsistent and can mislead readers searching for the interfaces.
This issue also appears on line 66 of the same file.
| `DisplayInfo` | Singleton wrapping `RPC::SmartInterfaceType<Exchange::IConnectionProperties>`. Also queries `IHDRProperties` and `IGraphicsProperties`. Registers `IConnectionProperties::INotification` to deliver display-change events. | `DisplayInfo.cpp`, `displayinfo.h` |
| `PlayerInfo` | Singleton wrapping `RPC::SmartInterfaceType<Exchange::IPlayerProperties>`. Queries `Exchange::Dolby::IOutput` and registers `Dolby::IOutput::INotification` for Dolby mode-change events. | `PlayerInfo.cpp`, `playerinfo.h` |
docs/README.md:123
- In the HAL integration table, several Exchange interfaces are listed without their
Exchange::namespace (IDeviceVideoCapabilities,IDeviceAudioCapabilities,IHDRProperties,IGraphicsProperties,Dolby::IOutput,IDiffieHellman,IVault). The implementation consistently usesExchange::...for these types (e.g.,Exchange::IDiffieHellman,Exchange::IVault), so qualifying them here improves accuracy and makes it easier to locate the interfaces.
| `Exchange::IDeviceInfo` / `IDeviceVideoCapabilities` / `IDeviceAudioCapabilities` | `SupportedVideoDisplays()`, `SupportedResolutions()`, `SupportedHdcp()`, `AudioCapabilities()` | DeviceInfo |
| `Exchange::IConnectionProperties` / `IHDRProperties` / `IGraphicsProperties` | `Width()`, `Height()`, `IsAudioPassthrough()`, `EDID()`, `HDRSetting()`, `TotalGpuRam()` | DisplayInfo |
| `Exchange::IPlayerProperties` / `Dolby::IOutput` | `VideoCodecs()`, `AudioCodecs()`, `Resolution()`, `SoundMode()`, `EnableAtmosOutput()`, `AudioModeChanged()` notification | PlayerInfo |
| `Exchange::ICryptography` / `IDiffieHellman` / `IVault` | `Hash()`, `Cipher()`, `DiffieHellman()`, `Vault()`, `Random()` | Cryptography |
docs/README.md:66
- This event trigger references
IConnectionProperties::INotification::Updatedwithout theExchange::namespace. In the code, this notification interface isExchange::IConnectionProperties::INotification, so the doc should use the fully qualified name for consistency and to avoid ambiguity.
| `displayinfo_display_output_change_cb` | Display connection properties change (e.g. HDMI cable insert/remove), delivered via `IConnectionProperties::INotification::Updated` | Callers registered with `displayinfo_register_display_output_change_callback()` |
|
The review comments are now addressed |
RDKDEV-1423
Added component documentation for thunderclientlibraries
I have read the CLA Document and I hereby sign the CLA