fix crash when enabling endpoint which has already been enabled

ambiot · Aug 7, 2023 · 9f65cbe · 9f65cbe
1 parent e9fd382
commit 9f65cbe
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 8 deletions.
diff --git a/component/common/application/matter/core/matter_data_model.cpp b/component/common/application/matter/core/matter_data_model.cpp
@@ -662,6 +662,12 @@ void Endpoint::setParentEndpointId(chip::EndpointId newParentEndpointId)
 
 void Endpoint::enableEndpoint(Span<const EmberAfDeviceType> deviceTypeList)
 {
+    if (enabled)
+    {
+        ChipLogDetail(DeviceLayer, "Endpoint %d already enabled", endpointId);
+        return;
+    }
+
     dataVersion = (chip::DataVersion*) calloc(clusters.size(), sizeof(chip::DataVersion));
 
     EmberAfEndpointType *endpointType = nullptr;
@@ -774,11 +780,12 @@ void Endpoint::enableEndpoint(Span<const EmberAfDeviceType> deviceTypeList)
     {
         ChipLogProgress(DeviceLayer, "Set dynamic endpoint %d success", endpointId);
         endpointMetadata = endpointType;
+        enabled = true;
         return;
     }
     else
     {
-        ChipLogError(DeviceLayer, "Failed to create dynamic endpoint %d with status %d", endpointId, status);
+        ChipLogError(DeviceLayer, "Failed to set dynamic endpoint %d with status %d", endpointId, status);
     }
 
     // free allocated memory if error
@@ -832,6 +839,8 @@ void Endpoint::disableEndpoint()
     emberAfClearDynamicEndpoint(endpointIndex);
     chip::DeviceLayer::PlatformMgr().UnlockChipStack();
 
+    enabled = false;
+
     // free allocated memory
     for (EmberAfCluster *cls : clusterGarbageCollector)
     {
@@ -921,7 +930,7 @@ chip::EndpointId Node::getNextEndpointId() const
     return nextEndpointId;
 }
 
-void Node::addEndpoint(const EndpointConfig& endpointConfig)
+chip::EndpointId Node::addEndpoint(const EndpointConfig& endpointConfig)
 {
     Endpoint endpoint(this, nextEndpointId, endpointCount);
     // Set parentEndpointId based on the previous endpoint's endpointId
@@ -964,6 +973,8 @@ void Node::addEndpoint(const EndpointConfig& endpointConfig)
     endpoints.push_back(endpoint);
     endpointCount++;
     nextEndpointId++;
+
+    return endpoint.getEndpointId();
 }
 
 void Node::removeEndpoint(chip::EndpointId endpointId)
@@ -978,6 +989,12 @@ void Node::removeEndpoint(chip::EndpointId endpointId)
         // Get the index of the endpoint in the vector
         int index = std::distance(endpoints.begin(), it);
 
+        if (it->enabled)
+        {
+            ChipLogError(DeviceLayer, "Endpoint not yet disabled!");
+            return;
+        }
+
         // Remove the endpoint from the vector
         endpoints.erase(it);
 

diff --git a/component/common/application/matter/core/matter_data_model.h b/component/common/application/matter/core/matter_data_model.h
@@ -342,6 +342,8 @@ class Cluster
 class Endpoint
 {
 public:
+    friend class Node;
+
     Endpoint(Node* node, chip::EndpointId endpointId, uint16_t endpointCount) : 
         endpointId(endpointId),
         endpointIndex(endpointCount),
@@ -366,6 +368,7 @@ class Endpoint
     Node* parentNode;
     chip::DataVersion *dataVersion = nullptr;
     EmberAfEndpointType *endpointMetadata;
+    bool enabled = false;
 
     // Garbage collectors
     // Store dynamic allocated objects here when endpoint is enabled, then delete it when disabled
@@ -384,7 +387,7 @@ class Node
     static Node& getInstance();
     Endpoint *getEndpoint(chip::EndpointId endpointId);
     chip::EndpointId getNextEndpointId() const;
-    void addEndpoint(const EndpointConfig& endpointConfig);
+    chip::EndpointId addEndpoint(const EndpointConfig& endpointConfig);
     void removeEndpoint(chip::EndpointId endpointId);
     void enableAllEndpoints(Span<const EmberAfDeviceType> deviceTypeList);
     void print() const;

diff --git a/component/common/application/matter/example/light_dm/example_matter_light.cpp b/component/common/application/matter/example/light_dm/example_matter_light.cpp
@@ -65,17 +65,26 @@ static void example_matter_light_task(void *pvParameters)
     Presets::Endpoints::matter_root_node_preset(&rootNodeEndpointConfig);
     Presets::Endpoints::matter_dimmable_light_preset(&dimmableLightEndpointConfig);
 
-    // Initial
+    chip::EndpointId testEndpointId;
+
+    // Initial, root node on ep0, dimmable light on ep1
     node.addEndpoint(rootNodeEndpointConfig);
     node.addEndpoint(dimmableLightEndpointConfig);
 
     // Enable endpoints
     node.enableAllEndpoints(Span<const EmberAfDeviceType>(deviceTypes));
 
-    // Test disable and remove endpoints
-    vTaskDelay(10000);
-    node.getEndpoint(1)->disableEndpoint();
-    node.removeEndpoint(1);
+    vTaskDelay(20000);
+
+    // Test add another dimmable light on ep2
+    testEndpointId = node.addEndpoint(dimmableLightEndpointConfig);
+    node.enableAllEndpoints(Span<const EmberAfDeviceType>(deviceTypes));
+
+    vTaskDelay(20000);
+
+    // Test remove ep2
+    node.getEndpoint(testEndpointId)->disableEndpoint();
+    node.removeEndpoint(testEndpointId);
 
     vTaskDelete(NULL);
 }