Marshal the data from BasicInformationCluster to FabricBridge (project-chip#34854)

andy31415 · restyled-commits · andreilitvin · austina-csa · commit 46480dcb1a52 · 2024-08-12T16:04:41.000-07:00
* 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

* 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

---------

Co-authored-by: Restyled.io &lt;commits@restyled.io&gt;
Co-authored-by: Andrei Litvin &lt;andreilitvin@google.com&gt;
diff --git a/examples/common/pigweed/protos/fabric_bridge_service.proto b/examples/common/pigweed/protos/fabric_bridge_service.proto
@@ -18,6 +18,7 @@ message SynchronizedDevice {
   optional string hardware_version_string = 9;
   optional uint32 software_version = 10;
   optional string software_version_string = 11;
+  optional bool is_icd = 12;
 }
 
 service FabricBridge {
diff --git a/examples/fabric-admin/BUILD.gn b/examples/fabric-admin/BUILD.gn
@@ -72,6 +72,8 @@ static_library("fabric-admin-utils") {
     "commands/common/RemoteDataModelLogger.cpp",
     "commands/common/RemoteDataModelLogger.h",
     "commands/fabric-sync/FabricSyncCommand.cpp",
+    "commands/pairing/DeviceSynchronization.cpp",
+    "commands/pairing/DeviceSynchronization.h",
     "commands/pairing/OpenCommissioningWindowCommand.cpp",
     "commands/pairing/OpenCommissioningWindowCommand.h",
     "commands/pairing/PairingCommand.cpp",
diff --git a/examples/fabric-admin/commands/pairing/DeviceSynchronization.cpp b/examples/fabric-admin/commands/pairing/DeviceSynchronization.cpp
@@ -0,0 +1,187 @@
+/*
+ *   Copyright (c) 2024 Project CHIP Authors
+ *   All rights reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ *
+ */
+
+#include "DeviceSynchronization.h"
+#include "rpc/RpcClient.h"
+
+#include <app/InteractionModelEngine.h>
+#include <app/server/Server.h>
+
+#include <app-common/zap-generated/ids/Attributes.h>
+#include <app-common/zap-generated/ids/Clusters.h>
+
+using namespace ::chip;
+using namespace ::chip::app;
+using chip::app::ReadClient;
+
+namespace {
+
+void OnDeviceConnectedWrapper(void * context, Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle)
+{
+    reinterpret_cast<DeviceSynchronizer *>(context)->OnDeviceConnected(exchangeMgr, sessionHandle);
+}
+
+void OnDeviceConnectionFailureWrapper(void * context, const ScopedNodeId & peerId, CHIP_ERROR error)
+{
+    reinterpret_cast<DeviceSynchronizer *>(context)->OnDeviceConnectionFailure(peerId, error);
+}
+
+bool SuccessOrLog(CHIP_ERROR err, const char * name)
+{
+    if (err == CHIP_NO_ERROR)
+    {
+        return true;
+    }
+
+    ChipLogError(NotSpecified, "Failed to read %s: %" CHIP_ERROR_FORMAT, name, err.Format());
+
+    return false;
+}
+
+} // namespace
+
+DeviceSynchronizer & DeviceSynchronizer::Instance()
+{
+    static DeviceSynchronizer instance;
+    return instance;
+}
+
+DeviceSynchronizer::DeviceSynchronizer() :
+    mOnDeviceConnectedCallback(OnDeviceConnectedWrapper, this),
+    mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureWrapper, this)
+{}
+
+void DeviceSynchronizer::OnAttributeData(const ConcreteDataAttributePath & path, TLV::TLVReader * data, const StatusIB & status)
+{
+    VerifyOrDie(path.mEndpointId == kRootEndpointId);
+    VerifyOrDie(path.mClusterId == Clusters::BasicInformation::Id);
+
+    switch (path.mAttributeId)
+    {
+    case Clusters::BasicInformation::Attributes::UniqueID::Id:
+        mCurrentDeviceData.has_unique_id =
+            SuccessOrLog(data->GetString(mCurrentDeviceData.unique_id, sizeof(mCurrentDeviceData.unique_id)), "UniqueId");
+        break;
+    case Clusters::BasicInformation::Attributes::VendorName::Id:
+        mCurrentDeviceData.has_vendor_name =
+            SuccessOrLog(data->GetString(mCurrentDeviceData.vendor_name, sizeof(mCurrentDeviceData.vendor_name)), "VendorName");
+        break;
+    case Clusters::BasicInformation::Attributes::VendorID::Id:
+        mCurrentDeviceData.has_vendor_id = SuccessOrLog(data->Get(mCurrentDeviceData.vendor_id), "VendorID");
+        break;
+    case Clusters::BasicInformation::Attributes::ProductName::Id:
+        mCurrentDeviceData.has_product_name =
+            SuccessOrLog(data->GetString(mCurrentDeviceData.product_name, sizeof(mCurrentDeviceData.product_name)), "ProductName");
+        break;
+    case Clusters::BasicInformation::Attributes::ProductID::Id:
+        mCurrentDeviceData.has_product_id = SuccessOrLog(data->Get(mCurrentDeviceData.product_id), "ProductID");
+        break;
+    case Clusters::BasicInformation::Attributes::NodeLabel::Id:
+        mCurrentDeviceData.has_node_label =
+            SuccessOrLog(data->GetString(mCurrentDeviceData.node_label, sizeof(mCurrentDeviceData.node_label)), "NodeLabel");
+        break;
+    case Clusters::BasicInformation::Attributes::HardwareVersion::Id:
+        mCurrentDeviceData.has_hardware_version = SuccessOrLog(data->Get(mCurrentDeviceData.hardware_version), "HardwareVersion");
+        break;
+    case Clusters::BasicInformation::Attributes::HardwareVersionString::Id:
+        mCurrentDeviceData.has_hardware_version_string = SuccessOrLog(
+            data->GetString(mCurrentDeviceData.hardware_version_string, sizeof(mCurrentDeviceData.hardware_version_string)),
+            "HardwareVersionString");
+        break;
+    case Clusters::BasicInformation::Attributes::SoftwareVersion::Id:
+        mCurrentDeviceData.has_software_version = SuccessOrLog(data->Get(mCurrentDeviceData.software_version), "HardwareVersion");
+        break;
+    case Clusters::BasicInformation::Attributes::SoftwareVersionString::Id:
+        mCurrentDeviceData.has_software_version_string = SuccessOrLog(
+            data->GetString(mCurrentDeviceData.software_version_string, sizeof(mCurrentDeviceData.software_version_string)),
+            "SoftwareVersionString");
+        break;
+    default:
+        break;
+    }
+}
+
+void DeviceSynchronizer::OnReportEnd()
+{
+    // Report end is at the end of all attributes (success)
+#if defined(PW_RPC_ENABLED)
+    AddSynchronizedDevice(mCurrentDeviceData);
+#else
+    ChipLogError(NotSpecified, "Cannot synchronize device with fabric bridge: RPC not enabled");
+#endif
+}
+
+void DeviceSynchronizer::OnDone(chip::app::ReadClient * apReadClient)
+{
+    // Nothing to do: error reported on OnError or report ended called.
+    mDeviceSyncInProcess = false;
+}
+
+void DeviceSynchronizer::OnError(CHIP_ERROR error)
+{
+    ChipLogProgress(NotSpecified, "Error fetching device data: %" CHIP_ERROR_FORMAT, error.Format());
+}
+
+void DeviceSynchronizer::OnDeviceConnected(chip::Messaging::ExchangeManager & exchangeMgr,
+                                           const chip::SessionHandle & sessionHandle)
+{
+    mClient = std::make_unique<ReadClient>(app::InteractionModelEngine::GetInstance(), &exchangeMgr /* echangeMgr */,
+                                           *this /* callback */, ReadClient::InteractionType::Read);
+    VerifyOrDie(mClient);
+
+    AttributePathParams readPaths[1];
+    readPaths[0] = AttributePathParams(kRootEndpointId, Clusters::BasicInformation::Id);
+
+    ReadPrepareParams readParams(sessionHandle);
+
+    readParams.mpAttributePathParamsList    = readPaths;
+    readParams.mAttributePathParamsListSize = 1;
+
+    CHIP_ERROR err = mClient->SendRequest(readParams);
+
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(NotSpecified, "Failed to issue read for BasicInformation data");
+        mDeviceSyncInProcess = false;
+    }
+}
+
+void DeviceSynchronizer::OnDeviceConnectionFailure(const chip::ScopedNodeId & peerId, CHIP_ERROR error)
+{
+    ChipLogError(NotSpecified, "Device Sync failed to connect to " ChipLogFormatX64, ChipLogValueX64(peerId.GetNodeId()));
+    mDeviceSyncInProcess = false;
+}
+
+void DeviceSynchronizer::StartDeviceSynchronization(chip::Controller::DeviceController & controller, chip::NodeId nodeId,
+                                                    bool deviceIsIcd)
+{
+    if (mDeviceSyncInProcess)
+    {
+        ChipLogError(NotSpecified, "Device Sync NOT POSSIBLE: another sync is in progress");
+        return;
+    }
+
+    mCurrentDeviceData            = chip_rpc_SynchronizedDevice_init_default;
+    mCurrentDeviceData.node_id    = nodeId;
+    mCurrentDeviceData.has_is_icd = true;
+    mCurrentDeviceData.is_icd     = deviceIsIcd;
+
+    mDeviceSyncInProcess = true;
+
+    controller.GetConnectedDevice(nodeId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback);
+}
diff --git a/examples/fabric-admin/commands/pairing/DeviceSynchronization.h b/examples/fabric-admin/commands/pairing/DeviceSynchronization.h
@@ -0,0 +1,70 @@
+/*
+ *   Copyright (c) 2024 Project CHIP Authors
+ *   All rights reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ *
+ */
+#pragma once
+
+#include <app/ReadClient.h>
+#include <controller/CHIPDeviceController.h>
+#include <lib/core/DataModelTypes.h>
+
+#include <memory>
+
+#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.
+///
+/// Includes a state machine that:
+///   - initiates a "read basic information data" command to fetch basic information
+///   - upon receiving such information, ensures that synchronized device data is sent
+///     to the remote end.
+class DeviceSynchronizer : public chip::app::ReadClient::Callback
+{
+public:
+    DeviceSynchronizer();
+
+    /// Usually called after commissioning is complete, initiates a
+    /// read of required data from the remote node ID and then will synchronize
+    /// the device towards the fabric bridge
+    void StartDeviceSynchronization(chip::Controller::DeviceController & controller, chip::NodeId nodeId, bool deviceIsIcd);
+
+    ///////////////////////////////////////////////////////////////
+    // ReadClient::Callback implementation
+    ///////////////////////////////////////////////////////////////
+    void OnAttributeData(const chip::app::ConcreteDataAttributePath & path, chip::TLV::TLVReader * data,
+                         const chip::app::StatusIB & status) override;
+    void OnReportEnd() override;
+    void OnError(CHIP_ERROR error) override;
+    void OnDone(chip::app::ReadClient * apReadClient) override;
+
+    ///////////////////////////////////////////////////////////////
+    // callbacks for CASE session establishment
+    ///////////////////////////////////////////////////////////////
+    void OnDeviceConnected(chip::Messaging::ExchangeManager & exchangeMgr, const chip::SessionHandle & sessionHandle);
+    void OnDeviceConnectionFailure(const chip::ScopedNodeId & peerId, CHIP_ERROR error);
+
+    static DeviceSynchronizer & Instance();
+
+private:
+    std::unique_ptr<chip::app::ReadClient> mClient;
+
+    chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;
+    chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
+
+    bool mDeviceSyncInProcess                      = false;
+    chip_rpc_SynchronizedDevice mCurrentDeviceData = chip_rpc_SynchronizedDevice_init_default;
+};
diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.cpp b/examples/fabric-admin/commands/pairing/PairingCommand.cpp
@@ -18,7 +18,10 @@
 
 #include "PairingCommand.h"
 
+#include <app-common/zap-generated/ids/Clusters.h>
 #include <commands/common/DeviceScanner.h>
+#include <commands/interactive/InteractiveCommands.h>
+#include <commands/pairing/DeviceSynchronization.h>
 #include <controller/ExampleOperationalCredentialsIssuer.h>
 #include <crypto/CHIPCryptoPAL.h>
 #include <lib/core/CHIPSafeCasts.h>
@@ -400,10 +403,7 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
     {
         // print to console
         fprintf(stderr, "New device with Node ID: 0x%lx has been successfully added.\n", nodeId);
-
-#if defined(PW_RPC_ENABLED)
-        AddSynchronizedDevice(nodeId);
-#endif
+        DeviceSynchronizer::Instance().StartDeviceSynchronization(CurrentCommissioner(), mNodeId, mDeviceIsICD);
     }
     else
     {
diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp
@@ -22,16 +22,9 @@
 #include <chrono>
 #include <condition_variable>
 #include <mutex>
-#include <string>
-#include <thread>
 
+#include "fabric_bridge_service/fabric_bridge_service.pb.h"
 #include "fabric_bridge_service/fabric_bridge_service.rpc.pb.h"
-#include "pw_assert/check.h"
-#include "pw_hdlc/decoder.h"
-#include "pw_hdlc/default_addresses.h"
-#include "pw_hdlc/rpc_channel.h"
-#include "pw_rpc/client.h"
-#include "pw_stream/socket_stream.h"
 
 using namespace chip;
 
@@ -113,44 +106,13 @@ CHIP_ERROR InitRpcClient(uint16_t rpcServerPort)
     return rpc::client::StartPacketProcessing();
 }
 
-CHIP_ERROR AddSynchronizedDevice(chip::NodeId nodeId)
+CHIP_ERROR AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & data)
 {
     ChipLogProgress(NotSpecified, "AddSynchronizedDevice");
 
-    chip_rpc_SynchronizedDevice device = chip_rpc_SynchronizedDevice_init_default;
-    device.node_id                     = nodeId;
-
-    // TODO: fill this with real data. For now we just add things for testing
-    strcpy(device.vendor_name, "Test Vendor");
-    device.has_vendor_name = true;
-
-    device.vendor_id     = 123;
-    device.has_vendor_id = true;
-
-    strcpy(device.product_name, "Test Product");
-    device.has_product_name = true;
-
-    device.product_id     = 234;
-    device.has_product_id = true;
-
-    strcpy(device.node_label, "Device Label");
-    device.has_node_label = true;
-
-    device.hardware_version     = 11;
-    device.has_hardware_version = true;
-
-    strcpy(device.hardware_version_string, "Hardware");
-    device.has_hardware_version_string = true;
-
-    device.software_version     = 22;
-    device.has_software_version = true;
-
-    strcpy(device.software_version_string, "Test 1.4.22");
-    device.has_software_version_string = true;
-
     // 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.AddSynchronizedDevice(device, OnAddDeviceResponseCompleted);
+    auto call = fabricBridgeClient.AddSynchronizedDevice(data, OnAddDeviceResponseCompleted);
 
     if (!call.active())
     {
diff --git a/examples/fabric-admin/rpc/RpcClient.h b/examples/fabric-admin/rpc/RpcClient.h
@@ -20,6 +20,8 @@
 
 #include <platform/CHIPDeviceLayer.h>
 
+#include "fabric_bridge_service/fabric_bridge_service.rpc.pb.h"
+
 constexpr uint16_t kFabricBridgeServerPort = 33002;
 
 /**
@@ -39,13 +41,12 @@ CHIP_ERROR InitRpcClient(uint16_t rpcServerPort);
  * It logs the progress and checks if an `AddSynchronizedDevice` 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 added.
  * @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 AddSynchronizedDevice(chip::NodeId nodeId);
+CHIP_ERROR AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & data);
 
 /**
  * @brief Removes a synchronized device from the RPC client.

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@ message SynchronizedDevice {`
`18`	`18`	`optional string hardware_version_string = 9;`
`19`	`19`	`optional uint32 software_version = 10;`
`20`	`20`	`optional string software_version_string = 11;`
	`21`	`+ optional bool is_icd = 12;`
`21`	`22`	`}`
`22`	`23`
`23`	`24`	`service FabricBridge {`