From 8d5589a0fc0db0eb23b58b433c730101cdbd4643 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 13 Dec 2021 20:07:13 -0500 Subject: [PATCH] Create an iterator over endpoints matching a cluster. (#12955) Very slightly larger code, but nicer API than the current callback function setup. --- src/app/clusters/basic/basic.cpp | 31 ++-- .../general-diagnostics-server.cpp | 168 +++++++----------- .../operational-credentials-server.cpp | 10 +- .../software-diagnostics-server.cpp | 29 ++- .../wifi-network-diagnostics-server.cpp | 77 ++++---- src/app/util/af.h | 54 ++++-- src/app/util/attribute-storage.cpp | 27 +-- 7 files changed, 177 insertions(+), 219 deletions(-) diff --git a/src/app/clusters/basic/basic.cpp b/src/app/clusters/basic/basic.cpp index ea34b025d5a5a6..38ccf5ecc31f96 100644 --- a/src/app/clusters/basic/basic.cpp +++ b/src/app/clusters/basic/basic.cpp @@ -44,21 +44,17 @@ class PlatformMgrDelegate : public DeviceLayer::PlatformManagerDelegate { ChipLogProgress(Zcl, "PlatformMgrDelegate: OnStartUp"); - ForAllEndpointsWithServerCluster( - Basic::Id, - [](EndpointId endpoint, intptr_t context) -> Loop { - // If Basic cluster is implemented on this endpoint - Events::StartUp::Type event{ static_cast(context) }; - EventNumber eventNumber; - - if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber, EventOptions::Type::kUrgent)) - { - ChipLogError(Zcl, "PlatformMgrDelegate: Failed to record StartUp event"); - } + for (auto endpoint : EnabledEndpointsWithServerCluster(Basic::Id)) + { + // If Basic cluster is implemented on this endpoint + Events::StartUp::Type event{ softwareVersion }; + EventNumber eventNumber; - return Loop::Continue; - }, - static_cast(softwareVersion)); + if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber, EventOptions::Type::kUrgent)) + { + ChipLogError(Zcl, "PlatformMgrDelegate: Failed to record StartUp event"); + } + } } // Gets called by the current Node prior to any orderly shutdown sequence on a best-effort basis. @@ -66,7 +62,8 @@ class PlatformMgrDelegate : public DeviceLayer::PlatformManagerDelegate { ChipLogProgress(Zcl, "PlatformMgrDelegate: OnShutDown"); - ForAllEndpointsWithServerCluster(Basic::Id, [](EndpointId endpoint, intptr_t context) -> Loop { + for (auto endpoint : EnabledEndpointsWithServerCluster(Basic::Id)) + { // If Basic cluster is implemented on this endpoint Events::ShutDown::Type event; EventNumber eventNumber; @@ -75,9 +72,7 @@ class PlatformMgrDelegate : public DeviceLayer::PlatformManagerDelegate { ChipLogError(Zcl, "PlatformMgrDelegate: Failed to record ShutDown event"); } - - return Loop::Continue; - }); + } } }; diff --git a/src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp b/src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp index 59f361a1eda6c3..a67a5b4f159b30 100644 --- a/src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp +++ b/src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp @@ -180,13 +180,10 @@ class GeneralDiagnosticsDelegate : public DeviceLayer::ConnectivityManagerDelega { static void ReportAttributeOnAllEndpoints(AttributeId attribute) { - ForAllEndpointsWithServerCluster( - GeneralDiagnostics::Id, - [](EndpointId endpoint, intptr_t context) -> Loop { - MatterReportingAttributeChangeCallback(endpoint, GeneralDiagnostics::Id, static_cast(context)); - return Loop::Continue; - }, - static_cast(attribute)); + for (auto endpoint : EnabledEndpointsWithServerCluster(GeneralDiagnostics::Id)) + { + MatterReportingAttributeChangeCallback(endpoint, GeneralDiagnostics::Id, attribute); + } } // Gets called when any network interface on the Node is updated. @@ -205,118 +202,83 @@ class GeneralDiagnosticsDelegate : public DeviceLayer::ConnectivityManagerDelega ReportAttributeOnAllEndpoints(GeneralDiagnostics::Attributes::BootReasons::Id); } - template - struct TFaults - { - GeneralFaults & previous; - GeneralFaults & current; - }; - // Get called when the Node detects a hardware fault has been raised. - void OnHardwareFaultsDetected(GeneralFaults & previousArg, - GeneralFaults & currentArg) override + void OnHardwareFaultsDetected(GeneralFaults & previous, + GeneralFaults & current) override { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnHardwareFaultsDetected"); - using Faults = TFaults; - Faults faults = { previousArg, currentArg }; - ForAllEndpointsWithServerCluster( - GeneralDiagnostics::Id, - [](EndpointId endpointId, intptr_t context) -> Loop { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::ActiveHardwareFaults::Id); - - auto * faultsPtr = reinterpret_cast(context); - auto & current = faultsPtr->current; - auto & previous = faultsPtr->previous; - // Record HardwareFault event - EventNumber eventNumber; - DataModel::List currentList = DataModel::List( - reinterpret_cast(current.data()), current.size()); - DataModel::List previousList = DataModel::List( - reinterpret_cast(previous.data()), previous.size()); - Events::HardwareFaultChange::Type event{ currentList, previousList }; - - if (CHIP_NO_ERROR != LogEvent(event, endpointId, eventNumber, EventOptions::Type::kUrgent)) - { - ChipLogError(Zcl, "GeneralDiagnosticsDelegate: Failed to record HardwareFault event"); - } - - return Loop::Continue; - }, - reinterpret_cast(&faults)); + for (auto endpointId : EnabledEndpointsWithServerCluster(GeneralDiagnostics::Id)) + { + // If General Diagnostics cluster is implemented on this endpoint + MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, + GeneralDiagnostics::Attributes::ActiveHardwareFaults::Id); + + // Record HardwareFault event + EventNumber eventNumber; + DataModel::List currentList = DataModel::List( + reinterpret_cast(current.data()), current.size()); + DataModel::List previousList = DataModel::List( + reinterpret_cast(previous.data()), previous.size()); + Events::HardwareFaultChange::Type event{ currentList, previousList }; + + if (CHIP_NO_ERROR != LogEvent(event, endpointId, eventNumber, EventOptions::Type::kUrgent)) + { + ChipLogError(Zcl, "GeneralDiagnosticsDelegate: Failed to record HardwareFault event"); + } + } } // Get called when the Node detects a radio fault has been raised. - void OnRadioFaultsDetected(GeneralFaults & previousArg, GeneralFaults & currentArg) override + void OnRadioFaultsDetected(GeneralFaults & previous, GeneralFaults & current) override { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnHardwareFaultsDetected"); - using Faults = TFaults; - Faults faults = { previousArg, currentArg }; - ForAllEndpointsWithServerCluster( - GeneralDiagnostics::Id, - [](EndpointId endpointId, intptr_t context) -> Loop { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::ActiveRadioFaults::Id); - - auto * faultsPtr = reinterpret_cast(context); - auto & current = faultsPtr->current; - auto & previous = faultsPtr->previous; - // Record RadioFault event - EventNumber eventNumber; - DataModel::List currentList = - DataModel::List(reinterpret_cast(current.data()), current.size()); - DataModel::List previousList = DataModel::List( - reinterpret_cast(previous.data()), previous.size()); - Events::RadioFaultChange::Type event{ currentList, previousList }; - - if (CHIP_NO_ERROR != LogEvent(event, endpointId, eventNumber, EventOptions::Type::kUrgent)) - { - ChipLogError(Zcl, "GeneralDiagnosticsDelegate: Failed to record RadioFault event"); - } - - return Loop::Continue; - }, - reinterpret_cast(&faults)); + for (auto endpointId : EnabledEndpointsWithServerCluster(GeneralDiagnostics::Id)) + { + // If General Diagnostics cluster is implemented on this endpoint + MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, + GeneralDiagnostics::Attributes::ActiveRadioFaults::Id); + + // Record RadioFault event + EventNumber eventNumber; + DataModel::List currentList = + DataModel::List(reinterpret_cast(current.data()), current.size()); + DataModel::List previousList = + DataModel::List(reinterpret_cast(previous.data()), previous.size()); + Events::RadioFaultChange::Type event{ currentList, previousList }; + + if (CHIP_NO_ERROR != LogEvent(event, endpointId, eventNumber, EventOptions::Type::kUrgent)) + { + ChipLogError(Zcl, "GeneralDiagnosticsDelegate: Failed to record RadioFault event"); + } + } } // Get called when the Node detects a network fault has been raised. - void OnNetworkFaultsDetected(GeneralFaults & previousArg, - GeneralFaults & currentArg) override + void OnNetworkFaultsDetected(GeneralFaults & previous, GeneralFaults & current) override { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnHardwareFaultsDetected"); - using Faults = TFaults; - Faults faults = { previousArg, currentArg }; - ForAllEndpointsWithServerCluster( - GeneralDiagnostics::Id, - [](EndpointId endpointId, intptr_t context) -> Loop { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::ActiveNetworkFaults::Id); - - auto * faultsPtr = reinterpret_cast(context); - auto & current = faultsPtr->current; - auto & previous = faultsPtr->previous; - // Record NetworkFault event - EventNumber eventNumber; - DataModel::List currentList = DataModel::List( - reinterpret_cast(current.data()), current.size()); - DataModel::List previousList = DataModel::List( - reinterpret_cast(previous.data()), previous.size()); - Events::NetworkFaultChange::Type event{ currentList, previousList }; - - if (CHIP_NO_ERROR != LogEvent(event, endpointId, eventNumber, EventOptions::Type::kUrgent)) - { - ChipLogError(Zcl, "GeneralDiagnosticsDelegate: Failed to record NetworkFault event"); - } - - return Loop::Continue; - }, - reinterpret_cast(&faults)); + for (auto endpointId : EnabledEndpointsWithServerCluster(GeneralDiagnostics::Id)) + { + // If General Diagnostics cluster is implemented on this endpoint + MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, + GeneralDiagnostics::Attributes::ActiveNetworkFaults::Id); + + // Record NetworkFault event + EventNumber eventNumber; + DataModel::List currentList = + DataModel::List(reinterpret_cast(current.data()), current.size()); + DataModel::List previousList = DataModel::List( + reinterpret_cast(previous.data()), previous.size()); + Events::NetworkFaultChange::Type event{ currentList, previousList }; + + if (CHIP_NO_ERROR != LogEvent(event, endpointId, eventNumber, EventOptions::Type::kUrgent)) + { + ChipLogError(Zcl, "GeneralDiagnosticsDelegate: Failed to record NetworkFault event"); + } + } } }; diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index c985701bcc5371..5725f72ddb80b4 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -230,8 +230,10 @@ class OpCredsFabricTableDelegate : public FabricTableDelegate emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Fabric 0x%" PRIu8 " was deleted from fabric storage.", fabricId); fabricListChanged(); - // The Leave event SHOULD be emitted by a Node prior to permanently leaving the Fabric. - ForAllEndpointsWithServerCluster(Basic::Id, [](EndpointId endpoint, intptr_t context) -> Loop { + // The Leave event SHOULD be emitted by a Node prior to permanently + // leaving the Fabric. + for (auto endpoint : EnabledEndpointsWithServerCluster(Basic::Id)) + { // If Basic cluster is implemented on this endpoint Basic::Events::Leave::Type event; EventNumber eventNumber; @@ -240,9 +242,7 @@ class OpCredsFabricTableDelegate : public FabricTableDelegate { ChipLogError(Zcl, "OpCredsFabricTableDelegate: Failed to record Leave event"); } - - return Loop::Continue; - }); + } } // Gets called when a fabric is loaded into the FabricTable from KVS store. diff --git a/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp b/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp index 4a8c99d0a77440..7fd9581a3d59a6 100644 --- a/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp +++ b/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp @@ -132,24 +132,17 @@ class SoftwareDiagnosticsDelegate : public DeviceLayer::SoftwareDiagnosticsDeleg { ChipLogProgress(Zcl, "SoftwareDiagnosticsDelegate: OnSoftwareFaultDetected"); - ForAllEndpointsWithServerCluster( - SoftwareDiagnostics::Id, - [](EndpointId endpoint, intptr_t context) -> Loop { - // If Software Diagnostics cluster is implemented on this endpoint - SoftwareDiagnostics::Structs::SoftwareFault::Type * pSoftwareFault = - reinterpret_cast(context); - - EventNumber eventNumber; - Events::SoftwareFault::Type event{ *pSoftwareFault }; - - if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) - { - ChipLogError(Zcl, "SoftwareDiagnosticsDelegate: Failed to record SoftwareFault event"); - } - - return Loop::Continue; - }, - reinterpret_cast(&softwareFault)); + for (auto endpoint : EnabledEndpointsWithServerCluster(SoftwareDiagnostics::Id)) + { + // If Software Diagnostics cluster is implemented on this endpoint + EventNumber eventNumber; + Events::SoftwareFault::Type event{ softwareFault }; + + if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + { + ChipLogError(Zcl, "SoftwareDiagnosticsDelegate: Failed to record SoftwareFault event"); + } + } } }; diff --git a/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp b/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp index 0d7ddc374fac7b..6149efe26b8f72 100644 --- a/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp +++ b/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp @@ -154,21 +154,17 @@ class WiFiDiagnosticsDelegate : public DeviceLayer::WiFiDiagnosticsDelegate { ChipLogProgress(Zcl, "WiFiDiagnosticsDelegate: OnDisconnectionDetected"); - ForAllEndpointsWithServerCluster( - WiFiNetworkDiagnostics::Id, - [](EndpointId endpoint, intptr_t context) -> Loop { - // If WiFi Network Diagnostics cluster is implemented on this endpoint - Events::Disconnection::Type event{ static_cast(context) }; - EventNumber eventNumber; - - if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) - { - ChipLogError(Zcl, "WiFiDiagnosticsDelegate: Failed to record Disconnection event"); - } - - return Loop::Continue; - }, - static_cast(reasonCode)); + for (auto endpoint : EnabledEndpointsWithServerCluster(WiFiNetworkDiagnostics::Id)) + { + // If WiFi Network Diagnostics cluster is implemented on this endpoint + Events::Disconnection::Type event{ reasonCode }; + EventNumber eventNumber; + + if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + { + ChipLogError(Zcl, "WiFiDiagnosticsDelegate: Failed to record Disconnection event"); + } + } } // Gets called when the Node fails to associate or authenticate an access point. @@ -178,21 +174,16 @@ class WiFiDiagnosticsDelegate : public DeviceLayer::WiFiDiagnosticsDelegate Events::AssociationFailure::Type event{ static_cast(associationFailureCause), status }; - ForAllEndpointsWithServerCluster( - WiFiNetworkDiagnostics::Id, - [](EndpointId endpoint, intptr_t context) -> Loop { - // If WiFi Network Diagnostics cluster is implemented on this endpoint - Events::AssociationFailure::Type * pEvent = reinterpret_cast(context); - EventNumber eventNumber; - - if (CHIP_NO_ERROR != LogEvent(*pEvent, endpoint, eventNumber)) - { - ChipLogError(Zcl, "WiFiDiagnosticsDelegate: Failed to record AssociationFailure event"); - } - - return Loop::Continue; - }, - reinterpret_cast(&event)); + for (auto endpoint : EnabledEndpointsWithServerCluster(WiFiNetworkDiagnostics::Id)) + { + // If WiFi Network Diagnostics cluster is implemented on this endpoint + EventNumber eventNumber; + + if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + { + ChipLogError(Zcl, "WiFiDiagnosticsDelegate: Failed to record AssociationFailure event"); + } + } } // Gets when the Node’s connection status to a Wi-Fi network has changed. @@ -200,21 +191,17 @@ class WiFiDiagnosticsDelegate : public DeviceLayer::WiFiDiagnosticsDelegate { ChipLogProgress(Zcl, "WiFiDiagnosticsDelegate: OnConnectionStatusChanged"); - ForAllEndpointsWithServerCluster( - WiFiNetworkDiagnostics::Id, - [](EndpointId endpoint, intptr_t context) -> Loop { - // If WiFi Network Diagnostics cluster is implemented on this endpoint - Events::ConnectionStatus::Type event{ static_cast(context) }; - EventNumber eventNumber; - - if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) - { - ChipLogError(Zcl, "WiFiDiagnosticsDelegate: Failed to record ConnectionStatus event"); - } - - return Loop::Continue; - }, - static_cast(connectionStatus)); + Events::ConnectionStatus::Type event{ static_cast(connectionStatus) }; + for (auto endpoint : EnabledEndpointsWithServerCluster(WiFiNetworkDiagnostics::Id)) + { + // If WiFi Network Diagnostics cluster is implemented on this endpoint + EventNumber eventNumber; + + if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + { + ChipLogError(Zcl, "WiFiDiagnosticsDelegate: Failed to record ConnectionStatus event"); + } + } } }; diff --git a/src/app/util/af.h b/src/app/util/af.h index d9586f79f71560..9520b1ab588c71 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -159,24 +159,6 @@ bool emberAfContainsServer(chip::EndpointId endpoint, chip::ClusterId clusterId) */ bool emberAfContainsServerFromIndex(uint16_t index, chip::ClusterId clusterId); -namespace chip { -namespace app { - -using EndpointCallback = Loop (*)(EndpointId endpoint, intptr_t context); - -/** - * @brief calls user-supplied function for every endpoint that has the given - * server cluster, until either the function returns Loop::Break or we run out - * of endpoints. - * - * Returns Loop::Break if the callee did, or Loop::Finished if we ran out of - * endpoints. - */ -Loop ForAllEndpointsWithServerCluster(ClusterId clusterId, EndpointCallback callback, intptr_t context = 0); - -} // namespace app -} // namespace chip - /** * @brief Returns true if endpoint contains cluster client. * @@ -1823,3 +1805,39 @@ int emberAfMain(MAIN_FUNCTION_PARAMETERS); * generated code. */ EmberAfStatus emberAfClusterSpecificCommandParse(EmberAfClusterCommand * cmd); + +namespace chip { +namespace app { + +class EnabledEndpointsWithServerCluster +{ +public: + EnabledEndpointsWithServerCluster(ClusterId clusterId); + + // Instead of having a separate Iterator class, optimize for codesize by + // just reusing ourselves as our own iterator. We could do a bit better + // here with C++17 and using a different type for the end iterator, but this + // is the best I've found with C++14 so far. + // + // This does mean that you can only iterate a given + // EnabledEndpointsWithServerCluster once, but that's OK given how we use it + // in practice. + EnabledEndpointsWithServerCluster & begin() { return *this; } + const EnabledEndpointsWithServerCluster & end() const { return *this; } + + bool operator!=(const EnabledEndpointsWithServerCluster & other) const { return mEndpointIndex != mEndpointCount; } + + EnabledEndpointsWithServerCluster & operator++(); + + EndpointId operator*() const { return emberAfEndpointFromIndex(mEndpointIndex); } + +private: + void EnsureMatchingEndpoint(); + + uint16_t mEndpointIndex = 0; + uint16_t mEndpointCount = emberAfEndpointCount(); + ClusterId mClusterId; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index cb86bf94b7e812..ce507bc0aec556 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -775,28 +775,31 @@ bool emberAfContainsServerFromIndex(uint16_t index, ClusterId clusterId) namespace chip { namespace app { -Loop ForAllEndpointsWithServerCluster(ClusterId clusterId, EndpointCallback callback, intptr_t context) +EnabledEndpointsWithServerCluster::EnabledEndpointsWithServerCluster(ClusterId clusterId) : mClusterId(clusterId) { - uint16_t count = emberAfEndpointCount(); - for (uint16_t index = 0; index < count; ++index) + EnsureMatchingEndpoint(); +} +EnabledEndpointsWithServerCluster & EnabledEndpointsWithServerCluster::operator++() +{ + ++mEndpointIndex; + EnsureMatchingEndpoint(); + return *this; +} + +void EnabledEndpointsWithServerCluster::EnsureMatchingEndpoint() +{ + for (; mEndpointIndex < mEndpointCount; ++mEndpointIndex) { - if (!emberAfEndpointIndexIsEnabled(index)) + if (!emberAfEndpointIndexIsEnabled(mEndpointIndex)) { continue; } - if (!emberAfContainsServerFromIndex(index, clusterId)) + if (!emberAfContainsServerFromIndex(mEndpointIndex, mClusterId)) { continue; } - - if (callback(emberAfEndpointFromIndex(index), context) == Loop::Break) - { - return Loop::Break; - } } - - return Loop::Finish; } } // namespace app