From a7af5e472182148f73f2e392add8560afa7d8fa9 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 20 Sep 2021 12:01:28 -0400 Subject: [PATCH] Make dynamic endpoint lifetime more similar to that of fixed endpoints. (#9662) Specific changes: 1) Ensure that dynamic endpoints land in initializeEndpoint (via emberAfEndpointEnableDisable) like fixed ones do (via emAfCallInits). 2) Ensure that clearing an dynamic endpoint properly disables it. This makes sure we call emberAfDeactivateClusterTick as needed and we can add other cleanup inside emberAfEndpointEnableDisable as it becomes useful. 3) Move the emberAfPluginDescriptorServerInitCallback calls for dymanic endpoints to the one choke-point in emberAfEndpointEnableDisable. This also fixes a pre-existing issue where disabling a fixed endpoint would not correctly update the descriptor bits. 4) In descriptor, check for enabled state before trying to actually touch the endpoint's data, not after we have tried to touch some of it. --- src/app/clusters/descriptor/descriptor.cpp | 6 +++--- src/app/util/attribute-storage.cpp | 25 +++++++++++----------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/app/clusters/descriptor/descriptor.cpp b/src/app/clusters/descriptor/descriptor.cpp index e8c820fab6db09..62480e5ac3bc2a 100644 --- a/src/app/clusters/descriptor/descriptor.cpp +++ b/src/app/clusters/descriptor/descriptor.cpp @@ -131,13 +131,13 @@ void emberAfPluginDescriptorServerInitCallback(void) for (uint16_t index = 0; index < emberAfEndpointCount(); index++) { - EndpointId endpoint = emberAfEndpointFromIndex(index); - if (!emberAfContainsCluster(endpoint, Descriptor::Id)) + if (!emberAfEndpointIndexIsEnabled(index)) { continue; } - if (!emberAfEndpointIndexIsEnabled(index)) + EndpointId endpoint = emberAfEndpointFromIndex(index); + if (!emberAfContainsCluster(endpoint, Descriptor::Id)) { continue; } diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 1d950a3b73d944..b7f3b1118a6b90 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -153,6 +153,8 @@ void emberAfEndpointConfigure(void) #ifdef DYNAMIC_ENDPOINT_COUNT if (MAX_ENDPOINT_COUNT > FIXED_ENDPOINT_COUNT) { + // This is assuming that EMBER_AF_ENDPOINT_DISABLED is 0 + static_assert(EMBER_AF_ENDPOINT_DISABLED == 0, "We are creating enabled dynamic endpoints!"); memset(&emAfEndpoints[FIXED_ENDPOINT_COUNT], 0, sizeof(EmberAfDefinedEndpoint) * (MAX_ENDPOINT_COUNT - FIXED_ENDPOINT_COUNT)); } @@ -201,15 +203,14 @@ EmberAfStatus emberAfSetDynamicEndpoint(uint16_t index, EndpointId id, EmberAfEn emAfEndpoints[index].deviceVersion = deviceVersion; emAfEndpoints[index].endpointType = ep; emAfEndpoints[index].networkIndex = 0; - emAfEndpoints[index].bitmask = EMBER_AF_ENDPOINT_ENABLED; + // Start the endpoint off as disabled. + emAfEndpoints[index].bitmask = EMBER_AF_ENDPOINT_DISABLED; emberAfSetDynamicEndpointCount(MAX_ENDPOINT_COUNT - FIXED_ENDPOINT_COUNT); - emberAfSetDeviceEnabled(id, true); -#ifdef ZCL_USING_DESCRIPTOR_CLUSTER_SERVER - // Rebuild descriptor attributes on all endpoints - emberAfPluginDescriptorServerInitCallback(); -#endif + // Now enable the endpoint. + emberAfEndpointEnableDisable(id, true); + emberAfSetDeviceEnabled(id, true); return EMBER_ZCL_STATUS_SUCCESS; } @@ -226,14 +227,9 @@ EndpointId emberAfClearDynamicEndpoint(uint16_t index) if (ep) { emberAfSetDeviceEnabled(ep, false); + emberAfEndpointEnableDisable(ep, false); emAfEndpoints[index].endpoint = 0; - emAfEndpoints[index].bitmask = 0; } - -#ifdef ZCL_USING_DESCRIPTOR_CLUSTER_SERVER - // Rebuild descriptor attributes on all endpoints - emberAfPluginDescriptorServerInitCallback(); -#endif } return ep; @@ -1014,6 +1010,11 @@ bool emberAfEndpointEnableDisable(EndpointId endpoint, bool enable) cur = next; } } + +#ifdef ZCL_USING_DESCRIPTOR_CLUSTER_SERVER + // Rebuild descriptor attributes on all endpoints + emberAfPluginDescriptorServerInitCallback(); +#endif } return true;