Add ICD support (featuremap and commands) to FabricAdmin/FabricBridge…

… samples (#34855) * Add extra attributes to the bridged device basic info structures, remove nonsense comments * Make use of AAI for BridgedDeviceBasicInformation cluster * Restyled by gn * Fix sizes for software version * Update the synchronized device proto to have more data in it * Switch to unique ptr in the registry, making sure memory management works (fixed memory leak on remove device) * Use more std::optional * Bump revision to 4 * Forward attributes from the create call into the bridged device * Make attribute mapping actually work * Restyle * Ensure unique IDs are generated * Restyle * Increase size to 33 to allow for a null terminator * make sure that the rpc structures are initialized * Restyle * Add some fake data to test moving the data around * Remove unused members that were likely just copied over * make the attributes optional * Prepare some device sync data - reading the basic info cluster * Prepare some device sync data - reading the basic info cluster * Full implementation of forwarding data * Restyle * Add missing file * Restyle * reset readclient, since this may reset the exchange manager ... seems cleaner * Add the verifyOrDie * Restyled by clang-format * Fix string size for HW and software versions * Move ICD support: set separate commands, handle feature mask * Restyle * Remove some of the spammier logs * Enfore RPC enabling for synchronized device addition * Add device sync in progress tracking * Undo submodule update * Fix up device sync progress tracking to better handle errors * Restyled by clang-format * Fix typo * Undo submodule update * Fix merge * Restyled by clang-format --------- Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Dec 13, 2024 · 1197167 · 1197167
1 parent 7fbaaba
commit 1197167
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 30 deletions.
diff --git a/examples/fabric-admin/commands/pairing/DeviceSynchronization.h b/examples/fabric-admin/commands/pairing/DeviceSynchronization.h
@@ -26,7 +26,7 @@
 #include "fabric_bridge_service/fabric_bridge_service.pb.h"
 #include "fabric_bridge_service/fabric_bridge_service.rpc.pb.h"
 
-/// Ensures that device data is synchronized to the remove fabric bridge.
+/// Ensures that device data is synchronized to the remote fabric bridge.
 ///
 /// Includes a state machine that:
 ///   - initiates a "read basic information data" command to fetch basic information

diff --git a/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h b/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h
@@ -43,11 +43,13 @@ class BridgedDevice
     BridgedDevice(chip::NodeId nodeId);
     virtual ~BridgedDevice() = default;
 
+    [[nodiscard]] bool IsReachable() const { return mReachable; }
+    void SetReachable(bool reachable);
+
     void LogActiveChangeEvent(uint32_t promisedActiveDurationMs);
 
-    bool IsReachable();
-    bool IsIcd();
-    void SetReachable(bool reachable);
+    [[nodiscard]] bool IsIcd() const { return mIsIcd; }
+    void SetIcd(bool icd) { mIsIcd = icd; }
 
     inline void SetEndpointId(chip::EndpointId id) { mEndpointId = id; };
     inline chip::EndpointId GetEndpointId() { return mEndpointId; };
@@ -63,11 +65,12 @@ class BridgedDevice
     void SetUniqueId(const std::string & value) { mAttributes.uniqueId = value; }
 
 protected:
-    bool mReachable;
-    bool mIsIcd = false;
-    chip::NodeId mNodeId;
-    chip::EndpointId mEndpointId;
-    chip::EndpointId mParentEndpointId;
+    bool mReachable = false;
+    bool mIsIcd     = false;
+
+    chip::NodeId mNodeId               = 0;
+    chip::EndpointId mEndpointId       = 0;
+    chip::EndpointId mParentEndpointId = 0;
 
     BridgedAttributes mAttributes;
 };
diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp
@@ -19,7 +19,6 @@
 #include "BridgedDevice.h"
 
 #include <cstdio>
-#include <string>
 
 #include <app/EventLogging.h>
 #include <platform/CHIPDeviceLayer.h>
@@ -32,7 +31,7 @@ struct ActiveChangeEventWorkData
     uint32_t mPromisedActiveDuration;
 };
 
-static void ActiveChangeEventWork(intptr_t arg)
+void ActiveChangeEventWork(intptr_t arg)
 {
     ActiveChangeEventWorkData * data = reinterpret_cast<ActiveChangeEventWorkData *>(arg);
 
@@ -68,16 +67,6 @@ void BridgedDevice::LogActiveChangeEvent(uint32_t promisedActiveDurationMs)
     chip::DeviceLayer::PlatformMgr().ScheduleWork(ActiveChangeEventWork, reinterpret_cast<intptr_t>(workdata));
 }
 
-bool BridgedDevice::IsReachable()
-{
-    return mReachable;
-}
-
-bool BridgedDevice::IsIcd()
-{
-    return mIsIcd;
-}
-
 void BridgedDevice::SetReachable(bool reachable)
 {
     mReachable = reachable;

diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceBasicInformationImpl.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceBasicInformationImpl.cpp
@@ -24,7 +24,6 @@
 #include <app/AttributeAccessInterfaceRegistry.h>
 
 static constexpr unsigned kBridgedDeviceBasicInformationClusterRevision = 4;
-static constexpr unsigned kBridgedDeviceBasicInformationFeatureMap      = 0;
 
 using namespace ::chip;
 using namespace ::chip::app;
@@ -49,9 +48,12 @@ CHIP_ERROR BridgedDeviceBasicInformationImpl::Read(const ConcreteReadAttributePa
     case BasicInformation::Attributes::ClusterRevision::Id:
         encoder.Encode(kBridgedDeviceBasicInformationClusterRevision);
         break;
-    case BasicInformation::Attributes::FeatureMap::Id:
-        encoder.Encode(kBridgedDeviceBasicInformationFeatureMap);
-        break;
+    case BasicInformation::Attributes::FeatureMap::Id: {
+        BitMask<Clusters::BridgedDeviceBasicInformation::Feature> features;
+        features.Set(Clusters::BridgedDeviceBasicInformation::Feature::kBridgedICDSupport, dev->IsIcd());
+        encoder.Encode(features);
+    }
+    break;
     case BasicInformation::Attributes::UniqueID::Id:
         encoder.Encode(CharSpan::fromCharString(dev->GetBridgedAttributes().uniqueId.c_str()));
         break;

diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceManager.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceManager.cpp
@@ -141,17 +141,31 @@ constexpr CommandId administratorCommissioningCommands[] = {
     kInvalidCommandId,
 };
 
+// clang-format off
 // Declare Cluster List for Bridged Node endpoint
 DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(bridgedNodeClusters)
-DECLARE_DYNAMIC_CLUSTER(Descriptor::Id, descriptorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
+    DECLARE_DYNAMIC_CLUSTER(Descriptor::Id, descriptorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
+    DECLARE_DYNAMIC_CLUSTER(BridgedDeviceBasicInformation::Id, bridgedDeviceBasicAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
+    DECLARE_DYNAMIC_CLUSTER(EcosystemInformation::Id, ecosystemInformationBasicAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
+    DECLARE_DYNAMIC_CLUSTER(AdministratorCommissioning::Id, AdministratorCommissioningAttrs, ZAP_CLUSTER_MASK(SERVER),
+                            administratorCommissioningCommands, nullptr)
+DECLARE_DYNAMIC_CLUSTER_LIST_END;
+
+DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(icdBridgedNodeClusters)
+    DECLARE_DYNAMIC_CLUSTER(Descriptor::Id, descriptorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
     DECLARE_DYNAMIC_CLUSTER(BridgedDeviceBasicInformation::Id, bridgedDeviceBasicAttrs, ZAP_CLUSTER_MASK(SERVER),
                             bridgedDeviceBasicInformationCommands, nullptr),
-    DECLARE_DYNAMIC_CLUSTER(EcosystemInformation::Id, ecosystemInformationBasicAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
     DECLARE_DYNAMIC_CLUSTER(AdministratorCommissioning::Id, AdministratorCommissioningAttrs, ZAP_CLUSTER_MASK(SERVER),
-                            administratorCommissioningCommands, nullptr) DECLARE_DYNAMIC_CLUSTER_LIST_END;
+                            administratorCommissioningCommands, nullptr)
+DECLARE_DYNAMIC_CLUSTER_LIST_END;
+// clang-format on
 
 // Declare Bridged Node endpoint
 DECLARE_DYNAMIC_ENDPOINT(sBridgedNodeEndpoint, bridgedNodeClusters);
+DECLARE_DYNAMIC_ENDPOINT(sIcdBridgedNodeEndpoint, icdBridgedNodeClusters);
+
+// TODO: this is a single version array, however we may have many
+//       different clusters that are independent.
 DataVersion sBridgedNodeDataVersions[ArraySize(bridgedNodeClusters)];
 
 const EmberAfDeviceType sBridgedDeviceTypes[] = { { DEVICE_TYPE_BRIDGED_NODE, DEVICE_VERSION_DEFAULT } };
@@ -171,9 +185,12 @@ void BridgedDeviceManager::Init()
 std::optional<unsigned> BridgedDeviceManager::AddDeviceEndpoint(std::unique_ptr<BridgedDevice> dev,
                                                                 chip::EndpointId parentEndpointId)
 {
-    EmberAfEndpointType * ep                                   = &sBridgedNodeEndpoint;
+    EmberAfEndpointType * ep = dev->IsIcd() ? &sIcdBridgedNodeEndpoint : &sBridgedNodeEndpoint;
+
     const chip::Span<const EmberAfDeviceType> & deviceTypeList = Span<const EmberAfDeviceType>(sBridgedDeviceTypes);
-    const chip::Span<chip::DataVersion> & dataVersionStorage   = Span<DataVersion>(sBridgedNodeDataVersions);
+
+    // TODO: this shares data version among different clusters, which seems incorrect
+    const chip::Span<chip::DataVersion> & dataVersionStorage = Span<DataVersion>(sBridgedNodeDataVersions);
 
     if (dev->GetBridgedAttributes().uniqueId.empty())
     {

diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp
@@ -109,6 +109,7 @@ pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice
     }
 
     device->SetBridgedAttributes(attributes);
+    device->SetIcd(request.has_is_icd && request.is_icd);
 
     auto result = BridgeDeviceMgr().AddDeviceEndpoint(std::move(device), 1 /* parentEndpointId */);
     if (!result.has_value())