RDKEMW-8865: Fixing Coding Guideline Errors SAP #572

kkarup694 · 2025-10-15T10:13:44Z

Reason for change: fixing coding guideline errors
Test Procedure: Mentioned in ticket
Risks: Low

Reason for change: fixing coding guideline errors Test Procedure: Mentioned in ticket Risks: Low Signed-off-by: kumar <[email protected]>

CLAassistant · 2025-10-15T10:14:13Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

rebase

Copilot

Pull request overview

This pull request addresses coding guideline violations in the SystemAudioPlayer interface by making two key corrections:

Changed the notification method from pure virtual to default implementation (line 34)
Updated all method return types from uint32_t to Core::hresult (lines 39-53)

Copilot · 2025-12-01T07:26:44Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult Configure(PluginHost::IShell* service) = 0;
+        virtual Core::hresult Register(INotification* sink) = 0;
+        virtual Core::hresult Unregister(INotification* sink) = 0;
+        virtual Core::hresult Open(const string &input, string &output /* @out */) = 0;


Missing required documentation tags for this method. According to the documentation guidelines, all methods must include:

@brief — A concise description of the purpose or behavior

@param — Description of each parameter (for input and output)

@retval — Description of each return code

Example format:

// @brief Opens an audio stream // @param input: Input parameters for opening the stream // @param output: Output result or status information // @retval Core::ERROR_NONE: Success // @retval Core::ERROR_GENERAL: Failed to open stream virtual Core::hresult Open(const string &input, string &output /* @out */) = 0;

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_doc_tags.instructions.md#documentation-tags-guidelines

Copilot · 2025-12-01T07:26:44Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult Play(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult PlayBuffer(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Pause(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Resume(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Stop(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Close(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult SetMixerLevels(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult SetSmartVolControl(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult IsPlaying(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Config(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult GetPlayerSessionId(const string &input, string &output /* @out */) = 0;


Missing required documentation tags for these methods. According to the documentation guidelines, all methods must include:

@brief — A concise description of the purpose or behavior

@param — Description of each parameter (for input and output)

@retval — Description of each return code

These methods need proper documentation to explain their purpose, parameters, and possible return values.

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_doc_tags.instructions.md#documentation-tags-guidelines

Copilot · 2025-12-01T07:26:45Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h


            virtual ~INotification() {}
-            virtual void OnSAPEvents(const string &data) = 0;        
+            virtual void OnSAPEvents(const string &data) {}        


Missing required documentation tags for the event method. According to the documentation guidelines, all events must include:

@brief — A concise description of the purpose or behavior

@param — Description of each parameter (for the data parameter in this case)

Example format:

// @text onSAPEvents // @brief Triggered when SAP events occur // @param data: Event data payload virtual void OnSAPEvents(const string &data) {}

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_doc_tags.instructions.md#documentation-tags-guidelines

Copilot · 2025-12-01T07:26:45Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

-        virtual uint32_t IsPlaying(const string &input, string &output /* @out */) = 0;
-	virtual uint32_t Config(const string &input, string &output /* @out */) = 0;
-        virtual uint32_t GetPlayerSessionId(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Configure(PluginHost::IShell* service) = 0;


Missing required documentation tags for this method. According to the documentation guidelines, all methods must include:

@brief — A concise description of the purpose or behavior

@param — Description of each parameter

@retval — Description of each return code

Example format:

// @brief Configures the SystemAudioPlayer service // @param service: Plugin host shell service pointer // @retval Core::ERROR_NONE: Success // @retval Core::ERROR_GENERAL: Configuration failed virtual Core::hresult Configure(PluginHost::IShell* service) = 0;

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_doc_tags.instructions.md#documentation-tags-guidelines

Copilot · 2025-12-01T07:26:45Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

-	virtual uint32_t Config(const string &input, string &output /* @out */) = 0;
-        virtual uint32_t GetPlayerSessionId(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Configure(PluginHost::IShell* service) = 0;
+        virtual Core::hresult Register(INotification* sink) = 0;


Missing required documentation tags for this method. According to the documentation guidelines, all methods must include:

@brief — A concise description of the purpose or behavior

@param — Description of each parameter

@retval — Description of each return code

Example format:

// @brief Registers a notification callback // @param sink: Notification interface pointer // @retval Core::ERROR_NONE: Success // @retval Core::ERROR_GENERAL: Registration failed virtual Core::hresult Register(INotification* sink) = 0;

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_doc_tags.instructions.md#documentation-tags-guidelines

Copilot · 2025-12-01T07:26:45Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

-        virtual uint32_t GetPlayerSessionId(const string &input, string &output /* @out */) = 0;
+        virtual Core::hresult Configure(PluginHost::IShell* service) = 0;
+        virtual Core::hresult Register(INotification* sink) = 0;
+        virtual Core::hresult Unregister(INotification* sink) = 0;


Missing required documentation tags for this method. According to the documentation guidelines, all methods must include:

@brief — A concise description of the purpose or behavior

@param — Description of each parameter

@retval — Description of each return code

Example format:

// @brief Unregisters a notification callback // @param sink: Notification interface pointer // @retval Core::ERROR_NONE: Success // @retval Core::ERROR_GENERAL: Unregistration failed virtual Core::hresult Unregister(INotification* sink) = 0;

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers_doc_tags.instructions.md#documentation-tags-guidelines

Reason for change: resolving guidelines errors Test Procedure: Mentioned in ticket Risks: Low Signed-off-by: kumar <[email protected]>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 16 comments.

Copilot · 2025-12-02T06:30:25Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult Stop(const string &input, string &output /* @out */) = 0;
+
+        // @text close
+        // @brief closing audio playback


The @brief description should start with a capital letter for consistency with documentation standards. Change "closing audio playback" to "Closing audio playback".

Suggested change

// @brief closing audio playback

// @brief Closing audio playback

Copilot · 2025-12-02T06:30:25Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult SetSmartVolControl(const string &input, string &output /* @out */) = 0;
+
+        // @text isPlaying
+        // @brief get state of audio player


The @brief description should start with a capital letter for consistency with documentation standards. Change "get state of audio player" to "Get state of audio player".

Suggested change

// @brief get state of audio player

// @brief Get state of audio player

Copilot · 2025-12-02T06:30:26Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        // @brief start playback of audio.
+        // @param input: parameters needed for audio playback
+        // @param output: response params
+        // @retval Core::NONE: Indicates successful opening of audio player


The @RetVal description is incorrect. This method is for the Play operation, not for opening the audio player. Change "Indicates successful opening of audio player" to "Indicates successful start of audio playback".

Suggested change

// @retval Core::NONE: Indicates successful opening of audio player

// @retval Core::NONE: Indicates successful start of audio playback

Copilot · 2025-12-02T06:30:26Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult Unregister(INotification* sink) = 0;
+
+        // @text open
+        // @brief open player instance


The @brief description should start with a capital letter for consistency with documentation standards. Change "open player instance" to "Open player instance".

Suggested change

// @brief open player instance

// @brief Open player instance

Copilot · 2025-12-02T06:30:27Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult IsPlaying(const string &input, string &output /* @out */) = 0;
+
+        // @text config
+        // @brief setting audio player configuration


The @brief description should start with a capital letter for consistency with documentation standards. Change "setting audio player configuration" to "Setting audio player configuration".

Suggested change

// @brief setting audio player configuration

// @brief Setting audio player configuration

Copilot · 2025-12-02T06:30:29Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult SetMixerLevels(const string &input, string &output /* @out */) = 0;
+
+        // @text setSmartVolControl
+        // @brief setting smart volume level


The @brief description should start with a capital letter for consistency with documentation standards. Change "setting smart volume level" to "Setting smart volume level".

Suggested change

// @brief setting smart volume level

// @brief Setting smart volume level

Copilot · 2025-12-02T06:30:29Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        // @text isPlaying
+        // @brief get state of audio player
+        // @param input: audio player details
+        // @param output: response params
+        // @retval Core::NONE: Indicates audio player state retrieved successfuly
+        virtual Core::hresult IsPlaying(const string &input, string &output /* @out */) = 0;


According to the getter/setter naming conventions, getter methods should start with "Get". The method name IsPlaying does not follow this convention. Consider renaming to GetPlayingState or similar, with the @text annotation as getPlayingState.

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#gettersetter-naming

Copilot · 2025-12-02T06:30:29Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult Resume(const string &input, string &output /* @out */) = 0;
+
+        // @text stop
+        // @brief stopping audio playback


The @brief description should start with a capital letter for consistency with documentation standards. Change "stopping audio playback" to "Stopping audio playback".

Suggested change

// @brief stopping audio playback

// @brief Stopping audio playback

Copilot · 2025-12-02T06:30:30Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        // @brief get state of audio player
+        // @param input: audio player details
+        // @param output: response params
+        // @retval Core::NONE: Indicates audio player state retrieved successfuly


Spelling error: "successfuly" should be "successfully".

Copilot · 2025-12-02T06:30:30Z

apis/SystemAudioPlayer/ISystemAudioPlayer.h

+        virtual Core::hresult Pause(const string &input, string &output /* @out */) = 0;
+
+        // @text resume
+        // @brief resuming audio playback


The @brief description should start with a capital letter for consistency with documentation standards. Change "resuming audio playback" to "Resuming audio playback".

Suggested change

// @brief resuming audio playback

// @brief Resuming audio playback

rebase

Reason for change: resolving guidelines errors Test Procedure: Mentioned in ticket Risks: Low Signed-off-by: kumar <[email protected]>

RDKEMW-8865: Fixing Coding Guideline Errors SAP

f01749b

Reason for change: fixing coding guideline errors Test Procedure: Mentioned in ticket Risks: Low Signed-off-by: kumar <[email protected]>

kkarup694 requested a review from a team as a code owner October 15, 2025 10:13

kkarup694 added 3 commits October 22, 2025 12:10

Merge pull request #581 from rdkcentral/develop

195bae0

rebase

Merge pull request #651 from rdkcentral/develop

53da4d2

rebase

Merge pull request #655 from rdkcentral/develop

1691625

rebase

suppal045 requested a review from Copilot December 1, 2025 07:24

Copilot started reviewing on behalf of suppal045 December 1, 2025 07:24 View session

Copilot finished reviewing on behalf of suppal045 December 1, 2025 07:26

Copilot AI reviewed Dec 1, 2025

View reviewed changes

RDKEMW-8865: Fixing Coding Guideline Errors

9c4d3a6

Reason for change: resolving guidelines errors Test Procedure: Mentioned in ticket Risks: Low Signed-off-by: kumar <[email protected]>

ssitar583 requested a review from Copilot December 2, 2025 06:26

Copilot started reviewing on behalf of ssitar583 December 2, 2025 06:26 View session

Copilot finished reviewing on behalf of ssitar583 December 2, 2025 06:28

Copilot AI reviewed Dec 2, 2025

View reviewed changes

kkarup694 added 2 commits December 5, 2025 12:18

Merge pull request #664 from rdkcentral/develop

1c0566b

rebase

RDKEMW-8865: Fixing Coding Guideline Errors

b9faf9f

Reason for change: resolving guidelines errors Test Procedure: Mentioned in ticket Risks: Low Signed-off-by: kumar <[email protected]>

	// @brief closing audio playback
	// @brief Closing audio playback

	// @brief get state of audio player
	// @brief Get state of audio player

	// @retval Core::NONE: Indicates successful opening of audio player
	// @retval Core::NONE: Indicates successful start of audio playback

	// @brief open player instance
	// @brief Open player instance

	// @brief setting audio player configuration
	// @brief Setting audio player configuration

	// @brief setting smart volume level
	// @brief Setting smart volume level

	// @brief stopping audio playback
	// @brief Stopping audio playback

	// @brief resuming audio playback
	// @brief Resuming audio playback

RDKEMW-8865: Fixing Coding Guideline Errors SAP #572

Are you sure you want to change the base?

RDKEMW-8865: Fixing Coding Guideline Errors SAP #572

Uh oh!

Conversation

kkarup694 commented Oct 15, 2025

Uh oh!

CLAassistant commented Oct 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants