Skip to content

TEST-123: Test PR for custom coding guidelines #625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ssitar583 wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

TEST-123: Test PR for custom coding guidelines #625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d9c373f
Update IHdcpProfile.h
ssitar583 Nov 12, 2025
12f297b
TEST-123: Update IXCast.h
ssitar583 Nov 12, 2025
575d1d2
Update IXCast.h
ssitar583 Nov 13, 2025
f5512af
Update IMiracastService.h
ssitar583 Nov 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions apis/HdcpProfile/IHdcpProfile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
{
namespace Exchange
{
/* @json 1.0.0 @text:keep */
/* @json 1.0.0*/
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

If an interface implements JSON-RPC and includes the @JSON tag with a version, the interface must also include the @text:keep tag. The @text:keep tag is missing from this line.

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#json-rpc-tagging

Copilot generated this review using guidance from repository custom instructions.
Copy link
Contributor Author

@ssitar583 ssitar583 Nov 12, 2025

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

struct EXTERNAL IHdcpProfile : virtual public Core::IUnknown
{
enum { ID = ID_HDCPPROFILE };
Expand All @@ -45,27 +45,25 @@
{
enum { ID = ID_HDCPPROFILE_NOTIFICATION };

// @text onDisplayConnectionChanged
// @text onChanged
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

Event names SHOULD follow the on[Object][Action] convention and use camelCase for JSON-RPC via @text annotation. The @text annotation onChanged does not follow the naming convention and lacks context. It should be onDisplayConnectionChanged to match the method name pattern.

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

Copilot generated this review using guidance from repository custom instructions.
// @brief Triggered if HDMI was connected or disconnected upon receiving onHdmiOutputHotPlug
// @param HDCPStatus: Contains HDCP-related data as separate properties
virtual void OnDisplayConnectionChanged(const HDCPStatus hdcpStatus/* @text HDCPStatus*/) {};
virtual void ONDisplayConnectionChanged(const HDCPStatus hdcpStatus/* @text HDCPStatus*/) {};
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

Method names SHOULD be in PascalCase in the interface header. The method name ONDisplayConnectionChanged has incorrect casing - it should be OnDisplayConnectionChanged (with lowercase 'n' after 'O').

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#api--event-naming-for-interface-and-json-rpc

Copilot generated this review using guidance from repository custom instructions.
};
virtual Core::hresult Register(IHdcpProfile::INotification* notification) = 0;
virtual Core::hresult Unregister(IHdcpProfile::INotification* notification ) = 0;

/**********************getHDCPStatus() - start****************************/
// @text getHDCPStatus
// @brief Returns HDCP-related data.
// @param HDCPStatus: Contains HDCP-related data as separate properties
// @param success: Indicates whether the operation was successful
virtual Core::hresult GetHDCPStatus(HDCPStatus& hdcpStatus /* @out @text HDCPStatus*/,bool& success /* @out */) = 0;
virtual uint32_t GetHDCPStatus(HDCPStatus& hdcpStatus /* @out @text HDCPStatus*/,bool& success /* @out */) = 0;
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

All methods MUST return Core::hresult. The return type has been changed from Core::hresult to uint32_t, which violates the interface guidelines. This should be reverted to Core::hresult.

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#method-return-type

Copilot generated this review using guidance from repository custom instructions.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

All methods MUST return Core::hresult. The return type uint32_t should be changed to Core::hresult.

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#method-return-type

Copilot generated this review using guidance from repository custom instructions.
/**********************getHDCPStatus() - end******************************/

/**********************getSettopHDCPSupport() - start*********************************/
// @text getSettopHDCPSupport
// @text GetSettopHDCPSupport
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

API names SHOULD be defined in camelCase for JSON-RPC definition via @text annotation. The @text annotation GetSettopHDCPSupport should be getSettopHDCPSupport (first letter must be lowercase).

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#api--event-naming-for-interface-and-json-rpc

Copilot generated this review using guidance from repository custom instructions.
// @brief Returns which version of HDCP is supported by the STB.
// @param supportedHDCPVersion: Supported HDCP protocol version by the host device
// @param isHDCPSupported: Indicates whether HDCP is supported by the STB
// @param success: Indicates whether the operation was successful
virtual Core::hresult GetSettopHDCPSupport(string& supportedHDCPVersion/* @out */,bool& isHDCPSupported/* @out */,bool& success /* @out */) = 0;
/**********************getSettopHDCPSupport() - end***********************************/
Expand Down
6 changes: 1 addition & 5 deletions apis/MiracastService/IMiracastService.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,8 @@ namespace WPEFramework

// @brief To abort the ongoing connection after accepted connection request
// @text stopClientConnection
// @param clientMac: MacAddress of the client device
// @param clientName: Name of the client device
// @param success: Is the operation successful or not
virtual Core::hresult StopClientConnection(const string &clientMac /* @text mac */, const string &clientName /* @text name */, Result &result /* @out */) = 0;

// @brief Update the Miracast Player State to the Miracast Service Plugin
// @text updatePlayerState
// @param clientMac: MacAddress of the client device
// @param playerState: Player state to be updated
Expand All @@ -146,4 +142,4 @@ namespace WPEFramework
virtual Core::hresult SetP2PBackendDiscovery(const bool enabled /* @text enabled */, Result &result /* @out */) = 0;
};
} // namespace Exchange
} // namespace WPEFramework
} // namespace WPEFramework
24 changes: 8 additions & 16 deletions apis/XCast/IXCast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ namespace WPEFramework {

enum StandbyBehavior : uint8_t {
ACTIVE = 0 /* @text active */,
INACTIVE = 1 /* @text inactive */
inactive = 1 /* @text inactive */
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

Enum names SHOULD be in ALL_UPPER_SNAKE_CASE. The enum value inactive should be INACTIVE to maintain consistency with naming conventions.

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

Copilot generated this review using guidance from repository custom instructions.
};

enum ErrorCode : uint16_t {
NONE = 200 /* @text none */ ,
FORBIDDEN = 403 /* @text forbidden */ ,
UNAVAILABLE = 404 /* @text unavailable */ ,
UN-AVAILABLE = 404 /* @text unavailable */ ,
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

Enum names SHOULD NOT contain any special characters other than underscore. The enum value UN-AVAILABLE should be UNAVAILABLE (hyphen is not allowed).

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

Copilot generated this review using guidance from repository custom instructions.
INVALID = 400 /* @text invalid */,
INTERNAL = 500 /* @text internal */
};
Expand Down Expand Up @@ -80,7 +80,7 @@ namespace WPEFramework {
// @brief Triggered when the cast service receives a launch request from a client with launch params
// @param applicationName: Registered application name
// @param parameter: Application launch string
virtual void OnApplicationLaunchRequest(const string& appName /* @text applicationName */ , const string& parameter /* @text parameter */ ) {};
virtual void OnApplicationLaunchRequest(const string& appName /* @text applicationName */ , const string& parameter /* @text parameter */ ) = 0;
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

Notification interfaces MUST provide default (empty) implementations for notification methods. Do NOT use pure virtual methods (= 0). This method should have {} instead of = 0.

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

Copilot generated this review using guidance from repository custom instructions.
// @text onApplicationStopRequest
// @brief Triggered when the cast service receives a stop request from a client
// @param applicationName: Registered application name
Expand All @@ -95,12 +95,12 @@ namespace WPEFramework {
// @brief Triggered when the cast service needs an update of the application state
// @param applicationName: Registered application name
// @param applicationId: Application instance ID
virtual void OnApplicationStateRequest(const string& appName /* @text applicationName */ , const string& appID /* @text applicationId */ ) {};
virtual void OnApplicationStateRequest(const string& appName /* @text ApplicationName */ , const string& appID /* @text applicationId */ ) {};
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

All parameter names in JSON RPC requests/responses SHOULD use camelCase. The @text annotation ApplicationName should be applicationName (first letter must be lowercase).

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

Copilot generated this review using guidance from repository custom instructions.
// @text onApplicationResumeRequest
// @brief Triggered when the cast service receives a resume request from a client
// @param applicationName: Registered application name
// @param applicationId: Application instance ID
virtual void OnApplicationResumeRequest(const string& appName /* @text applicationName */ , const string& appID /* @text applicationId */) {};
virtual void OnApplicationResumeRequest(const string& appName /* @text applicationName */ , const string& AppID /* @text applicationId */) {};
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

All parameter names in JSON RPC requests/responses SHOULD use camelCase. The @text annotation AppID should be applicationId (first letter must be lowercase, and consistent with the parameter naming convention in the documentation).

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

Copilot generated this review using guidance from repository custom instructions.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Parameter @text tag "AppID" violates camelCase naming convention. The first letter must be lowercase. This should be "applicationId".

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

Copilot generated this review using guidance from repository custom instructions.
};

virtual Core::hresult Register(IXCast::INotification* sink ) = 0;
Expand Down Expand Up @@ -130,15 +130,15 @@ namespace WPEFramework {
// @brief Sets the manufacturer name of the device
// @param manufacturer: The Manufacturer name of the device which used to update in dd.xml
// @param success: Whether the request succeeded
virtual Core::hresult SetManufacturerName(const string &manufacturername /* @text manufacturer */, XCastSuccess& success /* @out */) = 0;
virtual Core::hresult SetManufacturerName(const string &manufacturername /* @text @in manufacturer */, XCastSuccess& success /* @out */) = 0;
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

SHOULD NOT use @in for input parameters. The @in tag should be removed. Input parameters are usually defined with const, which already indicates they are input parameters.

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

Copilot generated this review using guidance from repository custom instructions.
/***************************************** setManufacturerName() **********************************/

/****************************************getManufacturerName()**********************************/
// @text getManufacturerName
// @brief Returns the manufacturer name set by setManufacturerName API
// @param manufacturer: The Manufacturer name of the device which used to update in dd.xml
// @param success: Whether the request succeeded
virtual Core::hresult GetManufacturerName(string &manufacturername /* @out @text manufacturer */, bool &success /* @out */) = 0;
virtual Core::hresult GetManufacturerName(string &manufacturername /* @out @text manufacturer */, bool &success) = 0;
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

Use @out tag for output parameters. The success parameter is missing the /* @out */ tag since it's an output parameter passed by reference.

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

Copilot generated this review using guidance from repository custom instructions.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Missing @out tag for the output parameter "success". Output parameters MUST be tagged with @out.

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

Copilot generated this review using guidance from repository custom instructions.
/***************************************** getManufacturerName() *********************************/

/****************************************setModelName()**********************************/
Expand All @@ -158,7 +158,7 @@ namespace WPEFramework {
/***************************************** getModelName() *********************************/

/****************************************setEnabled()**********************************/
// @text setEnabled
// @text Enabled
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

API names SHOULD be defined in camelCase for JSON-RPC definition via @text annotation. The @text annotation Enabled should be setEnabled (first letter lowercase, and method name should match the setter convention).

Refer: https://github.com/rdkcentral/entservices-apis/blob/develop/.github/instructions/api_headers.instructions.md#api--event-naming-for-interface-and-json-rpc

Copilot generated this review using guidance from repository custom instructions.
Copy link
Contributor Author

@ssitar583 ssitar583 Nov 12, 2025

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The @text annotation "Enabled" violates camelCase naming convention for JSON RPC. This should be "setEnabled" to match the setter naming pattern.

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

Copilot generated this review using guidance from repository custom instructions.
// @brief Enable or disable XCAST service
// @parm enabled: true for enabled or false for disabled
// @param success: Whether the request succeeded
Expand All @@ -175,7 +175,6 @@ namespace WPEFramework {

/****************************************setStandbyBehavior()**********************************/
// @text setStandbyBehavior
// @brief Sets the expected xcast behavior in standby mode
// @param standbybehavior: whether to remain active or inactive during standby mode (must be one of the following: active, inactive)
// @param success: Whether the request succeeded
virtual Core::hresult SetStandbyBehavior(const StandbyBehavior &standbybehavior /* @text standbybehavior */, XCastSuccess& success /* @out */) = 0;
Expand All @@ -192,14 +191,11 @@ namespace WPEFramework {
/****************************************setFriendlyName()**********************************/
// @text setFriendlyName
// @brief Sets the friendly name of the device
// @param friendlyname: The friendly name of the device which used to display on the client device list
// @param success: Whether the request succeeded
virtual Core::hresult SetFriendlyName(const string &friendlyname /* @text friendlyname */, XCastSuccess& success /* @out */) = 0;
/***************************************** setFriendlyName() **********************************/

/****************************************getFriendlyName()**********************************/
// @text getFriendlyName
// @brief Returns the friendly name set by setFriendlyName API
// @param friendlyname: The friendly name of the device which used to display on the client device list
// @param success: Whether the request succeeded
virtual Core::hresult GetFriendlyName(string &friendlyname /* @out @text friendlyname */, bool &success /* @out */) = 0;
Expand All @@ -214,10 +210,6 @@ namespace WPEFramework {
/****************************************registerApplications()**********************************/

/****************************************unregisterApplications()**********************************/
// @text unregisterApplications
// @brief Unregisters an application
// @param applications: One or more application name to unregister
// @param success: Whether the request succeeded
virtual Core::hresult UnregisterApplications(IStringIterator* const applications /* @text applications */, XCastSuccess& success /* @out */) = 0;
/****************************************unregisterApplications()**********************************/

Expand Down
Loading