[ON-OFF] Increase the size of the onoff cluster's eventControl array …

…to handl… (#26254) * Increase the size of the onoff cluster's eventControl array to handle the max dynamic enpoints count * 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 * Fix emberAfGetClusterServerEndpointIndex so the returned index is adjusted to the cluster server implementation * Update function brief as macOS Build complains about the : after the param name [-Werror,-Wdocumentation * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> * Add a VerifyorDie, Invert condition check for early return and outdent rest of the code * Restyled by clang-format * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> --------- Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: Restyled.io <[email protected]>
project-chip · Oct 24, 2023 · e14cc17 · e14cc17
1 parent df37b4f
commit e14cc17
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 2 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
@@ -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
@@ -652,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, 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
@@ -184,6 +184,48 @@ uint16_t emberAfIndexFromEndpointIncludingDisabledEndpoints(chip::EndpointId end
  */
 uint16_t emberAfFindClusterServerEndpointIndex(chip::EndpointId endpoint, chip::ClusterId clusterId);
 
+/**
+ *  @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, fixed or dynamic, if it does not return kEmberInvalidEndpointIndex.
+ *
+ * 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
+ * fixedClusterServerEndpointCount should be 2 and
+ *
+ * 1) emberAfGetClusterServerEndpointIndex(0, X) returns kEmberInvalidEndpointIndex
+ * 2) emberAfGetClusterServerEndpointIndex(1, X) returns 0
+ * 3) emberAfGetClusterServerEndpointIndex(2, X) returns kEmberInvalidEndpointIndex
+ * 4) emberAfGetClusterServerEndpointIndex(3, X) returns 1
+
+ * 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,
+ * (via emberAfSetDynamicEndpoint(1, 7, ...))
+ * then emberAfGetClusterServerEndpointIndex(7, X) returns 3 (fixedClusterServerEndpointCount{2} + DynamicEndpointIndex {1}).
+ *
+ * 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 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);
+
 /**
  * @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
@@ -878,6 +878,53 @@ static uint16_t findIndexFromEndpoint(EndpointId endpoint, bool ignoreDisabledEn
     return kEmberInvalidEndpointIndex;
 }
 
+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)
+    {
+        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)
+    {
+        // This endpoint is a fixed one.
+        // 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 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))
+            {
+                adjustedEndpointIndex++;
+            }
+        }
+
+        // If this asserts, the provided fixedClusterServerEndpointCount doesn't match the app data model.
+        VerifyOrDie(adjustedEndpointIndex < fixedClusterServerEndpointCount);
+        epIndex = adjustedEndpointIndex;
+    }
+    else
+    {
+        // This is a dynamic endpoint.
+        // Its index is just its index in the dynamic endpoint list, offset by fixedClusterServerEndpointCount.
+        epIndex = static_cast<uint16_t>(fixedClusterServerEndpointCount + (epIndex - FIXED_ENDPOINT_COUNT));
+    }
+
+    return epIndex;
+}
+
 bool emberAfEndpointIsEnabled(EndpointId endpoint)
 {
     uint16_t index = findIndexFromEndpoint(endpoint,