From a0fac9fd316cb63402430401bba7d4fe245b4efd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 7 Aug 2024 12:58:02 -0400 Subject: [PATCH] Fabric Bridge: use AttributeAccessInterface for BridgedDeviceBasicInformationCluster, expose more attributes (#34845) * 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 * Bump revision to 4 --------- Co-authored-by: Restyled.io --- .../fabric-bridge-common/BUILD.gn | 2 + .../BridgedDeviceBasicInformationImpl.h | 32 ++++++++ .../src/BridgedDeviceBasicInformationImpl.cpp | 78 +++++++++++++++++++ .../src/BridgedDeviceManager.cpp | 52 +++++++++---- .../fabric-bridge-common/src/ZCLCallbacks.cpp | 64 +-------------- examples/fabric-bridge-app/linux/main.cpp | 4 + 6 files changed, 154 insertions(+), 78 deletions(-) create mode 100644 examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDeviceBasicInformationImpl.h create mode 100644 examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceBasicInformationImpl.cpp diff --git a/examples/fabric-bridge-app/fabric-bridge-common/BUILD.gn b/examples/fabric-bridge-app/fabric-bridge-common/BUILD.gn index 10cb48c31c584e..38f2f8f329bd88 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/BUILD.gn +++ b/examples/fabric-bridge-app/fabric-bridge-common/BUILD.gn @@ -30,9 +30,11 @@ source_set("fabric-bridge-lib") { sources = [ "include/BridgedDevice.h", + "include/BridgedDeviceBasicInformationImpl.h", "include/BridgedDeviceManager.h", "include/CHIPProjectAppConfig.h", "src/BridgedDevice.cpp", + "src/BridgedDeviceBasicInformationImpl.cpp", "src/BridgedDeviceManager.cpp", "src/ZCLCallbacks.cpp", ] diff --git a/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDeviceBasicInformationImpl.h b/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDeviceBasicInformationImpl.h new file mode 100644 index 00000000000000..23403438ab2be8 --- /dev/null +++ b/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDeviceBasicInformationImpl.h @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2024 Project CHIP Authors + * + * 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 +#include + +class BridgedDeviceBasicInformationImpl : public chip::app::AttributeAccessInterface +{ +public: + BridgedDeviceBasicInformationImpl() : + chip::app::AttributeAccessInterface(chip::NullOptional /* endpointId */, + chip::app::Clusters::BridgedDeviceBasicInformation::Id) + {} + + // AttributeAccessInterface implementation + CHIP_ERROR Read(const chip::app::ConcreteReadAttributePath & path, chip::app::AttributeValueEncoder & encoder) override; + CHIP_ERROR Write(const chip::app::ConcreteDataAttributePath & path, chip::app::AttributeValueDecoder & decoder) override; +}; diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceBasicInformationImpl.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceBasicInformationImpl.cpp new file mode 100644 index 00000000000000..62630d730b4355 --- /dev/null +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceBasicInformationImpl.cpp @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2024 Project CHIP Authors + * + * 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 "BridgedDeviceBasicInformationImpl.h" + +#include "BridgedDeviceManager.h" + +#include +#include +#include + +#include + +static constexpr unsigned kBridgedDeviceBasicInformationClusterRevision = 4; +static constexpr unsigned kBridgedDeviceBasicInformationFeatureMap = 0; + +using namespace ::chip; +using namespace ::chip::app; +using namespace ::chip::app::Clusters; + +CHIP_ERROR BridgedDeviceBasicInformationImpl::Read(const ConcreteReadAttributePath & path, AttributeValueEncoder & encoder) +{ + // Registration is done for the bridged device basic information only + VerifyOrDie(path.mClusterId == app::Clusters::BridgedDeviceBasicInformation::Id); + + BridgedDevice * dev = BridgeDeviceMgr().GetDevice(path.mEndpointId); + VerifyOrReturnError(dev != nullptr, CHIP_ERROR_NOT_FOUND); + + switch (path.mAttributeId) + { + case BasicInformation::Attributes::Reachable::Id: + encoder.Encode(dev->IsReachable()); + break; + case BasicInformation::Attributes::NodeLabel::Id: + encoder.Encode(CharSpan::fromCharString(dev->GetName())); + break; + case BasicInformation::Attributes::ClusterRevision::Id: + encoder.Encode(kBridgedDeviceBasicInformationClusterRevision); + break; + case BasicInformation::Attributes::FeatureMap::Id: + encoder.Encode(kBridgedDeviceBasicInformationFeatureMap); + break; + default: + return CHIP_ERROR_INVALID_ARGUMENT; + } + return CHIP_NO_ERROR; +} + +CHIP_ERROR BridgedDeviceBasicInformationImpl::Write(const ConcreteDataAttributePath & path, AttributeValueDecoder & decoder) +{ + VerifyOrDie(path.mClusterId == app::Clusters::BridgedDeviceBasicInformation::Id); + + BridgedDevice * dev = BridgeDeviceMgr().GetDevice(path.mEndpointId); + VerifyOrReturnError(dev != nullptr, CHIP_ERROR_NOT_FOUND); + + if (!dev->IsReachable()) + { + return CHIP_ERROR_NOT_CONNECTED; + } + + ChipLogProgress(NotSpecified, "Bridged device basic information attempt to write attribute: ep=%d", path.mAttributeId); + + // nothing writable right now ... + + return CHIP_ERROR_INVALID_ARGUMENT; +} diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceManager.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceManager.cpp index 170b39cd2c94e2..7e78baa7ded548 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceManager.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDeviceManager.cpp @@ -45,8 +45,13 @@ using namespace chip::app::Clusters; namespace { -constexpr uint8_t kMaxRetries = 10; -constexpr int kNodeLabelSize = 32; +constexpr uint8_t kMaxRetries = 10; +constexpr int kNodeLabelSize = 32; +constexpr int kUniqueIdSize = 32; +constexpr int kVendorNameSize = 32; +constexpr int kProductNameSize = 32; +constexpr int kHardwareVersionSize = 32; +constexpr int kSoftwareVersionSize = 32; // Current ZCL implementation of Struct uses a max-size array of 254 bytes constexpr int kDescriptorAttributeArraySize = 254; @@ -76,27 +81,44 @@ constexpr int kDescriptorAttributeArraySize = 254; // - Bridged Device Basic Information // - Administrator Commissioning +// clang-format off // Declare Descriptor cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(descriptorAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::DeviceTypeList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* device list */ - DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::ServerList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* server list */ - DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::ClientList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* client list */ - DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::PartsList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* parts list */ - DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); + DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::DeviceTypeList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* device list */ + DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::ServerList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* server list */ + DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::ClientList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* client list */ + DECLARE_DYNAMIC_ATTRIBUTE(Descriptor::Attributes::PartsList::Id, ARRAY, kDescriptorAttributeArraySize, 0), /* parts list */ +DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Bridged Device Basic Information cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(bridgedDeviceBasicAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::NodeLabel::Id, CHAR_STRING, kNodeLabelSize, 0), /* NodeLabel */ - DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::Reachable::Id, BOOLEAN, 1, 0), /* Reachable */ - DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::FeatureMap::Id, BITMAP32, 4, 0), /* feature map */ - DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); + // The attributes below are MANDATORY in the Bridged Device Information Cluster + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::Reachable::Id, BOOLEAN, 1, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::UniqueID::Id, CHAR_STRING, kUniqueIdSize, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::FeatureMap::Id, BITMAP32, 4, 0), + + // The attributes below are OPTIONAL in the bridged device, however they are MANDATORY in the BasicInformation cluster + // so they can always be provided if desired + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::VendorName::Id, CHAR_STRING, kVendorNameSize, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::VendorID::Id, INT16U, 2, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::ProductName::Id, CHAR_STRING, kProductNameSize, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::ProductID::Id, INT16U, 2, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::NodeLabel::Id, CHAR_STRING, kNodeLabelSize, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::HardwareVersion::Id, INT16U, 2, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::HardwareVersionString::Id, CHAR_STRING, + kHardwareVersionSize, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::SoftwareVersion::Id, INT32U, 4, 0), + DECLARE_DYNAMIC_ATTRIBUTE(BridgedDeviceBasicInformation::Attributes::SoftwareVersionString::Id, CHAR_STRING, + kSoftwareVersionSize, 0), +DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Administrator Commissioning cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(AdministratorCommissioningAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(AdministratorCommissioning::Attributes::WindowStatus::Id, ENUM8, 1, 0), /* NodeLabel */ - DECLARE_DYNAMIC_ATTRIBUTE(AdministratorCommissioning::Attributes::AdminFabricIndex::Id, FABRIC_IDX, 1, 0), /* Reachable */ - DECLARE_DYNAMIC_ATTRIBUTE(AdministratorCommissioning::Attributes::AdminVendorId::Id, VENDOR_ID, 2, 0), /* Reachable */ - DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); + DECLARE_DYNAMIC_ATTRIBUTE(AdministratorCommissioning::Attributes::WindowStatus::Id, ENUM8, 1, 0), + DECLARE_DYNAMIC_ATTRIBUTE(AdministratorCommissioning::Attributes::AdminFabricIndex::Id, FABRIC_IDX, 1, 0), + DECLARE_DYNAMIC_ATTRIBUTE(AdministratorCommissioning::Attributes::AdminVendorId::Id, VENDOR_ID, 2, 0), +DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); +// clang-format on constexpr CommandId administratorCommissioningCommands[] = { app::Clusters::AdministratorCommissioning::Commands::OpenCommissioningWindow::Id, diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/ZCLCallbacks.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/ZCLCallbacks.cpp index 26e727901abf0e..d4da982cfa45a5 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/ZCLCallbacks.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/ZCLCallbacks.cpp @@ -25,88 +25,26 @@ using namespace ::chip; using namespace ::chip::app::Clusters; -#define ZCL_DESCRIPTOR_CLUSTER_REVISION (1u) #define ZCL_ADMINISTRATOR_COMMISSIONING_CLUSTER_REVISION (1u) -#define ZCL_BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_REVISION (2u) -#define ZCL_BRIDGED_DEVICE_BASIC_INFORMATION_FEATURE_MAP (0u) // External attribute read callback function Protocols::InteractionModel::Status emberAfExternalAttributeReadCallback(EndpointId endpoint, ClusterId clusterId, const EmberAfAttributeMetadata * attributeMetadata, uint8_t * buffer, uint16_t maxReadLength) { - AttributeId attributeId = attributeMetadata->attributeId; - - BridgedDevice * dev = BridgeDeviceMgr().GetDevice(endpoint); - if (dev == nullptr) - { - return Protocols::InteractionModel::Status::Failure; - } - - if (clusterId == BridgedDeviceBasicInformation::Id) - { - using namespace BridgedDeviceBasicInformation::Attributes; - ChipLogProgress(NotSpecified, "HandleReadBridgedDeviceBasicAttribute: attrId=%d, maxReadLength=%d", attributeId, - maxReadLength); - - if ((attributeId == Reachable::Id) && (maxReadLength == 1)) - { - *buffer = dev->IsReachable() ? 1 : 0; - } - else if ((attributeId == NodeLabel::Id) && (maxReadLength == 32)) - { - MutableByteSpan zclNameSpan(buffer, maxReadLength); - MakeZclCharString(zclNameSpan, dev->GetName()); - } - else if ((attributeId == ClusterRevision::Id) && (maxReadLength == 2)) - { - uint16_t rev = ZCL_BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_REVISION; - memcpy(buffer, &rev, sizeof(rev)); - } - else if ((attributeId == FeatureMap::Id) && (maxReadLength == 4)) - { - uint32_t featureMap = ZCL_BRIDGED_DEVICE_BASIC_INFORMATION_FEATURE_MAP; - memcpy(buffer, &featureMap, sizeof(featureMap)); - } - else - { - return Protocols::InteractionModel::Status::Failure; - } - return Protocols::InteractionModel::Status::Success; - } - if (clusterId == AdministratorCommissioning::Id) { // TODO(#34791) This is a workaround to prevent crash. CADMIN is still reading incorrect // Attribute values on dynamic endpoint as it only reads the root node and not the actual bridge // device we are representing here, when addressing the issue over there we can more easily // resolve this workaround. - if ((attributeId == AdministratorCommissioning::Attributes::ClusterRevision::Id) && (maxReadLength == 2)) + if ((attributeMetadata->attributeId == AdministratorCommissioning::Attributes::ClusterRevision::Id) && (maxReadLength == 2)) { uint16_t rev = ZCL_ADMINISTRATOR_COMMISSIONING_CLUSTER_REVISION; memcpy(buffer, &rev, sizeof(rev)); return Protocols::InteractionModel::Status::Success; } - return Protocols::InteractionModel::Status::Failure; } return Protocols::InteractionModel::Status::Failure; } - -// External attribute write callback function -Protocols::InteractionModel::Status emberAfExternalAttributeWriteCallback(EndpointId endpoint, ClusterId clusterId, - const EmberAfAttributeMetadata * attributeMetadata, - uint8_t * buffer) -{ - uint16_t endpointIndex = emberAfGetDynamicIndexFromEndpoint(endpoint); - Protocols::InteractionModel::Status ret = Protocols::InteractionModel::Status::Failure; - - BridgedDevice * dev = BridgeDeviceMgr().GetDevice(endpointIndex); - if (dev != nullptr && dev->IsReachable()) - { - ChipLogProgress(NotSpecified, "emberAfExternalAttributeWriteCallback: ep=%d, clusterId=%d", endpoint, clusterId); - ret = Protocols::InteractionModel::Status::Success; - } - - return ret; -} diff --git a/examples/fabric-bridge-app/linux/main.cpp b/examples/fabric-bridge-app/linux/main.cpp index c708b256727520..24ad4aa9a1690c 100644 --- a/examples/fabric-bridge-app/linux/main.cpp +++ b/examples/fabric-bridge-app/linux/main.cpp @@ -19,6 +19,7 @@ #include #include "BridgedDevice.h" +#include "BridgedDeviceBasicInformationImpl.h" #include "BridgedDeviceManager.h" #include "CommissionableInit.h" @@ -48,6 +49,8 @@ constexpr uint16_t kPollIntervalMs = 100; constexpr uint16_t kRetryIntervalS = 3; #endif +BridgedDeviceBasicInformationImpl gBridgedDeviceBasicInformationAttributes; + bool KeyboardHit() { int bytesWaiting; @@ -175,6 +178,7 @@ void ApplicationInit() ChipLogDetail(NotSpecified, "Fabric-Bridge: ApplicationInit()"); CommandHandlerInterfaceRegistry::RegisterCommandHandler(&gAdministratorCommissioningCommandHandler); + registerAttributeAccessOverride(&gBridgedDeviceBasicInformationAttributes); #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE InitRpcServer(kFabricBridgeServerPort);