From 12b3906ab5ae8ca718991e1495d1d745d62f17be Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 2 Dec 2021 22:43:41 -0500 Subject: [PATCH] Introduce a way to loop over endpoints that have a given server cluster. We have several places that are doing this, all of them in not-so-great ways (e.g. mapping indices to endpoint ids and then back to indices, which is O(N^2) in number of endpoints). Better to have a utility that does it right. --- .../general_diagnostics_server.cpp | 87 ++++--------------- .../software_diagnostics_server.cpp | 17 +--- src/app/util/af.h | 19 ++++ src/app/util/attribute-storage.cpp | 30 +++++++ src/lib/support/BUILD.gn | 1 + src/lib/support/Iterators.h | 43 +++++++++ src/lib/support/Pool.h | 9 +- 7 files changed, 114 insertions(+), 92 deletions(-) create mode 100644 src/lib/support/Iterators.h 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 688c3ba1ef6722..95c85f51345e16 100644 --- a/src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp +++ b/src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp @@ -169,26 +169,23 @@ CHIP_ERROR GeneralDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & a class GeneralDiagnosticsDelegate : public DeviceLayer::ConnectivityManagerDelegate, public DeviceLayer::GeneralDiagnosticsDelegate { + static void ReportAttributeOnAllEndpoints(AttributeId attribute) + { + ForAllEndpointsWithServerCluster( + GeneralDiagnostics::Id, + [](EndpointId endpoint, intptr_t context) -> Loop { + MatterReportingAttributeChangeCallback(endpoint, GeneralDiagnostics::Id, static_cast(context)); + return Loop::Continue; + }, + attribute); + } // Gets called when any network interface on the Node is updated. void OnNetworkInfoChanged() override { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnNetworkInfoChanged"); - for (uint16_t index = 0; index < emberAfEndpointCount(); index++) - { - if (emberAfEndpointIndexIsEnabled(index)) - { - EndpointId endpointId = emberAfEndpointFromIndex(index); - - if (emberAfContainsServer(endpointId, GeneralDiagnostics::Id)) - { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::NetworkInterfaces::Id); - } - } - } + ReportAttributeOnAllEndpoints(GeneralDiagnostics::Attributes::NetworkInterfaces::Id); } // Gets called when the device has been rebooted. @@ -196,22 +193,7 @@ class GeneralDiagnosticsDelegate : public DeviceLayer::ConnectivityManagerDelega { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnDeviceRebooted"); - for (uint16_t index = 0; index < emberAfEndpointCount(); index++) - { - if (emberAfEndpointIndexIsEnabled(index)) - { - EndpointId endpointId = emberAfEndpointFromIndex(index); - - if (emberAfContainsServer(endpointId, GeneralDiagnostics::Id)) - { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::RebootCount::Id); - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::BootReasons::Id); - } - } - } + ReportAttributeOnAllEndpoints(GeneralDiagnostics::Attributes::BootReasons::Id); } // Get called when the Node detects a hardware fault has been raised. @@ -219,20 +201,7 @@ class GeneralDiagnosticsDelegate : public DeviceLayer::ConnectivityManagerDelega { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnHardwareFaultsDetected"); - for (uint16_t index = 0; index < emberAfEndpointCount(); index++) - { - if (emberAfEndpointIndexIsEnabled(index)) - { - EndpointId endpointId = emberAfEndpointFromIndex(index); - - if (emberAfContainsServer(endpointId, GeneralDiagnostics::Id)) - { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::ActiveHardwareFaults::Id); - } - } - } + ReportAttributeOnAllEndpoints(GeneralDiagnostics::Attributes::ActiveHardwareFaults::Id); } // Get called when the Node detects a radio fault has been raised. @@ -240,20 +209,7 @@ class GeneralDiagnosticsDelegate : public DeviceLayer::ConnectivityManagerDelega { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnHardwareFaultsDetected"); - for (uint16_t index = 0; index < emberAfEndpointCount(); index++) - { - if (emberAfEndpointIndexIsEnabled(index)) - { - EndpointId endpointId = emberAfEndpointFromIndex(index); - - if (emberAfContainsServer(endpointId, GeneralDiagnostics::Id)) - { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::ActiveRadioFaults::Id); - } - } - } + ReportAttributeOnAllEndpoints(GeneralDiagnostics::Attributes::ActiveRadioFaults::Id); } // Get called when the Node detects a network fault has been raised. @@ -261,20 +217,7 @@ class GeneralDiagnosticsDelegate : public DeviceLayer::ConnectivityManagerDelega { ChipLogProgress(Zcl, "GeneralDiagnosticsDelegate: OnHardwareFaultsDetected"); - for (uint16_t index = 0; index < emberAfEndpointCount(); index++) - { - if (emberAfEndpointIndexIsEnabled(index)) - { - EndpointId endpointId = emberAfEndpointFromIndex(index); - - if (emberAfContainsServer(endpointId, GeneralDiagnostics::Id)) - { - // If General Diagnostics cluster is implemented on this endpoint - MatterReportingAttributeChangeCallback(endpointId, GeneralDiagnostics::Id, - GeneralDiagnostics::Attributes::ActiveNetworkFaults::Id); - } - } - } + ReportAttributeOnAllEndpoints(GeneralDiagnostics::Attributes::ActiveNetworkFaults::Id); } }; 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 8d93b297ab57b9..f0b4b5b151f8df 100644 --- a/src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp +++ b/src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp @@ -131,19 +131,10 @@ class SoftwareDiagnosticsDelegate : public DeviceLayer::SoftwareDiagnosticsDeleg { ChipLogProgress(Zcl, "SoftwareDiagnosticsDelegate: OnSoftwareFaultDetected"); - for (uint16_t index = 0; index < emberAfEndpointCount(); index++) - { - if (emberAfEndpointIndexIsEnabled(index)) - { - EndpointId endpointId = emberAfEndpointFromIndex(index); - - if (emberAfContainsServer(endpointId, SoftwareDiagnostics::Id)) - { - // If Software Diagnostics cluster is implemented on this endpoint - // TODO: Log SoftwareFault event - } - } - } + ForAllEndpointsWithServerCluster(GeneralDiagnostics::Id, [](EndpointId endpoint, intptr_t) -> Loop { + // TODO: Log SoftwareFault event and walk them all. + return Loop::Break; + }); } }; diff --git a/src/app/util/af.h b/src/app/util/af.h index 08a1d94a951ff0..f4e981e94cd1b1 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -72,6 +72,7 @@ #include #include +#include #include /** @name Attribute Storage */ @@ -158,6 +159,24 @@ 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. * diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 7cdd712a33ec12..c512cbf1e56363 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -836,6 +836,36 @@ bool emberAfContainsServerFromIndex(uint16_t index, ClusterId clusterId) } } +namespace chip { +namespace app { + +Loop ForAllEndpointsWithServerCluster(ClusterId clusterId, EndpointCallback callback, intptr_t context) +{ + uint16_t count = emberAfEndpointCount(); + for (uint16_t index = 0; index < count; ++index) + { + if (!emberAfEndpointIndexIsEnabled(index)) + { + continue; + } + + if (!emberAfContainsServerFromIndex(index, clusterId)) + { + continue; + } + + if (callback(emberAfEndpointFromIndex(index), context) == Loop::Break) + { + return Loop::Break; + } + } + + return Loop::Finish; +} + +} // namespace app +} // namespace chip + // Finds the cluster that matches endpoint, clusterId, direction, and manufacturerCode. EmberAfCluster * emberAfFindClusterWithMfgCode(EndpointId endpoint, ClusterId clusterId, EmberAfClusterMask mask, uint16_t manufacturerCode) diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn index 2c5e651db49830..fe8c92dd4909e9 100644 --- a/src/lib/support/BUILD.gn +++ b/src/lib/support/BUILD.gn @@ -78,6 +78,7 @@ static_library("support") { "FibonacciUtils.h", "FixedBufferAllocator.cpp", "FixedBufferAllocator.h", + "Iterators.h", "LifetimePersistedCounter.cpp", "LifetimePersistedCounter.h", "ObjectLifeCycle.h", diff --git a/src/lib/support/Iterators.h b/src/lib/support/Iterators.h new file mode 100644 index 00000000000000..6f622e06c69c11 --- /dev/null +++ b/src/lib/support/Iterators.h @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2021 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 + +namespace chip { + +/** + * Enum used to control iteration (e.g. via a callback function that is called + * for each of a set of elements). + * + * When used as the callback return type: + * Continue: Continue the iteration. + * Break: Stop the iteration. + * + * When used as the return type of the entire iteration procedure: + * Break: Some callback returned Break. + * Finish: All callbacks returned Continue. + */ +enum class Loop : uint8_t +{ + Continue, + Break, + Finish, +}; + +} // namespace chip diff --git a/src/lib/support/Pool.h b/src/lib/support/Pool.h index 24605ea2425ace..679ea1f9e37fb2 100644 --- a/src/lib/support/Pool.h +++ b/src/lib/support/Pool.h @@ -24,6 +24,8 @@ #include +#include + #include #include #include @@ -32,13 +34,6 @@ namespace chip { -enum class Loop : uint8_t -{ - Continue, - Break, - Finish, -}; - namespace internal { class Statistics