Skip to content

RDKEMW-997 : Add COM-RPC support to LinearPlaybackControl plugin#721

Open
naveen-0206 wants to merge 2 commits intodevelopfrom
feature/29_jan_apis
Open

RDKEMW-997 : Add COM-RPC support to LinearPlaybackControl plugin#721
naveen-0206 wants to merge 2 commits intodevelopfrom
feature/29_jan_apis

Conversation

@naveen-0206
Copy link
Contributor

Reason For Change: Adding LinearPlaybackControl interface header Changes
Test procedure : Test LinearPlaybackControl
Priority: P1

Copilot AI review requested due to automatic review settings January 29, 2026 06:12
@naveen-0206 naveen-0206 requested a review from a team as a code owner January 29, 2026 06:12
@naveen-0206 naveen-0206 review requested due to automatic review settings January 29, 2026 06:12
@github-actions
Copy link

Documentation Auto-Generated

Documentation 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.

Copilot AI review requested due to automatic review settings January 29, 2026 06:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings February 6, 2026 12:29
@naveen-0206 naveen-0206 requested review from Copilot and removed request for Copilot February 6, 2026 12:31
@github-actions
Copy link

github-actions bot commented Feb 6, 2026

Documentation Auto-Generated

Documentation 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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment on lines +39 to +43
// @text speedchanged
// @brief Triggered when the trick play speed has changed
// @param speed: New trick play speed (negative=rewind, positive=fast forward)
// @param muxId: Stream muxId
virtual void OnSpeedChanged(const int16_t speed, const uint8_t muxId) { }
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The JSON-RPC @text for this notification is speedchanged, which is neither camelCase nor following the required on[Object][Action] pattern. This should be something like onSpeedChanged to align with the notification naming rules.
Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_notifications.instructions.md#notification-naming-for-interface-and-json-rpc and https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_notifications.instructions.md#notification-name-formatting

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +45 to +46

virtual Core::hresult Register(ILinearPlaybackControl::INotification* notification) = 0;
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

Register/Unregister are missing the required documentation tags (at minimum @brief, @param, and expected return codes via @retval). Please annotate these methods similarly to the rest of the interface so the generated API docs are complete.
Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#documentation-tags-guidelines

Suggested change
virtual Core::hresult Register(ILinearPlaybackControl::INotification* notification) = 0;
// @text register
// @brief Registers a notification callback for linear playback control events
// @param notification: Notification interface instance to register
// @retval ERROR_NONE: Success
// @retval ERROR_GENERAL: Registration failed
virtual Core::hresult Register(ILinearPlaybackControl::INotification* notification) = 0;
// @text unregister
// @brief Unregisters a previously registered notification callback
// @param notification: Notification interface instance to unregister
// @retval ERROR_NONE: Success
// @retval ERROR_GENERAL: Unregistration failed

Copilot uses AI. Check for mistakes.
Comment on lines +55 to +57
// @retval ERROR_NONE: Success
// @retval ERROR_BAD_REQUEST: Bad request
virtual Core::hresult SetChannel(const string& demuxerId, const string& channel, uint32_t& result /* @out */) = 0;
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The @retval values here (e.g., ERROR_NONE, ERROR_BAD_REQUEST) don’t appear to be defined anywhere in this interface (or in the included entservices_errorcodes.h), so the generated documentation will reference unknown symbols. Consider either (1) defining an enum for the result out-parameter in this header and using it consistently in @retval, or (2) switching the documentation to established error codes already used across the repo (e.g., ErrorCode::ERROR_* / Core::ERROR_*).
Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#documentation-tags-guidelines

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings February 6, 2026 12:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

| :-------- | :-------- | :-------- |
| result | object | |
| result.channel | string | Channel address |
| result.result | integer | Result code @retval ERROR_NONE: Success @retval ERROR_BAD_REQUEST: Bad request |
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The documented result.result return codes for getChannel only list ERROR_NONE and ERROR_BAD_REQUEST, but the interface header declares additional return codes (e.g., ERROR_READ_ERROR). Please ensure the generated markdown reflects the full set of documented @retval values from the header (and apply consistently across the other methods too).

Suggested change
| result.result | integer | Result code @retval ERROR_NONE: Success @retval ERROR_BAD_REQUEST: Bad request |
| result.result | integer | Result code @retval ERROR_NONE: Success @retval ERROR_BAD_REQUEST: Bad request @retval ERROR_READ_ERROR: Read error |

Copilot uses AI. Check for mistakes.

| Event | Description |
| :-------- | :-------- |
| [speedchanged](#speedchanged) | Triggered when the trick play speed has changed |
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The notifications table links to #speedchanged, but the actual section anchor is onSpeedChanged (<a id="onSpeedChanged">). This makes the in-page link broken and also creates inconsistent event naming within the doc (speedchanged vs onSpeedChanged).

Suggested change
| [speedchanged](#speedchanged) | Triggered when the trick play speed has changed |
| [onSpeedChanged](#onSpeedChanged) | Triggered when the trick play speed has changed |

Copilot uses AI. Check for mistakes.
Reason For Change: Adding LinearPlaybackControl interface header Changes
Test procedure : Test LinearPlaybackControl
Priority: P1

Signed-off-by: naveen-0206 <naveenkumar.t@ltts.com>
Copilot AI review requested due to automatic review settings February 9, 2026 10:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment on lines +53 to +57
// @param channel: Channel address
// @param result: Result code
// @retval ERROR_NONE: Success
// @retval ERROR_BAD_REQUEST: Bad request
virtual Core::hresult SetChannel(const string& demuxerId, const string& channel, uint32_t& result /* @out */) = 0;
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The demux/mux identifier type is inconsistent: methods take demuxerId as a string, while the notification uses muxId as a uint8_t. If these represent the same concept, please standardize on one type/name (ideally matching the JSON-RPC index type) to avoid ambiguous conversions and mismatched call sites.

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +32
/* @json 1.0.0 @text:keep */
struct EXTERNAL ILinearPlaybackControl : virtual public Core::IUnknown
{
enum { ID = ID_LINEAR_PLAYBACK_CONTROL };
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The PR deletes apis/LinearPlaybackControl/LinearPlaybackControl.json, but it’s still referenced elsewhere (e.g., tools/md_generator/json/LinearPlaybackControl/LinearPlaybackControlPlugin.json points to {interfacedir}/LinearPlaybackControl.json). This will break doc/interface generation and any JSON-RPC consumers that rely on this interface definition. Either keep the JSON interface alongside the new COM-RPC header, or update all repo references and generated docs accordingly.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants