From 247ea4d09d908425ac73be5b938f6895e53ce548 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 27 Aug 2024 09:36:30 -0400 Subject: [PATCH 1/8] Initial work --- .../protos/fabric_bridge_service.proto | 12 ++++++ .../pigweed/rpc_services/FabricBridge.h | 5 +++ .../include/BridgedDevice.h | 3 +- .../src/BridgedAdministratorCommissioning.cpp | 2 +- .../src/BridgedDevice.cpp | 27 +++++++++++++ .../fabric-bridge-app/linux/RpcServer.cpp | 38 +++++++++++++++++++ 6 files changed, 85 insertions(+), 2 deletions(-) diff --git a/examples/common/pigweed/protos/fabric_bridge_service.proto b/examples/common/pigweed/protos/fabric_bridge_service.proto index 0de3b3fa243d77..06845b347bb24b 100644 --- a/examples/common/pigweed/protos/fabric_bridge_service.proto +++ b/examples/common/pigweed/protos/fabric_bridge_service.proto @@ -26,8 +26,20 @@ message KeepActiveChanged { uint32 promised_active_duration_ms = 2; } +message AdministratorCommissioningChange { + uint64 node_id = 1; + // TODO figure out the types we should use here + //uint8 window_status = 2; + //optional uint8 admin_fabric_index = 3; + //optional uint16 admin_vendor_id = 4; + uint32 window_status = 2; + optional uint32 opener_fabric_index = 3; + optional uint32 opener_vendor_id = 4; +} + service FabricBridge { rpc AddSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){} rpc RemoveSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){} rpc ActiveChanged(KeepActiveChanged) returns (pw.protobuf.Empty){} + rpc AdminCommissioningAttributeChanged(AdministratorCommissioningChange) returns (pw.protobuf.Empty){} } diff --git a/examples/common/pigweed/rpc_services/FabricBridge.h b/examples/common/pigweed/rpc_services/FabricBridge.h index aab714223968df..2939fe9b6e38b9 100644 --- a/examples/common/pigweed/rpc_services/FabricBridge.h +++ b/examples/common/pigweed/rpc_services/FabricBridge.h @@ -48,6 +48,11 @@ class FabricBridge : public pw_rpc::nanopb::FabricBridge::Service { return pw::Status::Unimplemented(); } + + virtual pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChange & request, pw_protobuf_Empty & response) + { + return pw::Status::Unimplemented(); + } }; } // namespace rpc diff --git a/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h b/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h index 7081278f4dc0f3..0de35ecbfa32e8 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h +++ b/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h @@ -68,7 +68,8 @@ class BridgedDevice [[nodiscard]] const BridgedAttributes & GetBridgedAttributes() const { return mAttributes; } void SetBridgedAttributes(const BridgedAttributes & value) { mAttributes = value; } - // TODO(#35077): Need to allow mAdminCommissioningAttributes to be set from fabric-admin. + + void SetAdminCommissioningAttributes(const AdminCommissioningAttributes & aAdminCommissioningAttributes); const AdminCommissioningAttributes & GetAdminCommissioningAttributes() const { return mAdminCommissioningAttributes; } /// Convenience method to set just the unique id of a bridged device as it diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp index a0d87cbb3b81ef..57e9faf894807e 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp @@ -55,7 +55,7 @@ CHIP_ERROR BridgedAdministratorCommissioning::Read(const ConcreteReadAttributePa switch (aPath.mAttributeId) { case Attributes::WindowStatus::Id: { - return aEncoder.Encode(attr.commissioningWindowStatus); + return aEncoder.Encode(attr.commissioningWindowStatus ); } case Attributes::AdminFabricIndex::Id: { DataModel::Nullable encodeableFabricIndex = DataModel::NullNullable; diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp index 21f19189afefe6..dff9800d0f8cf9 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp @@ -21,6 +21,7 @@ #include #include +#include #include namespace { @@ -80,3 +81,29 @@ void BridgedDevice::SetReachable(bool reachable) ChipLogProgress(NotSpecified, "BridgedDevice[%s]: OFFLINE", mAttributes.uniqueId.c_str()); } } + +void BridgedDevice::SetAdminCommissioningAttributes(const AdminCommissioningAttributes & aAdminCommissioningAttributes) +{ + bool window_changed = (aAdminCommissioningAttributes.commissioningWindowStatus != mAdminCommissioningAttributes.commissioningWindowStatus); + bool fabric_index_changed = (aAdminCommissioningAttributes.openerFabricIndex != mAdminCommissioningAttributes.openerFabricIndex); + bool vendor_changed = (aAdminCommissioningAttributes.openerVendorId != mAdminCommissioningAttributes.openerVendorId); + + mAdminCommissioningAttributes = aAdminCommissioningAttributes; + + if (window_changed) + { + MatterReportingAttributeChangeCallback(mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::WindowStatus::Id); + } + if (fabric_index_changed) + { + MatterReportingAttributeChangeCallback(mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::AdminFabricIndex::Id); + } + + if (vendor_changed) + { + MatterReportingAttributeChangeCallback(mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::AdminVendorId::Id); + } +} diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp index bae007ed484935..318e99cdfcebcd 100644 --- a/examples/fabric-bridge-app/linux/RpcServer.cpp +++ b/examples/fabric-bridge-app/linux/RpcServer.cpp @@ -46,6 +46,7 @@ class FabricBridge final : public chip::rpc::FabricBridge pw::Status AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status ActiveChanged(const chip_rpc_KeepActiveChanged & request, pw_protobuf_Empty & response) override; + pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChange & request, pw_protobuf_Empty & response) override; }; pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) @@ -160,6 +161,43 @@ pw::Status FabricBridge::ActiveChanged(const chip_rpc_KeepActiveChanged & reques return pw::OkStatus(); } +pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChange & request, pw_protobuf_Empty & response) +{ + NodeId nodeId = request.node_id; + ChipLogProgress(NotSpecified, "Received CADMIN attribut change: " ChipLogFormatX64, ChipLogValueX64(nodeId)); + + auto * device = BridgeDeviceMgr().GetDeviceByNodeId(nodeId); + if (device == nullptr) + { + ChipLogError(NotSpecified, "Could not find bridged device associated with nodeId=0x" ChipLogFormatX64, + ChipLogValueX64(nodeId)); + return pw::Status::NotFound(); + } + + BridgedDevice::AdminCommissioningAttributes adminCommissioningAttributes; + + // TODO these casts are all hacks just place holder until I figure out proto types + adminCommissioningAttributes.commissioningWindowStatus = static_cast(request.window_status); + if (request.has_opener_fabric_index) { + adminCommissioningAttributes.openerFabricIndex = static_cast(request.opener_fabric_index); + } + else + { + adminCommissioningAttributes.openerFabricIndex = std::nullopt; + } + + if (request.has_opener_vendor_id) { + adminCommissioningAttributes.openerVendorId = static_cast(request.opener_vendor_id); + } + else + { + adminCommissioningAttributes.openerVendorId = std::nullopt; + } + + device->SetAdminCommissioningAttributes(adminCommissioningAttributes); + return pw::OkStatus(); +} + FabricBridge fabric_bridge_service; #endif // defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE From e360c5cc07551eb5c43c5edb5ea89535881239f7 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 27 Aug 2024 14:31:00 +0000 Subject: [PATCH 2/8] Clean up TODOs --- .../pigweed/protos/fabric_bridge_service.proto | 4 ---- examples/fabric-bridge-app/linux/RpcServer.cpp | 13 +++++++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/examples/common/pigweed/protos/fabric_bridge_service.proto b/examples/common/pigweed/protos/fabric_bridge_service.proto index 06845b347bb24b..95d6775bed50da 100644 --- a/examples/common/pigweed/protos/fabric_bridge_service.proto +++ b/examples/common/pigweed/protos/fabric_bridge_service.proto @@ -28,10 +28,6 @@ message KeepActiveChanged { message AdministratorCommissioningChange { uint64 node_id = 1; - // TODO figure out the types we should use here - //uint8 window_status = 2; - //optional uint8 admin_fabric_index = 3; - //optional uint16 admin_vendor_id = 4; uint32 window_status = 2; optional uint32 opener_fabric_index = 3; optional uint32 opener_vendor_id = 4; diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp index 318e99cdfcebcd..dc8a6c38b52a17 100644 --- a/examples/fabric-bridge-app/linux/RpcServer.cpp +++ b/examples/fabric-bridge-app/linux/RpcServer.cpp @@ -176,22 +176,27 @@ pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_Admin BridgedDevice::AdminCommissioningAttributes adminCommissioningAttributes; - // TODO these casts are all hacks just place holder until I figure out proto types + uint32_t max_window_status_value = static_cast(chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum::kUnknownEnumValue); + VerifyOrReturnValue(request.window_status < max_window_status_value, pw::Status::InvalidArgument()); adminCommissioningAttributes.commissioningWindowStatus = static_cast(request.window_status); - if (request.has_opener_fabric_index) { + if (request.has_opener_fabric_index) + { + VerifyOrReturnValue(request.opener_fabric_index >= chip::kMinValidFabricIndex, pw::Status::InvalidArgument()); + VerifyOrReturnValue(request.opener_fabric_index <= chip::kMaxValidFabricIndex, pw::Status::InvalidArgument()); adminCommissioningAttributes.openerFabricIndex = static_cast(request.opener_fabric_index); } else { - adminCommissioningAttributes.openerFabricIndex = std::nullopt; + adminCommissioningAttributes.openerFabricIndex.reset(); } if (request.has_opener_vendor_id) { + VerifyOrReturnValue(request.opener_vendor_id != chip::VendorId::NotSpecified, pw::Status::InvalidArgument()); adminCommissioningAttributes.openerVendorId = static_cast(request.opener_vendor_id); } else { - adminCommissioningAttributes.openerVendorId = std::nullopt; + adminCommissioningAttributes.openerVendorId.reset(); } device->SetAdminCommissioningAttributes(adminCommissioningAttributes); From d9c5a614f3ffbbf0aa9b98e784cd7b312d59cf9b Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 27 Aug 2024 15:22:36 +0000 Subject: [PATCH 3/8] Finishing touches --- .../pigweed/protos/fabric_bridge_service.proto | 4 ++-- .../common/pigweed/rpc_services/FabricBridge.h | 2 +- examples/fabric-admin/rpc/RpcClient.cpp | 17 +++++++++++++++++ examples/fabric-admin/rpc/RpcClient.h | 13 ++++++++++++- examples/fabric-bridge-app/linux/RpcServer.cpp | 4 ++-- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/examples/common/pigweed/protos/fabric_bridge_service.proto b/examples/common/pigweed/protos/fabric_bridge_service.proto index 95d6775bed50da..1c699e6942b358 100644 --- a/examples/common/pigweed/protos/fabric_bridge_service.proto +++ b/examples/common/pigweed/protos/fabric_bridge_service.proto @@ -26,7 +26,7 @@ message KeepActiveChanged { uint32 promised_active_duration_ms = 2; } -message AdministratorCommissioningChange { +message AdministratorCommissioningChanged { uint64 node_id = 1; uint32 window_status = 2; optional uint32 opener_fabric_index = 3; @@ -37,5 +37,5 @@ service FabricBridge { rpc AddSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){} rpc RemoveSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){} rpc ActiveChanged(KeepActiveChanged) returns (pw.protobuf.Empty){} - rpc AdminCommissioningAttributeChanged(AdministratorCommissioningChange) returns (pw.protobuf.Empty){} + rpc AdminCommissioningAttributeChanged(AdministratorCommissioningChanged) returns (pw.protobuf.Empty){} } diff --git a/examples/common/pigweed/rpc_services/FabricBridge.h b/examples/common/pigweed/rpc_services/FabricBridge.h index 2939fe9b6e38b9..90b084edd5625c 100644 --- a/examples/common/pigweed/rpc_services/FabricBridge.h +++ b/examples/common/pigweed/rpc_services/FabricBridge.h @@ -49,7 +49,7 @@ class FabricBridge : public pw_rpc::nanopb::FabricBridge::Service return pw::Status::Unimplemented(); } - virtual pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChange & request, pw_protobuf_Empty & response) + virtual pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChanged & request, pw_protobuf_Empty & response) { return pw::Status::Unimplemented(); } diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp index f5672fc0bd8f85..29fc2b1ea9d73c 100644 --- a/examples/fabric-admin/rpc/RpcClient.cpp +++ b/examples/fabric-admin/rpc/RpcClient.cpp @@ -180,3 +180,20 @@ CHIP_ERROR ActiveChanged(chip::NodeId nodeId, uint32_t promisedActiveDurationMs) return WaitForResponse(call); } + +CHIP_ERROR AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & data) +{ + ChipLogProgress(NotSpecified, "AdminCommissioningAttributeChanged"); + + // The RPC call is kept alive until it completes. When a response is received, it will be logged by the handler + // function and the call will complete. + auto call = fabricBridgeClient.AdminCommissioningAttributeChanged(data, RpcCompletedWithEmptyResponse); + + if (!call.active()) + { + // The RPC call was not sent. This could occur due to, for example, an invalid channel ID. Handle if necessary. + return CHIP_ERROR_INTERNAL; + } + + return WaitForResponse(call); +} diff --git a/examples/fabric-admin/rpc/RpcClient.h b/examples/fabric-admin/rpc/RpcClient.h index 8edee9a3b256ed..8f0ee88c419b8c 100644 --- a/examples/fabric-admin/rpc/RpcClient.h +++ b/examples/fabric-admin/rpc/RpcClient.h @@ -55,7 +55,7 @@ CHIP_ERROR AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & data); * It logs the progress and checks if a `RemoveSynchronizedDevice` operation is already in progress. * If an operation is in progress, it returns `CHIP_ERROR_BUSY`. * - * @param nodeId The Node ID of the device to be removed. + * @param data The Node ID of the device to be removed. * @return CHIP_ERROR An error code indicating the success or failure of the operation. * - CHIP_NO_ERROR: The RPC command was successfully processed. * - CHIP_ERROR_BUSY: Another operation is currently in progress. @@ -74,3 +74,14 @@ CHIP_ERROR RemoveSynchronizedDevice(chip::NodeId nodeId); * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call. */ CHIP_ERROR ActiveChanged(chip::NodeId nodeId, uint32_t promisedActiveDurationMs); + +/** + * @brief CADMIN attribute has changed of one of the bridged devices that was previously added. + * + * @param data information regarding change in AdministratorCommissioning attributes + * @return CHIP_ERROR An error code indicating the success or failure of the operation. + * - CHIP_NO_ERROR: The RPC command was successfully processed. + * - CHIP_ERROR_BUSY: Another operation is currently in progress. + * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call. + */ +CHIP_ERROR AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & data); diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp index dc8a6c38b52a17..d4d5d315e5b506 100644 --- a/examples/fabric-bridge-app/linux/RpcServer.cpp +++ b/examples/fabric-bridge-app/linux/RpcServer.cpp @@ -46,7 +46,7 @@ class FabricBridge final : public chip::rpc::FabricBridge pw::Status AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status ActiveChanged(const chip_rpc_KeepActiveChanged & request, pw_protobuf_Empty & response) override; - pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChange & request, pw_protobuf_Empty & response) override; + pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChanged & request, pw_protobuf_Empty & response) override; }; pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) @@ -161,7 +161,7 @@ pw::Status FabricBridge::ActiveChanged(const chip_rpc_KeepActiveChanged & reques return pw::OkStatus(); } -pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChange & request, pw_protobuf_Empty & response) +pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, pw_protobuf_Empty & response) { NodeId nodeId = request.node_id; ChipLogProgress(NotSpecified, "Received CADMIN attribut change: " ChipLogFormatX64, ChipLogValueX64(nodeId)); From 276a3f9270e22b444d549fedf5dcbb21f718aa6e Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 27 Aug 2024 15:25:59 +0000 Subject: [PATCH 4/8] Restyled by clang-format --- .../common/pigweed/rpc_services/FabricBridge.h | 3 ++- .../src/BridgedAdministratorCommissioning.cpp | 2 +- .../fabric-bridge-common/src/BridgedDevice.cpp | 6 ++++-- examples/fabric-bridge-app/linux/RpcServer.cpp | 15 ++++++++++----- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/examples/common/pigweed/rpc_services/FabricBridge.h b/examples/common/pigweed/rpc_services/FabricBridge.h index 90b084edd5625c..9bd520278c13b4 100644 --- a/examples/common/pigweed/rpc_services/FabricBridge.h +++ b/examples/common/pigweed/rpc_services/FabricBridge.h @@ -49,7 +49,8 @@ class FabricBridge : public pw_rpc::nanopb::FabricBridge::Service return pw::Status::Unimplemented(); } - virtual pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChanged & request, pw_protobuf_Empty & response) + virtual pw::Status AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, + pw_protobuf_Empty & response) { return pw::Status::Unimplemented(); } diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp index 57e9faf894807e..a0d87cbb3b81ef 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedAdministratorCommissioning.cpp @@ -55,7 +55,7 @@ CHIP_ERROR BridgedAdministratorCommissioning::Read(const ConcreteReadAttributePa switch (aPath.mAttributeId) { case Attributes::WindowStatus::Id: { - return aEncoder.Encode(attr.commissioningWindowStatus ); + return aEncoder.Encode(attr.commissioningWindowStatus); } case Attributes::AdminFabricIndex::Id: { DataModel::Nullable encodeableFabricIndex = DataModel::NullNullable; diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp index dff9800d0f8cf9..178f5b0734059f 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp @@ -84,8 +84,10 @@ void BridgedDevice::SetReachable(bool reachable) void BridgedDevice::SetAdminCommissioningAttributes(const AdminCommissioningAttributes & aAdminCommissioningAttributes) { - bool window_changed = (aAdminCommissioningAttributes.commissioningWindowStatus != mAdminCommissioningAttributes.commissioningWindowStatus); - bool fabric_index_changed = (aAdminCommissioningAttributes.openerFabricIndex != mAdminCommissioningAttributes.openerFabricIndex); + bool window_changed = + (aAdminCommissioningAttributes.commissioningWindowStatus != mAdminCommissioningAttributes.commissioningWindowStatus); + bool fabric_index_changed = + (aAdminCommissioningAttributes.openerFabricIndex != mAdminCommissioningAttributes.openerFabricIndex); bool vendor_changed = (aAdminCommissioningAttributes.openerVendorId != mAdminCommissioningAttributes.openerVendorId); mAdminCommissioningAttributes = aAdminCommissioningAttributes; diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp index d4d5d315e5b506..3ca7ccd4cb2d4a 100644 --- a/examples/fabric-bridge-app/linux/RpcServer.cpp +++ b/examples/fabric-bridge-app/linux/RpcServer.cpp @@ -46,7 +46,8 @@ class FabricBridge final : public chip::rpc::FabricBridge pw::Status AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status ActiveChanged(const chip_rpc_KeepActiveChanged & request, pw_protobuf_Empty & response) override; - pw::Status AdminCommissioningAttributeChanged (const chip_rpc_AdministratorCommissioningChanged & request, pw_protobuf_Empty & response) override; + pw::Status AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, + pw_protobuf_Empty & response) override; }; pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) @@ -161,7 +162,8 @@ pw::Status FabricBridge::ActiveChanged(const chip_rpc_KeepActiveChanged & reques return pw::OkStatus(); } -pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, pw_protobuf_Empty & response) +pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, + pw_protobuf_Empty & response) { NodeId nodeId = request.node_id; ChipLogProgress(NotSpecified, "Received CADMIN attribut change: " ChipLogFormatX64, ChipLogValueX64(nodeId)); @@ -176,9 +178,11 @@ pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_Admin BridgedDevice::AdminCommissioningAttributes adminCommissioningAttributes; - uint32_t max_window_status_value = static_cast(chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum::kUnknownEnumValue); + uint32_t max_window_status_value = + static_cast(chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum::kUnknownEnumValue); VerifyOrReturnValue(request.window_status < max_window_status_value, pw::Status::InvalidArgument()); - adminCommissioningAttributes.commissioningWindowStatus = static_cast(request.window_status); + adminCommissioningAttributes.commissioningWindowStatus = + static_cast(request.window_status); if (request.has_opener_fabric_index) { VerifyOrReturnValue(request.opener_fabric_index >= chip::kMinValidFabricIndex, pw::Status::InvalidArgument()); @@ -190,7 +194,8 @@ pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_Admin adminCommissioningAttributes.openerFabricIndex.reset(); } - if (request.has_opener_vendor_id) { + if (request.has_opener_vendor_id) + { VerifyOrReturnValue(request.opener_vendor_id != chip::VendorId::NotSpecified, pw::Status::InvalidArgument()); adminCommissioningAttributes.openerVendorId = static_cast(request.opener_vendor_id); } From 1103148bcd64eb78dd7bf7f7dffacae76da4c9e0 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 27 Aug 2024 15:36:20 +0000 Subject: [PATCH 5/8] Self-review --- examples/fabric-admin/rpc/RpcClient.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/fabric-admin/rpc/RpcClient.h b/examples/fabric-admin/rpc/RpcClient.h index 8f0ee88c419b8c..6dd2b5b20bbd90 100644 --- a/examples/fabric-admin/rpc/RpcClient.h +++ b/examples/fabric-admin/rpc/RpcClient.h @@ -55,7 +55,7 @@ CHIP_ERROR AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & data); * It logs the progress and checks if a `RemoveSynchronizedDevice` operation is already in progress. * If an operation is in progress, it returns `CHIP_ERROR_BUSY`. * - * @param data The Node ID of the device to be removed. + * @param nodeId The Node ID of the device to be removed. * @return CHIP_ERROR An error code indicating the success or failure of the operation. * - CHIP_NO_ERROR: The RPC command was successfully processed. * - CHIP_ERROR_BUSY: Another operation is currently in progress. From 76e0b56c29cbd00a86a31a2850409eb34f282484 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 27 Aug 2024 21:45:24 +0000 Subject: [PATCH 6/8] Address PR comments --- examples/fabric-bridge-app/linux/RpcServer.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp index 3ca7ccd4cb2d4a..bc8eed5c3dd9f0 100644 --- a/examples/fabric-bridge-app/linux/RpcServer.cpp +++ b/examples/fabric-bridge-app/linux/RpcServer.cpp @@ -189,20 +189,12 @@ pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_Admin VerifyOrReturnValue(request.opener_fabric_index <= chip::kMaxValidFabricIndex, pw::Status::InvalidArgument()); adminCommissioningAttributes.openerFabricIndex = static_cast(request.opener_fabric_index); } - else - { - adminCommissioningAttributes.openerFabricIndex.reset(); - } if (request.has_opener_vendor_id) { VerifyOrReturnValue(request.opener_vendor_id != chip::VendorId::NotSpecified, pw::Status::InvalidArgument()); adminCommissioningAttributes.openerVendorId = static_cast(request.opener_vendor_id); } - else - { - adminCommissioningAttributes.openerVendorId.reset(); - } device->SetAdminCommissioningAttributes(adminCommissioningAttributes); return pw::OkStatus(); From 4d48625ea4a05bca34664b9c3ecb930dcafb60a0 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 28 Aug 2024 00:27:39 +0000 Subject: [PATCH 7/8] Fix after more local testing --- .../src/BridgedDevice.cpp | 57 ++++++++++++------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp index 178f5b0734059f..58960ddd51089f 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp @@ -32,6 +32,14 @@ struct ActiveChangeEventWorkData uint32_t mPromisedActiveDuration; }; +struct ReportAttributeChangedWorkData +{ + chip::EndpointId mEndpointId; + bool mWindowChanged = false; + bool mFabricIndexChanged = false; + bool mVendorChanged = false; +}; + void ActiveChangeEventWork(intptr_t arg) { ActiveChangeEventWorkData * data = reinterpret_cast(arg); @@ -48,6 +56,28 @@ void ActiveChangeEventWork(intptr_t arg) chip::Platform::Delete(data); } +void ReportAttributeChangedWork(intptr_t arg) +{ + ReportAttributeChangedWorkData * data = reinterpret_cast(arg); + + if (data->mWindowChanged) + { + MatterReportingAttributeChangeCallback(data->mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::WindowStatus::Id); + } + if (data->mFabricIndexChanged) + { + MatterReportingAttributeChangeCallback(data->mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::AdminFabricIndex::Id); + } + if (data->mVendorChanged) + { + MatterReportingAttributeChangeCallback(data->mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::AdminVendorId::Id); + } + chip::Platform::Delete(data); +} + } // namespace using namespace chip::app::Clusters::Actions; @@ -84,28 +114,15 @@ void BridgedDevice::SetReachable(bool reachable) void BridgedDevice::SetAdminCommissioningAttributes(const AdminCommissioningAttributes & aAdminCommissioningAttributes) { - bool window_changed = + ReportAttributeChangedWorkData * workdata = chip::Platform::New(); + + workdata->mEndpointId = mEndpointId; + workdata->mWindowChanged = (aAdminCommissioningAttributes.commissioningWindowStatus != mAdminCommissioningAttributes.commissioningWindowStatus); - bool fabric_index_changed = + workdata->mFabricIndexChanged = (aAdminCommissioningAttributes.openerFabricIndex != mAdminCommissioningAttributes.openerFabricIndex); - bool vendor_changed = (aAdminCommissioningAttributes.openerVendorId != mAdminCommissioningAttributes.openerVendorId); + workdata->mVendorChanged = (aAdminCommissioningAttributes.openerVendorId != mAdminCommissioningAttributes.openerVendorId); mAdminCommissioningAttributes = aAdminCommissioningAttributes; - - if (window_changed) - { - MatterReportingAttributeChangeCallback(mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, - chip::app::Clusters::AdministratorCommissioning::Attributes::WindowStatus::Id); - } - if (fabric_index_changed) - { - MatterReportingAttributeChangeCallback(mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, - chip::app::Clusters::AdministratorCommissioning::Attributes::AdminFabricIndex::Id); - } - - if (vendor_changed) - { - MatterReportingAttributeChangeCallback(mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, - chip::app::Clusters::AdministratorCommissioning::Attributes::AdminVendorId::Id); - } + chip::DeviceLayer::PlatformMgr().ScheduleWork(ReportAttributeChangedWork, reinterpret_cast(workdata)); } From d7209a52506adc29c512e1c1a1191fb496e8cf25 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 28 Aug 2024 00:30:11 +0000 Subject: [PATCH 8/8] Restyled by clang-format --- .../fabric-bridge-common/src/BridgedDevice.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp index 58960ddd51089f..27364976d121f5 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp @@ -35,9 +35,9 @@ struct ActiveChangeEventWorkData struct ReportAttributeChangedWorkData { chip::EndpointId mEndpointId; - bool mWindowChanged = false; + bool mWindowChanged = false; bool mFabricIndexChanged = false; - bool mVendorChanged = false; + bool mVendorChanged = false; }; void ActiveChangeEventWork(intptr_t arg) @@ -114,7 +114,7 @@ void BridgedDevice::SetReachable(bool reachable) void BridgedDevice::SetAdminCommissioningAttributes(const AdminCommissioningAttributes & aAdminCommissioningAttributes) { - ReportAttributeChangedWorkData * workdata = chip::Platform::New(); + ReportAttributeChangedWorkData * workdata = chip::Platform::New(); workdata->mEndpointId = mEndpointId; workdata->mWindowChanged =