From f40e0bf9cc086d789aaeff780602befbea483987 Mon Sep 17 00:00:00 2001 From: Junior Martinez Date: Tue, 25 Apr 2023 14:05:50 -0400 Subject: [PATCH 1/8] Increase the size of the onoff cluster's eventControl array to handle the max dynamic enpoints count --- src/app/clusters/on-off-server/on-off-server.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/on-off-server/on-off-server.cpp b/src/app/clusters/on-off-server/on-off-server.cpp index 1f62a3261635d7..d234e4b8dd00b2 100644 --- a/src/app/clusters/on-off-server/on-off-server.cpp +++ b/src/app/clusters/on-off-server/on-off-server.cpp @@ -48,7 +48,9 @@ using chip::Protocols::InteractionModel::Status; static OnOffEffect * firstEffect = nullptr; OnOffServer OnOffServer::instance; -static EmberEventControl gEventControls[EMBER_AF_ON_OFF_CLUSTER_SERVER_ENDPOINT_COUNT]; +static constexpr size_t kOnOffMaxEnpointCount = + EMBER_AF_ON_OFF_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; +static EmberEventControl gEventControls[kOnOffMaxEnpointCount]; /********************************************************** * Function definition From 922dc607369424ce095aafb9aac23f2d4f5fbf61 Mon Sep 17 00:00:00 2001 From: Junior Martinez Date: Thu, 11 May 2023 17:50:35 -0400 Subject: [PATCH 2/8] Create a new function to get the real index of the endpoint, that contains a specific cluster server, as stored in emAfEndpoints. Use it in Onoff cluster server to map the endpoint to the EventControl array of the server --- .../clusters/on-off-server/on-off-server.cpp | 2 +- src/app/util/af.h | 36 +++++++++++++++++++ src/app/util/attribute-storage.cpp | 17 +++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/on-off-server/on-off-server.cpp b/src/app/clusters/on-off-server/on-off-server.cpp index d234e4b8dd00b2..fb945ddd1475ba 100644 --- a/src/app/clusters/on-off-server/on-off-server.cpp +++ b/src/app/clusters/on-off-server/on-off-server.cpp @@ -654,7 +654,7 @@ bool OnOffServer::areStartUpOnOffServerAttributesNonVolatile(EndpointId endpoint */ EmberEventControl * OnOffServer::getEventControl(EndpointId endpoint) { - uint16_t index = emberAfFindClusterServerEndpointIndex(endpoint, OnOff::Id); + uint16_t index = emberAfGetClusterServerEndpointIndex(endpoint, OnOff::Id); if (index >= ArraySize(gEventControls)) { return nullptr; diff --git a/src/app/util/af.h b/src/app/util/af.h index 2acece42a97de9..7c98aa4cb4d412 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -184,6 +184,42 @@ uint16_t emberAfIndexFromEndpointIncludingDisabledEndpoints(chip::EndpointId end */ uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId clusterId); +/** + * Returns the index of the given endpoint, in the list of all defined endpoints, + * that support the given cluster. + * + * Returns kEmberInvalidEndpointIndex if the given endpoint does not support the + * given cluster or if the given endpoint is disabled. + * + * Contrary to emberAfFindClusterServerEndpointIndex, this function always return the same index + * for a given endpointId, static or dynamic. + * + * This index reflects where the data of the endpoint is located in + * emAfEndpoints + * + * For example, if a device has 4 fixed endpoints (ids 0-3) and 2 dynamic + * endpoints, and cluster X is supported on endpoints 1 and 3, then: + * + * 1) emberAfGetClusterServerEndpointIndex(0, X) returns kEmberInvalidEndpointIndex + * 2) emberAfGetClusterServerEndpointIndex(1, X) returns 1 + * 3) emberAfGetClusterServerEndpointIndex(2, X) returns kEmberInvalidEndpointIndex + * 4) emberAfGetClusterServerEndpointIndex(3, X) returns 3 + + * Note for Dynamic endpoints the index will always be >= to FIXED_ENDPOINT_COUNT + * + * If a dynamic endpoint is defined to dynamic index 1 with endpoint id 7, + * and supports cluster X, (via emberAfSetDynamicEndpoint(1, 7, ...)) + * then emberAfGetClusterServerEndpointIndex(7, X) returns 5 (DynamicEndpointIndex 1 + FIXED_ENDPOINT_COUNT). + * + * If now a second dynamic endpoint is defined to dynamic index 0 + * with endpoint id 9, and also supports cluster X (via emberAfSetDynamicEndpoint(0, 9, ...)), + * + * emberAfGetClusterServerEndpointIndex(9, X) returns 4. + * and emberAfGetClusterServerEndpointIndex(7, X) still returns 5 + * + */ +uint16_t emberAfGetClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId cluster); + /** * @brief Returns the total number of endpoints (dynamic and pre-compiled). */ diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 322c22cd549e67..e85be1dce12631 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -878,6 +878,23 @@ static uint16_t findIndexFromEndpoint(EndpointId endpoint, bool ignoreDisabledEn return kEmberInvalidEndpointIndex; } +uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId cluster) +{ + uint16_t epIndex = findIndexFromEndpoint(endpoint, true /*ignoreDisabledEndpoints*/); + + // Endpoint must be configured and enabled + if (epIndex != kEmberInvalidEndpointIndex) + { + if (emberAfFindClusterInType(emAfEndpoints[epIndex].endpointType, cluster, CLUSTER_MASK_SERVER) == nullptr) + { + // The provided endpoint do not contain the given cluster server. + return kEmberInvalidEndpointIndex; + } + } + + return epIndex; +} + bool emberAfEndpointIsEnabled(EndpointId endpoint) { uint16_t index = findIndexFromEndpoint(endpoint, From a4f5f2efb7e35d794f4fef46219967e3bf8218d4 Mon Sep 17 00:00:00 2001 From: Junior Martinez Date: Fri, 12 May 2023 12:53:37 -0400 Subject: [PATCH 3/8] Fix emberAfGetClusterServerEndpointIndex so the returned index is adjusted to the cluster server implementation --- .../clusters/on-off-server/on-off-server.cpp | 2 +- src/app/util/af.h | 42 ++++++++++--------- src/app/util/attribute-storage.cpp | 30 ++++++++++++- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/app/clusters/on-off-server/on-off-server.cpp b/src/app/clusters/on-off-server/on-off-server.cpp index fb945ddd1475ba..c79b9d592cdca8 100644 --- a/src/app/clusters/on-off-server/on-off-server.cpp +++ b/src/app/clusters/on-off-server/on-off-server.cpp @@ -654,7 +654,7 @@ bool OnOffServer::areStartUpOnOffServerAttributesNonVolatile(EndpointId endpoint */ EmberEventControl * OnOffServer::getEventControl(EndpointId endpoint) { - uint16_t index = emberAfGetClusterServerEndpointIndex(endpoint, OnOff::Id); + uint16_t index = emberAfGetClusterServerEndpointIndex(endpoint, OnOff::Id, EMBER_AF_ON_OFF_CLUSTER_SERVER_ENDPOINT_COUNT); if (index >= ArraySize(gEventControls)) { return nullptr; diff --git a/src/app/util/af.h b/src/app/util/af.h index 7c98aa4cb4d412..fc48db6b3b6c62 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -185,40 +185,44 @@ uint16_t emberAfIndexFromEndpointIncludingDisabledEndpoints(chip::EndpointId end uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId clusterId); /** - * Returns the index of the given endpoint, in the list of all defined endpoints, - * that support the given cluster. + * @brief Returns an index of the given endpoint the given cluster server * * Returns kEmberInvalidEndpointIndex if the given endpoint does not support the * given cluster or if the given endpoint is disabled. * - * Contrary to emberAfFindClusterServerEndpointIndex, this function always return the same index - * for a given endpointId, static or dynamic. + * Unlike emberAfFindClusterServerEndpointIndex, this function always returns the same index + * for a given endpointId instance, static or dynamic. * - * This index reflects where the data of the endpoint is located in - * emAfEndpoints + * The return index is determined by the location of the endpoint in + * emAfEndpoints database but is adjusted to the given Cluster server implementation scope * * For example, if a device has 4 fixed endpoints (ids 0-3) and 2 dynamic - * endpoints, and cluster X is supported on endpoints 1 and 3, then: + * endpoints, and cluster X is supported on endpoints 1 and 3, then + * fixedClusterServerEndpointCount should be 2 and * * 1) emberAfGetClusterServerEndpointIndex(0, X) returns kEmberInvalidEndpointIndex - * 2) emberAfGetClusterServerEndpointIndex(1, X) returns 1 + * 2) emberAfGetClusterServerEndpointIndex(1, X) returns 0 * 3) emberAfGetClusterServerEndpointIndex(2, X) returns kEmberInvalidEndpointIndex - * 4) emberAfGetClusterServerEndpointIndex(3, X) returns 3 + * 4) emberAfGetClusterServerEndpointIndex(3, X) returns 1 - * Note for Dynamic endpoints the index will always be >= to FIXED_ENDPOINT_COUNT - * - * If a dynamic endpoint is defined to dynamic index 1 with endpoint id 7, - * and supports cluster X, (via emberAfSetDynamicEndpoint(1, 7, ...)) - * then emberAfGetClusterServerEndpointIndex(7, X) returns 5 (DynamicEndpointIndex 1 + FIXED_ENDPOINT_COUNT). + * The Dynamic endpoints are placed after the fixed ones + * therefore their return index will always be >= to fixedClusterServerEndpointCount * - * If now a second dynamic endpoint is defined to dynamic index 0 - * with endpoint id 9, and also supports cluster X (via emberAfSetDynamicEndpoint(0, 9, ...)), + * If a dynamic endpoint, supporting cluster X, is defined to dynamic index 1 with endpoint id 7, + * (via emberAfSetDynamicEndpoint(1, 7, ...)) + * then emberAfGetClusterServerEndpointIndex(7, X) returns 3 (fixedClusterServerEndpointCount{2} + DynamicEndpointIndex {1}). * - * emberAfGetClusterServerEndpointIndex(9, X) returns 4. - * and emberAfGetClusterServerEndpointIndex(7, X) still returns 5 + * If now a second dynamic endpoint, also supporting cluster X, is defined to dynamic index 0 + * with endpoint id 9 (via emberAfSetDynamicEndpoint(0, 9, ...)), + * emberAfGetClusterServerEndpointIndex(9, X) returns 2. (fixedClusterServerEndpointCount{2} + DynamicEndpointIndex {0}). + * and emberAfGetClusterServerEndpointIndex(7, X) still returns 3 * + * @param endpoint: Endpoint number + * @param cluster: Id the of the Cluster server you are interrested on + * @param fixedClusterServerEndpointCount: The amount of Fixed endpoints containing this cluster server */ -uint16_t emberAfGetClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId cluster); +uint16_t emberAfGetClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId cluster, + uint16_t fixedClusterServerEndpointCount); /** * @brief Returns the total number of endpoints (dynamic and pre-compiled). diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index e85be1dce12631..7ed3da3e90e3f3 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -878,7 +878,7 @@ static uint16_t findIndexFromEndpoint(EndpointId endpoint, bool ignoreDisabledEn return kEmberInvalidEndpointIndex; } -uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId cluster) +uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId cluster, uint16_t fixedClusterServerEndpointCount) { uint16_t epIndex = findIndexFromEndpoint(endpoint, true /*ignoreDisabledEndpoints*/); @@ -890,6 +890,34 @@ uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId clu // The provided endpoint do not contain the given cluster server. return kEmberInvalidEndpointIndex; } + + if (epIndex < FIXED_ENDPOINT_COUNT) + { + // This endpoint is a fixed one. + // ajust the return index to be in the scope of the ClusterServer[0 to fixedClusterServerEndpointCount] + // in chronological order of appearance of the cluster server in the endpoint database + uint16_t ajustedEndpointIndex = 0; + for (uint16_t i = 0; i < epIndex; i++) + { + // increase ajustedEndpointIndex for every endpoint containing the cluster server + // before our endpoint of interest + if (emAfEndpoints[i].endpoint != kInvalidEndpointId && + (emberAfFindClusterInType(emAfEndpoints[i].endpointType, cluster, CLUSTER_MASK_SERVER) != nullptr)) + { + ajustedEndpointIndex++; + } + } + + // If this asserts, The provided fixedClusterServerEndpointCount doesn't match the app data model" + VerifyOrDie(ajustedEndpointIndex < fixedClusterServerEndpointCount); + epIndex = ajustedEndpointIndex; + } + else + { + // This is a Dynamic endpoint + // ajust index after the fixedClusterServerEndpointCount index + epIndex = static_cast(fixedClusterServerEndpointCount + (epIndex - FIXED_ENDPOINT_COUNT)); + } } return epIndex; From 691dac0abc867c2273a82b4513989bb3a2c9931a Mon Sep 17 00:00:00 2001 From: Junior Martinez Date: Mon, 15 May 2023 11:06:24 -0400 Subject: [PATCH 4/8] Update function brief as macOS Build complains about the : after the param name [-Werror,-Wdocumentation --- src/app/util/af.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/util/af.h b/src/app/util/af.h index fc48db6b3b6c62..2644eb4364a2e7 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -217,9 +217,9 @@ uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip:: * emberAfGetClusterServerEndpointIndex(9, X) returns 2. (fixedClusterServerEndpointCount{2} + DynamicEndpointIndex {0}). * and emberAfGetClusterServerEndpointIndex(7, X) still returns 3 * - * @param endpoint: Endpoint number - * @param cluster: Id the of the Cluster server you are interrested on - * @param fixedClusterServerEndpointCount: The amount of Fixed endpoints containing this cluster server + * @param endpoint Endpoint number + * @param cluster Id the of the Cluster server you are interrested on + * @param fixedClusterServerEndpointCount The amount of Fixed endpoints containing this cluster server */ uint16_t emberAfGetClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId cluster, uint16_t fixedClusterServerEndpointCount); From f7a67341e25341552ead5bc672b331ccee207d9d Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Mon, 15 May 2023 15:06:42 -0400 Subject: [PATCH 5/8] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/app/util/af.h | 13 +++++++------ src/app/util/attribute-storage.cpp | 6 +++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/app/util/af.h b/src/app/util/af.h index 2644eb4364a2e7..f78ca77e399efc 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -185,16 +185,17 @@ uint16_t emberAfIndexFromEndpointIncludingDisabledEndpoints(chip::EndpointId end uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId clusterId); /** - * @brief Returns an index of the given endpoint the given cluster server + * @brief Returns the index of the given endpoint in the list of all endpoints that might support the given cluster server. * * Returns kEmberInvalidEndpointIndex if the given endpoint does not support the * given cluster or if the given endpoint is disabled. * * Unlike emberAfFindClusterServerEndpointIndex, this function always returns the same index - * for a given endpointId instance, static or dynamic. + * for a given endpointId instance, fixed or dynamic, if it does not return kEmberInvalidEndpointIndex. * - * The return index is determined by the location of the endpoint in - * emAfEndpoints database but is adjusted to the given Cluster server implementation scope + * The return index is identical to emberAfFindClusterServerEndpointIndex for fixed endpoints, + * but for dynamic endpoints the indexing assumes that any dynamic endpoint could start supporting + * the given server cluster. * * For example, if a device has 4 fixed endpoints (ids 0-3) and 2 dynamic * endpoints, and cluster X is supported on endpoints 1 and 3, then @@ -205,7 +206,7 @@ uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip:: * 3) emberAfGetClusterServerEndpointIndex(2, X) returns kEmberInvalidEndpointIndex * 4) emberAfGetClusterServerEndpointIndex(3, X) returns 1 - * The Dynamic endpoints are placed after the fixed ones + * The Dynamic endpoints are placed after the fixed ones; * therefore their return index will always be >= to fixedClusterServerEndpointCount * * If a dynamic endpoint, supporting cluster X, is defined to dynamic index 1 with endpoint id 7, @@ -219,7 +220,7 @@ uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip:: * * @param endpoint Endpoint number * @param cluster Id the of the Cluster server you are interrested on - * @param fixedClusterServerEndpointCount The amount of Fixed endpoints containing this cluster server + * @param fixedClusterServerEndpointCount The number of fixed endpoints containing this cluster server. Typically one of the EMBER_AF_*_CLUSTER_SERVER_ENDPOINT_COUNT constants. */ uint16_t emberAfGetClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId cluster, uint16_t fixedClusterServerEndpointCount); diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 7ed3da3e90e3f3..8aff96019c404e 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -887,7 +887,7 @@ uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId clu { if (emberAfFindClusterInType(emAfEndpoints[epIndex].endpointType, cluster, CLUSTER_MASK_SERVER) == nullptr) { - // The provided endpoint do not contain the given cluster server. + // The provided endpoint does not contain the given cluster server. return kEmberInvalidEndpointIndex; } @@ -914,8 +914,8 @@ uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId clu } else { - // This is a Dynamic endpoint - // ajust index after the fixedClusterServerEndpointCount index + // This is a dynamic endpoint. + // It's index is just its index in the dynamic endpoint list, offset by fixedClusterServerEndpointCount. epIndex = static_cast(fixedClusterServerEndpointCount + (epIndex - FIXED_ENDPOINT_COUNT)); } } From 3d3e1248b47ef5b25a44cba9c1daf5fd814814fa Mon Sep 17 00:00:00 2001 From: Junior Martinez Date: Mon, 15 May 2023 15:29:04 -0400 Subject: [PATCH 6/8] Add a VerifyorDie, Invert condition check for early return and outdent rest of the code --- src/app/util/attribute-storage.cpp | 61 ++++++++++++++++-------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 8aff96019c404e..03754bdc419990 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -880,44 +880,47 @@ static uint16_t findIndexFromEndpoint(EndpointId endpoint, bool ignoreDisabledEn uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId cluster, uint16_t fixedClusterServerEndpointCount) { + VerifyOrDie(fixedClusterServerEndpointCount <= FIXED_ENDPOINT_COUNT); uint16_t epIndex = findIndexFromEndpoint(endpoint, true /*ignoreDisabledEndpoints*/); // Endpoint must be configured and enabled - if (epIndex != kEmberInvalidEndpointIndex) + if (epIndex == kEmberInvalidEndpointIndex) { - if (emberAfFindClusterInType(emAfEndpoints[epIndex].endpointType, cluster, CLUSTER_MASK_SERVER) == nullptr) - { - // The provided endpoint does not contain the given cluster server. - return kEmberInvalidEndpointIndex; - } + return kEmberInvalidEndpointIndex; + } + + if (emberAfFindClusterInType(emAfEndpoints[epIndex].endpointType, cluster, CLUSTER_MASK_SERVER) == nullptr) + { + // The provided endpoint does not contain the given cluster server. + return kEmberInvalidEndpointIndex; + } - if (epIndex < FIXED_ENDPOINT_COUNT) + if (epIndex < FIXED_ENDPOINT_COUNT) + { + // This endpoint is a fixed one. + // ajust the return index to be in the scope of the ClusterServer[0 to fixedClusterServerEndpointCount] + // in chronological order of appearance of the cluster server in the endpoint database + uint16_t ajustedEndpointIndex = 0; + for (uint16_t i = 0; i < epIndex; i++) { - // This endpoint is a fixed one. - // ajust the return index to be in the scope of the ClusterServer[0 to fixedClusterServerEndpointCount] - // in chronological order of appearance of the cluster server in the endpoint database - uint16_t ajustedEndpointIndex = 0; - for (uint16_t i = 0; i < epIndex; i++) + // increase ajustedEndpointIndex for every endpoint containing the cluster server + // before our endpoint of interest + if (emAfEndpoints[i].endpoint != kInvalidEndpointId && + (emberAfFindClusterInType(emAfEndpoints[i].endpointType, cluster, CLUSTER_MASK_SERVER) != nullptr)) { - // increase ajustedEndpointIndex for every endpoint containing the cluster server - // before our endpoint of interest - if (emAfEndpoints[i].endpoint != kInvalidEndpointId && - (emberAfFindClusterInType(emAfEndpoints[i].endpointType, cluster, CLUSTER_MASK_SERVER) != nullptr)) - { - ajustedEndpointIndex++; - } + ajustedEndpointIndex++; } - - // If this asserts, The provided fixedClusterServerEndpointCount doesn't match the app data model" - VerifyOrDie(ajustedEndpointIndex < fixedClusterServerEndpointCount); - epIndex = ajustedEndpointIndex; - } - else - { - // This is a dynamic endpoint. - // It's index is just its index in the dynamic endpoint list, offset by fixedClusterServerEndpointCount. - epIndex = static_cast(fixedClusterServerEndpointCount + (epIndex - FIXED_ENDPOINT_COUNT)); } + + // If this asserts, The provided fixedClusterServerEndpointCount doesn't match the app data model" + VerifyOrDie(ajustedEndpointIndex < fixedClusterServerEndpointCount); + epIndex = ajustedEndpointIndex; + } + else + { + // This is a dynamic endpoint. + // It's index is just its index in the dynamic endpoint list, offset by fixedClusterServerEndpointCount. + epIndex = static_cast(fixedClusterServerEndpointCount + (epIndex - FIXED_ENDPOINT_COUNT)); } return epIndex; From bada0a13ef35555691de5ca951cbf69640b20783 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 15 May 2023 19:29:27 +0000 Subject: [PATCH 7/8] Restyled by clang-format --- src/app/util/af.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/util/af.h b/src/app/util/af.h index f78ca77e399efc..f118006e317a65 100644 --- a/src/app/util/af.h +++ b/src/app/util/af.h @@ -220,7 +220,8 @@ uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip:: * * @param endpoint Endpoint number * @param cluster Id the of the Cluster server you are interrested on - * @param fixedClusterServerEndpointCount The number of fixed endpoints containing this cluster server. Typically one of the EMBER_AF_*_CLUSTER_SERVER_ENDPOINT_COUNT constants. + * @param fixedClusterServerEndpointCount The number of fixed endpoints containing this cluster server. Typically one of the + EMBER_AF_*_CLUSTER_SERVER_ENDPOINT_COUNT constants. */ uint16_t emberAfGetClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId cluster, uint16_t fixedClusterServerEndpointCount); From 63d7edd2a6b09fc4a909bc6593a1a8f5245a152a Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Tue, 16 May 2023 11:48:15 -0400 Subject: [PATCH 8/8] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/app/util/attribute-storage.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 03754bdc419990..bbef9d492e807b 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -898,28 +898,27 @@ uint16_t emberAfGetClusterServerEndpointIndex(EndpointId endpoint, ClusterId clu if (epIndex < FIXED_ENDPOINT_COUNT) { // This endpoint is a fixed one. - // ajust the return index to be in the scope of the ClusterServer[0 to fixedClusterServerEndpointCount] - // in chronological order of appearance of the cluster server in the endpoint database - uint16_t ajustedEndpointIndex = 0; + // Return the index of this endpoint in the list of fixed endpoints that support the given cluster. + uint16_t adjustedEndpointIndex = 0; for (uint16_t i = 0; i < epIndex; i++) { - // increase ajustedEndpointIndex for every endpoint containing the cluster server + // Increase adjustedEndpointIndex for every endpoint containing the cluster server // before our endpoint of interest if (emAfEndpoints[i].endpoint != kInvalidEndpointId && (emberAfFindClusterInType(emAfEndpoints[i].endpointType, cluster, CLUSTER_MASK_SERVER) != nullptr)) { - ajustedEndpointIndex++; + adjustedEndpointIndex++; } } - // If this asserts, The provided fixedClusterServerEndpointCount doesn't match the app data model" - VerifyOrDie(ajustedEndpointIndex < fixedClusterServerEndpointCount); - epIndex = ajustedEndpointIndex; + // If this asserts, the provided fixedClusterServerEndpointCount doesn't match the app data model. + VerifyOrDie(adjustedEndpointIndex < fixedClusterServerEndpointCount); + epIndex = adjustedEndpointIndex; } else { // This is a dynamic endpoint. - // It's index is just its index in the dynamic endpoint list, offset by fixedClusterServerEndpointCount. + // Its index is just its index in the dynamic endpoint list, offset by fixedClusterServerEndpointCount. epIndex = static_cast(fixedClusterServerEndpointCount + (epIndex - FIXED_ENDPOINT_COUNT)); }