From 81e3c92b923295f7e5ae98f3bf00642cac29e71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Thu, 24 Nov 2022 15:10:27 +0100 Subject: [PATCH 01/14] Ability to notify specific binding and disable connection on init --- src/app/clusters/bindings/BindingManager.cpp | 47 +++++++++++++++++--- src/app/clusters/bindings/BindingManager.h | 13 ++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index ad13376a0dfe42..2356df125647db 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -83,13 +83,16 @@ CHIP_ERROR BindingManager::Init(const BindingManagerInitParams & params) } else { - for (const EmberBindingTableEntry & entry : BindingTable::GetInstance()) + if (params.establishConnectionOnInit) { - if (entry.type == EMBER_UNICAST_BINDING) + for (const EmberBindingTableEntry & entry : BindingTable::GetInstance()) { - // The CASE connection can also fail if the unicast peer is offline. - // There is recovery mechanism to retry connection on-demand so ignore error. - (void) UnicastBindingCreated(entry.fabricIndex, entry.nodeId); + if (entry.type == EMBER_UNICAST_BINDING) + { + // The CASE connection can also fail if the unicast peer is offline. + // There is recovery mechanism to retry connection on-demand so ignore error. + (void) UnicastBindingCreated(entry.fabricIndex, entry.nodeId); + } } } } @@ -201,4 +204,38 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste return error; } +CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingTableIndex, void * context) +{ + VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mBoundDeviceChangedHandler, CHIP_NO_ERROR); + + CHIP_ERROR error = CHIP_NO_ERROR; + auto * bindingContext = mPendingNotificationMap.NewPendingNotificationContext(context); + VerifyOrReturnError(bindingContext != nullptr, CHIP_ERROR_NO_MEMORY); + + bindingContext->IncrementConsumersNumber(); + + EmberBindingTableEntry entry = BindingTable::GetInstance().GetAt(bindingTableIndex); + + if (entry.local == endpoint && (!entry.clusterId.HasValue() || entry.clusterId.Value() == cluster)) + { + if (entry.type == EMBER_UNICAST_BINDING) + { + error = mPendingNotificationMap.AddPendingNotification(bindingTableIndex, bindingContext); + SuccessOrExit(error); + error = EstablishConnection(ScopedNodeId(entry.nodeId, entry.fabricIndex)); + SuccessOrExit(error); + } + else if (entry.type == EMBER_MULTICAST_BINDING) + { + mBoundDeviceChangedHandler(entry, nullptr, bindingContext->GetContext()); + } + } + +exit: + bindingContext->DecrementConsumersNumber(); + + return error; +} + } // namespace chip diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index 646281f6718e4f..dda06655941593 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -53,6 +53,7 @@ struct BindingManagerInitParams FabricTable * mFabricTable = nullptr; CASESessionManager * mCASESessionManager = nullptr; PersistentStorageDelegate * mStorage = nullptr; + bool establishConnectionOnInit = true; }; /** @@ -118,6 +119,18 @@ class BindingManager */ CHIP_ERROR NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, void * context); + /* + * Notify a cluster change to a specific bound device associated with the (endpoint, cluster) tuple. + * + * For a unicast bindings with an active session and multicast bindings, the BoundDeviceChangedHandler + * will be called before the function returns. + * + * For unicast bindings without an active session, the notification will be queued and a new session will + * be initiated. The BoundDeviceChangedHandler will be called once the session is established. + * + */ + CHIP_ERROR NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, void * context); + static BindingManager & GetInstance() { return sBindingManager; } private: From 2b54aa77ef838eecc6352c9f32e3fca57efcdb1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Thu, 24 Nov 2022 15:16:13 +0100 Subject: [PATCH 02/14] Aligned param naming --- src/app/clusters/bindings/BindingManager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index 2356df125647db..c469d15f64b2a6 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -204,7 +204,7 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste return error; } -CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingTableIndex, void * context) +CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, void * context) { VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mBoundDeviceChangedHandler, CHIP_NO_ERROR); @@ -215,13 +215,13 @@ CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, bindingContext->IncrementConsumersNumber(); - EmberBindingTableEntry entry = BindingTable::GetInstance().GetAt(bindingTableIndex); + EmberBindingTableEntry entry = BindingTable::GetInstance().GetAt(bindingIndex); if (entry.local == endpoint && (!entry.clusterId.HasValue() || entry.clusterId.Value() == cluster)) { if (entry.type == EMBER_UNICAST_BINDING) { - error = mPendingNotificationMap.AddPendingNotification(bindingTableIndex, bindingContext); + error = mPendingNotificationMap.AddPendingNotification(bindingIndex, bindingContext); SuccessOrExit(error); error = EstablishConnection(ScopedNodeId(entry.nodeId, entry.fabricIndex)); SuccessOrExit(error); From 56f3a7bd60f26711b60e110a91381e50c9b63f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Thu, 24 Nov 2022 15:38:57 +0100 Subject: [PATCH 03/14] Fixes from restyle --- src/app/clusters/bindings/BindingManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index c469d15f64b2a6..d0420e8b2bf7f8 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -204,7 +204,8 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste return error; } -CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, void * context) +CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, + void * context) { VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mBoundDeviceChangedHandler, CHIP_NO_ERROR); From 0a726c5ae9386de7a199704d7ee7b6088bc0b568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Thu, 24 Nov 2022 15:41:22 +0100 Subject: [PATCH 04/14] Missing restyle --- src/app/clusters/bindings/BindingManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index d0420e8b2bf7f8..c19df96cfd25a7 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -204,7 +204,7 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste return error; } -CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, +CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, void * context) { VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE); From b2420fbf7286f2af2de0d58e38f3b4dc15b676a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Thu, 5 Jan 2023 12:57:31 +0100 Subject: [PATCH 05/14] Changed member name and added documentation --- src/app/clusters/bindings/BindingManager.cpp | 5 ++++- src/app/clusters/bindings/BindingManager.h | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index c19df96cfd25a7..2935dce21d661c 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -83,7 +83,10 @@ CHIP_ERROR BindingManager::Init(const BindingManagerInitParams & params) } else { - if (params.establishConnectionOnInit) + // In case the application does not want the BindingManager to establish a CASE session + // to the available bindings, it can be disabled by setting mEstablishConnectionOnInit + // to false. + if (params.mEstablishConnectionOnInit) { for (const EmberBindingTableEntry & entry : BindingTable::GetInstance()) { diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index dda06655941593..adae93c08e2c03 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -53,7 +53,7 @@ struct BindingManagerInitParams FabricTable * mFabricTable = nullptr; CASESessionManager * mCASESessionManager = nullptr; PersistentStorageDelegate * mStorage = nullptr; - bool establishConnectionOnInit = true; + bool mEstablishConnectionOnInit = true; }; /** @@ -62,6 +62,7 @@ struct BindingManagerInitParams * when a binding is ready to be communicated with. * * A CASE connection will be triggered when: + * - During init of the BindingManager, unless the application activly disables this using mEstablishConnectionOnInit * - The binding cluster adds a unicast entry to the binding table. * - A watched cluster changes with a unicast binding but we cannot find an active connection to the peer. * From 1de95a65f0159d47f6085d21e11c283b0adc0853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Tue, 10 Jan 2023 16:22:00 +0100 Subject: [PATCH 06/14] Updated validation in bindings and removed AtIndex function --- src/app/clusters/bindings/BindingManager.cpp | 35 -------------- src/app/clusters/bindings/BindingManager.h | 12 ----- src/app/clusters/bindings/bindings.cpp | 49 ++++++++++++++++---- 3 files changed, 40 insertions(+), 56 deletions(-) diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index 2935dce21d661c..fc59b09b91c33d 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -207,39 +207,4 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste return error; } -CHIP_ERROR BindingManager::NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, - void * context) -{ - VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(mBoundDeviceChangedHandler, CHIP_NO_ERROR); - - CHIP_ERROR error = CHIP_NO_ERROR; - auto * bindingContext = mPendingNotificationMap.NewPendingNotificationContext(context); - VerifyOrReturnError(bindingContext != nullptr, CHIP_ERROR_NO_MEMORY); - - bindingContext->IncrementConsumersNumber(); - - EmberBindingTableEntry entry = BindingTable::GetInstance().GetAt(bindingIndex); - - if (entry.local == endpoint && (!entry.clusterId.HasValue() || entry.clusterId.Value() == cluster)) - { - if (entry.type == EMBER_UNICAST_BINDING) - { - error = mPendingNotificationMap.AddPendingNotification(bindingIndex, bindingContext); - SuccessOrExit(error); - error = EstablishConnection(ScopedNodeId(entry.nodeId, entry.fabricIndex)); - SuccessOrExit(error); - } - else if (entry.type == EMBER_MULTICAST_BINDING) - { - mBoundDeviceChangedHandler(entry, nullptr, bindingContext->GetContext()); - } - } - -exit: - bindingContext->DecrementConsumersNumber(); - - return error; -} - } // namespace chip diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index adae93c08e2c03..2512bcb5443e73 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -120,18 +120,6 @@ class BindingManager */ CHIP_ERROR NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, void * context); - /* - * Notify a cluster change to a specific bound device associated with the (endpoint, cluster) tuple. - * - * For a unicast bindings with an active session and multicast bindings, the BoundDeviceChangedHandler - * will be called before the function returns. - * - * For unicast bindings without an active session, the notification will be queued and a new session will - * be initiated. The BoundDeviceChangedHandler will be called once the session is established. - * - */ - CHIP_ERROR NotifyBoundClusterAtIndexChanged(EndpointId endpoint, ClusterId cluster, uint8_t bindingIndex, void * context); - static BindingManager & GetInstance() { return sBindingManager; } private: diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index a3b10c27f39bd5..4a347de07e22b4 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -45,6 +45,7 @@ class BindingTableAccess : public AttributeAccessInterface CHIP_ERROR Read(const ConcreteReadAttributePath & path, AttributeValueEncoder & encoder) override; CHIP_ERROR Write(const ConcreteDataAttributePath & path, AttributeValueDecoder & decoder) override; + void OnListWriteEnd(const app::ConcreteAttributePath & aPath, bool aWriteWasSuccessful) override; private: CHIP_ERROR ReadBindingTable(EndpointId endpoint, AttributeValueEncoder & encoder); @@ -55,19 +56,45 @@ class BindingTableAccess : public AttributeAccessInterface BindingTableAccess gAttrAccess; -bool IsValidBinding(const TargetStructType & entry) +bool IsValidBinding(const EndpointId localEndpoint, const TargetStructType & entry) { - return (!entry.group.HasValue() && entry.endpoint.HasValue() && entry.node.HasValue()) || - (!entry.endpoint.HasValue() && !entry.node.HasValue() && entry.group.HasValue()); + bool isValid = false; + + // Entry has endpoint, node id and no group id + if (!entry.group.HasValue() && entry.endpoint.HasValue() && entry.node.HasValue()) + { + if (entry.cluster.HasValue()) + { + if (emberAfContainsClient(localEndpoint, entry.cluster.Value())) + { + // Valid node/endpoint/cluster binding + isValid = true; + } + } + else + { + // Valid node/endpoint (no cluster id) binding + isValid = true; + } + } + // Entry has group id and no endpoint and node id + else if (!entry.endpoint.HasValue() && !entry.node.HasValue() && entry.group.HasValue()) + { + // Valid group binding + isValid = true; + } + + return isValid; } -CHIP_ERROR CheckValidBindingList(const DecodableBindingListType & bindingList, FabricIndex accessingFabricIndex) +CHIP_ERROR CheckValidBindingList(const EndpointId localEndpoint, const DecodableBindingListType & bindingList, + FabricIndex accessingFabricIndex) { size_t listSize = 0; auto iter = bindingList.begin(); while (iter.Next()) { - VerifyOrReturnError(IsValidBinding(iter.GetValue()), CHIP_IM_GLOBAL_STATUS(ConstraintError)); + VerifyOrReturnError(IsValidBinding(localEndpoint, iter.GetValue()), CHIP_IM_GLOBAL_STATUS(ConstraintError)); listSize++; } ReturnErrorOnFailure(iter.GetStatus()); @@ -159,6 +186,12 @@ CHIP_ERROR BindingTableAccess::Write(const ConcreteDataAttributePath & path, Att return CHIP_NO_ERROR; } +void BindingTableAccess::OnListWriteEnd(const app::ConcreteAttributePath & aPath, bool aWriteWasSuccessful) +{ + // Notify binding table has changed + LogErrorOnFailure(NotifyBindingsChanged()); +} + CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath & path, AttributeValueDecoder & decoder) { FabricIndex accessingFabricIndex = decoder.AccessingFabricIndex(); @@ -167,7 +200,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath DecodableBindingListType newBindingList; ReturnErrorOnFailure(decoder.Decode(newBindingList)); - ReturnErrorOnFailure(CheckValidBindingList(newBindingList, accessingFabricIndex)); + ReturnErrorOnFailure(CheckValidBindingList(path.mEndpointId, newBindingList, accessingFabricIndex)); // Clear all entries for the current accessing fabric and endpoint auto bindingTableIter = BindingTable::GetInstance().begin(); @@ -193,19 +226,17 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath { CreateBindingEntry(iter.GetValue(), path.mEndpointId); } - LogErrorOnFailure(NotifyBindingsChanged()); return CHIP_NO_ERROR; } if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem) { TargetStructType target; ReturnErrorOnFailure(decoder.Decode(target)); - if (!IsValidBinding(target)) + if (!IsValidBinding(path.mEndpointId, target)) { return CHIP_IM_GLOBAL_STATUS(ConstraintError); } CreateBindingEntry(target, path.mEndpointId); - LogErrorOnFailure(NotifyBindingsChanged()); return CHIP_NO_ERROR; } return CHIP_IM_GLOBAL_STATUS(UnsupportedWrite); From c399cc7c58e60d8b20a6467ebddb0a293cd76f4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Tue, 10 Jan 2023 16:43:43 +0100 Subject: [PATCH 07/14] Added missing include file --- src/app/clusters/bindings/bindings.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index 4a347de07e22b4..aab0af8bfa7f88 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include using namespace chip; From d6c5e4679378b11a108b85984fd9e1042ae36948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Tue, 10 Jan 2023 17:43:11 +0100 Subject: [PATCH 08/14] Fixed missing emberAfContainsClient by using other available functions --- src/app/clusters/bindings/bindings.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index aab0af8bfa7f88..444ff53791a324 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -66,7 +66,9 @@ bool IsValidBinding(const EndpointId localEndpoint, const TargetStructType & ent { if (entry.cluster.HasValue()) { - if (emberAfContainsClient(localEndpoint, entry.cluster.Value())) + // Determine if the client cluster is available on the endpoint + if (emberAfFindClusterInType(emberAfFindEndpointType(localEndpoint), entry.cluster.Value(), CLUSTER_MASK_CLIENT) != + nullptr) { // Valid node/endpoint/cluster binding isValid = true; From 8f795914de6d972dae99b38f602f47648cf228cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Tue, 10 Jan 2023 18:03:02 +0100 Subject: [PATCH 09/14] Added check of emberAfFindEndpointType in case it returns nullptr --- src/app/clusters/bindings/bindings.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index 444ff53791a324..46040bf5c94394 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -67,11 +68,14 @@ bool IsValidBinding(const EndpointId localEndpoint, const TargetStructType & ent if (entry.cluster.HasValue()) { // Determine if the client cluster is available on the endpoint - if (emberAfFindClusterInType(emberAfFindEndpointType(localEndpoint), entry.cluster.Value(), CLUSTER_MASK_CLIENT) != - nullptr) + const EmberAfEndpointType * emberEndpointType = emberAfFindEndpointType(localEndpoint); + if (emberEndpointType != nullptr) { - // Valid node/endpoint/cluster binding - isValid = true; + if (emberAfFindClusterInType(emberEndpointType, entry.cluster.Value(), CLUSTER_MASK_CLIENT) != nullptr) + { + // Valid node/endpoint/cluster binding + isValid = true; + } } } else From e8262ca2ce101882e7bea8558c33593669f9624f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Tue, 10 Jan 2023 19:27:58 +0100 Subject: [PATCH 10/14] Reintroduced emberAfContainsClient and reverted bindings to use this --- src/app/clusters/bindings/bindings.cpp | 12 +++--------- src/app/util/af.h | 8 ++++++++ src/app/util/attribute-storage.cpp | 12 ++++++++++++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index 46040bf5c94394..aab0af8bfa7f88 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -67,15 +66,10 @@ bool IsValidBinding(const EndpointId localEndpoint, const TargetStructType & ent { if (entry.cluster.HasValue()) { - // Determine if the client cluster is available on the endpoint - const EmberAfEndpointType * emberEndpointType = emberAfFindEndpointType(localEndpoint); - if (emberEndpointType != nullptr) + if (emberAfContainsClient(localEndpoint, entry.cluster.Value())) { - if (emberAfFindClusterInType(emberEndpointType, entry.cluster.Value(), CLUSTER_MASK_CLIENT) != nullptr) - { - // Valid node/endpoint/cluster binding - isValid = true; - } + // Valid node/endpoint/cluster binding + isValid = true; } } else diff --git a/src/app/util/af.h b/src/app/util/af.h index 9c9bad4b3f1345..9bc5dbca599bda 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -80,6 +80,14 @@ bool emberAfContainsServer(chip::EndpointId endpoint, chip::ClusterId clusterId) */ bool emberAfContainsServerFromIndex(uint16_t index, chip::ClusterId clusterId); +/** + * @brief Returns true if endpoint contains the ZCL client with specified id. + * + * This function returns true if + * the endpoint contains client of a given cluster. + */ +bool emberAfContainsClient(chip::EndpointId endpoint, chip::ClusterId clusterId); + /** * @brief write an attribute, performing all the checks. * diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index e30b990546b62d..db55e2bd6e1d7e 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -675,6 +675,18 @@ bool emberAfContainsServer(EndpointId endpoint, ClusterId clusterId) return (emberAfFindServerCluster(endpoint, clusterId) != nullptr); } +// Returns whether the given endpoint has the client of the given cluster on it. +bool emberAfContainsClient(EndpointId endpoint, ClusterId clusterId) +{ + uint16_t ep = emberAfIndexFromEndpoint(endpoint); + if (ep == kEmberInvalidEndpointIndex) + { + return false; + } + + return (emberAfFindClusterInType(emAfEndpoints[ep].endpointType, clusterId, CLUSTER_MASK_CLIENT) != nullptr); +} + // This will find the first server that has the clusterId given from the index of endpoint. bool emberAfContainsServerFromIndex(uint16_t index, ClusterId clusterId) { From 43bd36c8f82694473fcd96ab9f22eeae65287dea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Wed, 11 Jan 2023 10:55:50 +0100 Subject: [PATCH 11/14] Enabled onOff client on all-clusters-app to fix failing tests --- .../all-clusters-app.matter | 48 +++++++++++++++++++ .../all-clusters-common/all-clusters-app.zap | 2 +- .../zap-generated/CHIPClusters.h | 9 ++++ .../zap-generated/endpoint_config.h | 17 +++++-- .../zap-generated/gen_config.h | 5 ++ 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter index 808d781ce4ca27..5c07da8cf4586b 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter @@ -215,6 +215,53 @@ server cluster Scenes = 5 { fabric command GetSceneMembership(GetSceneMembershipRequest): GetSceneMembershipResponse = 6; } +client cluster OnOff = 6 { + enum OnOffDelayedAllOffEffectVariant : ENUM8 { + kFadeToOffIn0p8Seconds = 0; + kNoFade = 1; + k50PercentDimDownIn0p8SecondsThenFadeToOffIn12Seconds = 2; + } + + enum OnOffDyingLightEffectVariant : ENUM8 { + k20PercenterDimUpIn0p5SecondsThenFadeToOffIn1Second = 0; + } + + enum OnOffEffectIdentifier : ENUM8 { + kDelayedAllOff = 0; + kDyingLight = 1; + } + + enum OnOffStartUpOnOff : ENUM8 { + kOff = 0; + kOn = 1; + kTogglePreviousOnOff = 2; + } + + bitmap OnOffControl : BITMAP8 { + kAcceptOnlyWhenOn = 0x1; + } + + bitmap OnOffFeature : BITMAP32 { + kLighting = 0x1; + } + + bitmap SceneFeatures : BITMAP32 { + kSceneNames = 0x1; + } + + readonly attribute boolean onOff = 0; + readonly attribute boolean globalSceneControl = 16384; + attribute int16u onTime = 16385; + attribute int16u offWaitTime = 16386; + attribute access(write: manage) nullable OnOffStartUpOnOff startUpOnOff = 16387; + readonly attribute bitmap32 featureMap = 65532; + readonly attribute int16u clusterRevision = 65533; + + command Off(): DefaultSuccess = 0; + command On(): DefaultSuccess = 1; + command Toggle(): DefaultSuccess = 2; +} + server cluster OnOff = 6 { enum OnOffDelayedAllOffEffectVariant : ENUM8 { kFadeToOffIn0p8Seconds = 0; @@ -4266,6 +4313,7 @@ endpoint 0 { } endpoint 1 { device type onofflight = 256, version 1; + binding cluster OnOff; server cluster Identify { ram attribute identifyTime; diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap index 8efd7b77f1b467..c64b29531e0781 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap @@ -9366,7 +9366,7 @@ "mfgCode": null, "define": "ON_OFF_CLUSTER", "side": "client", - "enabled": 0, + "enabled": 1, "commands": [ { "name": "Off", diff --git a/zzz_generated/all-clusters-app/zap-generated/CHIPClusters.h b/zzz_generated/all-clusters-app/zap-generated/CHIPClusters.h index fffb265ae9c797..c46edbc34f8e93 100644 --- a/zzz_generated/all-clusters-app/zap-generated/CHIPClusters.h +++ b/zzz_generated/all-clusters-app/zap-generated/CHIPClusters.h @@ -30,6 +30,15 @@ namespace chip { namespace Controller { +class DLL_EXPORT OnOffCluster : public ClusterBase +{ +public: + OnOffCluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) : + ClusterBase(exchangeManager, session, app::Clusters::OnOff::Id, endpoint) + {} + ~OnOffCluster() {} +}; + class DLL_EXPORT OtaSoftwareUpdateProviderCluster : public ClusterBase { public: diff --git a/zzz_generated/all-clusters-app/zap-generated/endpoint_config.h b/zzz_generated/all-clusters-app/zap-generated/endpoint_config.h index 155a8034770c73..b9b88be100bd90 100644 --- a/zzz_generated/all-clusters-app/zap-generated/endpoint_config.h +++ b/zzz_generated/all-clusters-app/zap-generated/endpoint_config.h @@ -1904,7 +1904,7 @@ // clang-format on #define ZAP_CLUSTER_MASK(mask) CLUSTER_MASK_##mask -#define GENERATED_CLUSTER_COUNT 79 +#define GENERATED_CLUSTER_COUNT 80 // clang-format off #define GENERATED_CLUSTERS { \ @@ -2260,6 +2260,17 @@ .acceptedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 97 ) ,\ .generatedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 105 ) ,\ },\ + { \ + /* Endpoint: 1, Cluster: On/Off (client) */ \ + .clusterId = 0x00000006, \ + .attributes = ZAP_ATTRIBUTE_INDEX(253), \ + .attributeCount = 0, \ + .clusterSize = 0, \ + .mask = ZAP_CLUSTER_MASK(CLIENT), \ + .functions = NULL, \ + .acceptedCommandList = nullptr ,\ + .generatedCommandList = nullptr ,\ + },\ { \ /* Endpoint: 1, Cluster: On/Off (server) */ \ .clusterId = 0x00000006, \ @@ -2788,8 +2799,8 @@ // This is an array of EmberAfEndpointType structures. #define GENERATED_ENDPOINT_TYPES \ { \ - { ZAP_CLUSTER_INDEX(0), 29, 377 }, { ZAP_CLUSTER_INDEX(29), 44, 3459 }, { ZAP_CLUSTER_INDEX(73), 5, 105 }, \ - { ZAP_CLUSTER_INDEX(78), 1, 0 }, \ + { ZAP_CLUSTER_INDEX(0), 29, 377 }, { ZAP_CLUSTER_INDEX(29), 45, 3459 }, { ZAP_CLUSTER_INDEX(74), 5, 105 }, \ + { ZAP_CLUSTER_INDEX(79), 1, 0 }, \ } // Largest attribute size is needed for various buffers diff --git a/zzz_generated/all-clusters-app/zap-generated/gen_config.h b/zzz_generated/all-clusters-app/zap-generated/gen_config.h index b4d285a75344ae..cf31e2a2c02a91 100644 --- a/zzz_generated/all-clusters-app/zap-generated/gen_config.h +++ b/zzz_generated/all-clusters-app/zap-generated/gen_config.h @@ -32,6 +32,7 @@ #define EMBER_AF_IDENTIFY_CLUSTER_SERVER_ENDPOINT_COUNT (2) #define EMBER_AF_GROUPS_CLUSTER_SERVER_ENDPOINT_COUNT (3) #define EMBER_AF_SCENES_CLUSTER_SERVER_ENDPOINT_COUNT (1) +#define EMBER_AF_ON_OFF_CLUSTER_CLIENT_ENDPOINT_COUNT (1) #define EMBER_AF_ON_OFF_CLUSTER_SERVER_ENDPOINT_COUNT (2) #define EMBER_AF_ON_OFF_SWITCH_CONFIGURATION_CLUSTER_SERVER_ENDPOINT_COUNT (1) #define EMBER_AF_LEVEL_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT (1) @@ -124,6 +125,10 @@ #define MATTER_CLUSTER_SCENE_NAME_SUPPORT_MASK 0x0001 #define MATTER_CLUSTER_SCENE_NAME_SUPPORT (0x0000 & MATTER_CLUSTER_SCENE_NAME_SUPPORT_MASK) +// Use this macro to check if the client side of the On/Off cluster is included +#define ZCL_USING_ON_OFF_CLUSTER_CLIENT +#define EMBER_AF_PLUGIN_ON_OFF_CLIENT + // Use this macro to check if the server side of the On/Off cluster is included #define ZCL_USING_ON_OFF_CLUSTER_SERVER #define EMBER_AF_PLUGIN_ON_OFF_SERVER From b48c05308029529aca40ff377570886da308bfb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Thu, 12 Jan 2023 09:00:33 +0100 Subject: [PATCH 12/14] Fixed spelling error and added notify if not list operation --- src/app/clusters/bindings/BindingManager.h | 2 +- src/app/clusters/bindings/bindings.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index 2512bcb5443e73..8822630ab0f87a 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -62,7 +62,7 @@ struct BindingManagerInitParams * when a binding is ready to be communicated with. * * A CASE connection will be triggered when: - * - During init of the BindingManager, unless the application activly disables this using mEstablishConnectionOnInit + * - During init of the BindingManager, unless the application actively disables this using mEstablishConnectionOnInit * - The binding cluster adds a unicast entry to the binding table. * - A watched cluster changes with a unicast binding but we cannot find an active connection to the peer. * diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index aab0af8bfa7f88..e1eed51abf760d 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -227,6 +227,14 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath { CreateBindingEntry(iter.GetValue(), path.mEndpointId); } + + // If this was not caused by a list operation, OnListWriteEnd is not going to be truggered + // so a notification is sent here. + if (!path.IsListOperation()) + { + // Notify binding table has changed + LogErrorOnFailure(NotifyBindingsChanged()); + } return CHIP_NO_ERROR; } if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem) From 529d358c78b55695491c89a5aa62082822a452ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= <69624991+ReneJosefsen@users.noreply.github.com> Date: Thu, 12 Jan 2023 17:50:26 +0100 Subject: [PATCH 13/14] Update src/app/clusters/bindings/bindings.cpp Co-authored-by: Boris Zbarsky --- src/app/clusters/bindings/bindings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index e1eed51abf760d..65d2c90a8d65c7 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -228,7 +228,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath CreateBindingEntry(iter.GetValue(), path.mEndpointId); } - // If this was not caused by a list operation, OnListWriteEnd is not going to be truggered + // If this was not caused by a list operation, OnListWriteEnd is not going to be triggered // so a notification is sent here. if (!path.IsListOperation()) { From 35abfa9ff1bbcd1fc0bf787fdb4957e9bc2569d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Josefsen?= Date: Wed, 18 Jan 2023 14:37:05 +0100 Subject: [PATCH 14/14] Try to fix CI failure --- .../all-clusters-app/app-templates/CHIPClusters.h | 9 --------- .../all-clusters-app/app-templates/endpoint_config.h | 11 ----------- .../all-clusters-app/app-templates/gen_config.h | 5 ----- 3 files changed, 25 deletions(-) diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CHIPClusters.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CHIPClusters.h index c46edbc34f8e93..fffb265ae9c797 100644 --- a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CHIPClusters.h +++ b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CHIPClusters.h @@ -30,15 +30,6 @@ namespace chip { namespace Controller { -class DLL_EXPORT OnOffCluster : public ClusterBase -{ -public: - OnOffCluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) : - ClusterBase(exchangeManager, session, app::Clusters::OnOff::Id, endpoint) - {} - ~OnOffCluster() {} -}; - class DLL_EXPORT OtaSoftwareUpdateProviderCluster : public ClusterBase { public: diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h index 65709e0e05b1d4..c7b4d3954fa04b 100644 --- a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h +++ b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h @@ -2237,17 +2237,6 @@ .acceptedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 94 ) ,\ .generatedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 102 ) ,\ },\ - { \ - /* Endpoint: 1, Cluster: On/Off (client) */ \ - .clusterId = 0x00000006, \ - .attributes = ZAP_ATTRIBUTE_INDEX(253), \ - .attributeCount = 0, \ - .clusterSize = 0, \ - .mask = ZAP_CLUSTER_MASK(CLIENT), \ - .functions = NULL, \ - .acceptedCommandList = nullptr ,\ - .generatedCommandList = nullptr ,\ - },\ { \ /* Endpoint: 1, Cluster: On/Off (server) */ \ .clusterId = 0x00000006, \ diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/gen_config.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/gen_config.h index 574b29b143c9b8..b608d4fe5dbf93 100644 --- a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/gen_config.h +++ b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/gen_config.h @@ -32,7 +32,6 @@ #define EMBER_AF_IDENTIFY_CLUSTER_SERVER_ENDPOINT_COUNT (2) #define EMBER_AF_GROUPS_CLUSTER_SERVER_ENDPOINT_COUNT (3) #define EMBER_AF_SCENES_CLUSTER_SERVER_ENDPOINT_COUNT (1) -#define EMBER_AF_ON_OFF_CLUSTER_CLIENT_ENDPOINT_COUNT (1) #define EMBER_AF_ON_OFF_CLUSTER_SERVER_ENDPOINT_COUNT (2) #define EMBER_AF_ON_OFF_SWITCH_CONFIGURATION_CLUSTER_SERVER_ENDPOINT_COUNT (1) #define EMBER_AF_LEVEL_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT (1) @@ -124,10 +123,6 @@ #define MATTER_CLUSTER_SCENE_NAME_SUPPORT_MASK 0x0001 #define MATTER_CLUSTER_SCENE_NAME_SUPPORT (0x0000 & MATTER_CLUSTER_SCENE_NAME_SUPPORT_MASK) -// Use this macro to check if the client side of the On/Off cluster is included -#define ZCL_USING_ON_OFF_CLUSTER_CLIENT -#define EMBER_AF_PLUGIN_ON_OFF_CLIENT - // Use this macro to check if the server side of the On/Off cluster is included #define ZCL_USING_ON_OFF_CLUSTER_SERVER #define EMBER_AF_PLUGIN_ON_OFF_SERVER