fix(ref): validate device exists before opening commissioning window by Srividhya-23 · Pull Request #212 · rdkcentral/BartonCore

Srividhya-23 · 2026-04-24T06:05:24Z

The ocw command was returning setup codes for non-existent device IDs instead of reporting an error. Add validation to check if the target device exists on the fabric before attempting the operation.

Refs: BARTON-227

Copilot

Pull request overview

This PR fixes the ocw (open commissioning window) CLI command to avoid returning setup codes for non-existent device IDs by validating the target device exists on the fabric before attempting to open its commissioning window (BARTON-227).

Changes:

Added pre-validation for non-local device IDs (!= "0") to ensure the device exists before calling into b_core_client_open_commissioning_window.
Updated commissioning window invocation to pass NULL when no device id argument is provided (local commissioning semantics).
Added a guard to treat “no pairing data returned” (empty manual + QR codes) as an error instead of printing empty results.

The ocw command was returning setup codes for non-existent device IDs instead of reporting an error. Add validation to check if the target device exists on the fabric before attempting the operation. Refs: BARTON-227

Srividhya-23 · 2026-04-24T06:46:45Z

Force pushed a change in commit message in the PR from fix(app) -> fix(ref).

Add proper NULL pointer checks and memory management

Copilot

Pull request overview

Fixes the ocw (open commissioning window) reference CLI behavior to error out when a non-existent device ID is provided, instead of returning pairing codes.

Changes:

Add pre-flight validation that the specified target device exists (except for local device id "0").
Harden argument handling for openCommissioningWindow (argc/argv validation) and pass NULL when no device id is provided.
Add handling for the case where commissioning succeeds but no pairing data is returned.

… variable Agent-Logs-Url: https://github.com/rdkcentral/BartonCore/sessions/0c3abc59-8df1-4e16-a387-f347161a266a Co-authored-by: Srividhya-23 <182346403+Srividhya-23@users.noreply.github.com>

cleithner-comcast · 2026-04-24T15:53:51Z

+     * the device exists before attempting to open its commissioning window. */
+    if (argc >= 1 && strcmp(argv[0], "0") != 0)
+    {
+        g_autoptr(BCoreDevice) device = b_core_client_get_device_by_id(client, argv[0]);


You may double check that this is a reliable pass-through for node id. If I remember correctly, matter device uuid is node id with zero padding. I'm pretty sure this API doesn't require a zero-padded input but I'm not 100% sure.

You're correct, b_core_client_get_device_by_id does lookup based on uuid which is node id with zero padding in this case. Updated changes.

Ensures consistent device lookup by normalizing user input node id to stored UUID format.

Copilot

Pull request overview

This PR updates the reference CLI ocw (open commissioning window) command to avoid returning setup codes for non-existent Matter device IDs by validating the target device exists on the fabric before invoking the commissioning-window operation (BARTON-227).

Changes:

Add argument validation and a pre-flight device existence check for non-local node IDs (with node-id normalization for lookup).
Switch commissioning info handling to g_autoptr for automatic cleanup.
Improve output/error handling when commissioning window opens but pairing data is missing.

Parse and validate node IDs, normalize to 16-char format before device lookup. Add timeout range validation for ocw command.

tleacmcsa · 2026-04-27T13:04:56Z

+    g_return_val_if_fail(argc >= 0 && argc <= 2, false);
+    g_return_val_if_fail(argc == 0 || argv != NULL, false);
+    g_return_val_if_fail(argc < 1 || argv[0] != NULL, false);
+    g_return_val_if_fail(argc < 2 || argv[1] != NULL, false);


Some of these checks are redundant with commandExecute() (the argc checks) and the others should be moved to commandExecute to validate args for all commands (the argv null checks). The point of the command interface is that commands are added with their requirements and those requirements should be validated before the command's function is executed. This contract prevents checks like these from being either duplicated everywhere or missed in some places.

Please add the argv checks to commandExecute and add a comment to commandExecute in command.h which states that arguments are validated before execution.

Moved all argc/argv validation to commandExecute.

tleacmcsa · 2026-04-27T13:05:43Z

+    /* If a target device id was provided, parse and validate it.
+     * Treat nodeId == 0 as local.
+     * Otherwise, verify the device exists after normalizing the ID. */
+    if (argc >= 1)


this is always true. the command has minArgs of 1.

tleacmcsa · 2026-04-27T13:10:43Z

+            g_autofree gchar *normalizedId =
+                g_strdup_printf("%016llx", nodeId);
+
+            g_autoptr(BCoreDevice) device =


This exposes an internal detail/assumption to the client (this example/reference app). Currently we do use Matter node ids for our device ids, but that is not an external contract and could change. I think it is fine to leave it here in this reference code for now, but please add a comment that this may not be a reliable way to do the lookup if the internal mapping between Matter node id and Barton device ids change.

tleacmcsa · 2026-04-27T13:19:50Z

+    g_autoptr(BCoreCommissioningInfo) commissioningInfo =
+        b_core_client_open_commissioning_window(
+            client,
+            (argc >= 1 ? argv[0] : NULL),


this argc check is unnecessary. Also, should this be argv[0] or normalizedId?

We should be using normalized Id, changed it.

tleacmcsa · 2026-04-27T13:22:22Z

+            const gchar *manualCodeOutput = manualCode != NULL ? manualCode : "";
+            const gchar *qrCodeOutput = qrCode != NULL ? qrCode : "";


duplicated null checks.

Move argc/argv validation from individual commands to commandExecute() to eliminate duplication and ensure consistency. Remove redundant null checks and simplify output handling in openCommissioningWindow function. Signed-off-by: ss411 <srividhya_s@comcast.com>

Copilot

Pull request overview

This PR addresses an issue in the reference CLI where ocw (open commissioning window) could return pairing codes for non-existent device IDs by adding device existence validation before attempting to open the commissioning window.

Changes:

Add node-id parsing/validation and a “device exists” check before opening a commissioning window.
Add timeout parsing/validation and extra validation around returned pairing data.
Centralize basic argv/argc validation in commandExecute and document the behavior.

Reviewed changes

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

File	Description
reference/src/matterCategory.c	Adds validation for `ocw` inputs and checks the target device exists before opening a commissioning window.
reference/src/command.c	Adds a guard to avoid calling command functions with `argc > 0` but `argv == NULL`.
reference/src/command.h	Documents the new argument validation behavior for `commandExecute`.

Add missing return statement in setWifiCredsFunc. Support both decimal and hex (0x prefix) formats for node IDs in openCommissioningWindow.

Copilot AI review requested due to automatic review settings April 24, 2026 06:05

Srividhya-23 requested review from cleithner-comcast, kfundecmcsa, mkkoch, rchowdcmcsa and tleacmcsa as code owners April 24, 2026 06:05

Copilot started reviewing on behalf of Srividhya-23 April 24, 2026 06:06 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread reference/src/matterCategory.c Outdated

Comment thread reference/src/matterCategory.c Outdated

Comment thread reference/src/matterCategory.c Outdated

Srividhya-23 changed the title ~~fix(app): validate device exists before opening commissioning window~~ fix(ref): validate device exists before opening commissioning window Apr 24, 2026

fix(ref): validate device exists before opening commissioning window

bf165f2

The ocw command was returning setup codes for non-existent device IDs instead of reporting an error. Add validation to check if the target device exists on the fabric before attempting the operation. Refs: BARTON-227

Srividhya-23 force-pushed the ss411/dev/BARTON-227 branch from 246243f to bf165f2 Compare April 24, 2026 06:29

fix(ref): add safe checks to openCommissioningWindow

6fa4bff

Add proper NULL pointer checks and memory management

Copilot AI review requested due to automatic review settings April 24, 2026 06:54

Copilot started reviewing on behalf of Srividhya-23 April 24, 2026 06:54 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread reference/src/matterCategory.c Outdated

Copilot started work on behalf of Srividhya-23 April 24, 2026 08:30 View session

fix(ref): use g_autoptr directly on commissioningInfo to avoid unused…

a8f726a

… variable Agent-Logs-Url: https://github.com/rdkcentral/BartonCore/sessions/0c3abc59-8df1-4e16-a387-f347161a266a Co-authored-by: Srividhya-23 <182346403+Srividhya-23@users.noreply.github.com>

Copilot finished work on behalf of Srividhya-23 April 24, 2026 08:32

cleithner-comcast previously approved these changes Apr 24, 2026

View reviewed changes

kfundecmcsa previously approved these changes Apr 24, 2026

View reviewed changes

fix(ref): normalize Matter node IDs before device lookup

f62af01

Ensures consistent device lookup by normalizing user input node id to stored UUID format.

Copilot AI review requested due to automatic review settings April 24, 2026 17:06

Srividhya-23 dismissed stale reviews from kfundecmcsa and cleithner-comcast via f62af01 April 24, 2026 17:06

Copilot started reviewing on behalf of Srividhya-23 April 24, 2026 17:06 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread reference/src/matterCategory.c Outdated

Srividhya-23 requested review from cleithner-comcast and kfundecmcsa April 24, 2026 17:27

fix(ref): validate node IDs and normalize before device lookup

96b179d

Parse and validate node IDs, normalize to 16-char format before device lookup. Add timeout range validation for ocw command.

tleacmcsa requested changes Apr 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 27, 2026 15:22

Copilot started reviewing on behalf of Srividhya-23 April 27, 2026 15:22 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread reference/src/matterCategory.c

Comment thread reference/src/matterCategory.c

fix(ref): add missing return and flexible node ID parsing

c815db8

Add missing return statement in setWifiCredsFunc. Support both decimal and hex (0x prefix) formats for node IDs in openCommissioningWindow.

Srividhya-23 requested a review from tleacmcsa April 27, 2026 16:07

		const gchar *manualCodeOutput = manualCode != NULL ? manualCode : "";
		const gchar *qrCodeOutput = qrCode != NULL ? qrCode : "";

Conversation

Srividhya-23 commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Srividhya-23 commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants