address comments

project-chip · Aug 11, 2022 · da81c13 · da81c13
1 parent 28b0941
commit da81c13
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 30 deletions.
diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -77,9 +77,9 @@ void InteractionModelEngine::Shutdown()
     //
     while (handlerIter)
     {
-        CommandHandlerInterface * pNext = handlerIter->GetNext();
+        CommandHandlerInterface * nextHandler = handlerIter->GetNext();
         handlerIter->SetNext(nullptr);
-        handlerIter = pNext;
+        handlerIter = nextHandler;
     }
 
     mCommandHandlerList = nullptr;
@@ -236,24 +236,6 @@ uint32_t InteractionModelEngine::GetNumActiveWriteHandlers() const
     return numActive;
 }
 
-void InteractionModelEngine::CloseTransactionsFromFabricIndex(FabricIndex aFabricIndex)
-{
-    //
-    // Walk through all existing subscriptions and shut down those whose subscriber matches
-    // that which just came in.
-    //
-    mReadHandlers.ForEachActiveObject([this, aFabricIndex](ReadHandler * handler) {
-        if (handler->GetAccessingFabricIndex() == aFabricIndex)
-        {
-            ChipLogProgress(InteractionModel, "Deleting expired ReadHandler for NodeId: " ChipLogFormatX64 ", FabricIndex: %u",
-                            ChipLogValueX64(handler->GetInitiatorNodeId()), aFabricIndex);
-            mReadHandlers.ReleaseObject(handler);
-        }
-
-        return Loop::Continue;
-    });
-}
-
 CHIP_ERROR InteractionModelEngine::ShutdownSubscription(SubscriptionId aSubscriptionId)
 {
     for (auto * readClient = mpActiveReadClientList; readClient != nullptr; readClient = readClient->GetNextClient())
@@ -1440,7 +1422,8 @@ void InteractionModelEngine::OnFabricRemoved(const FabricTable & fabricTable, Fa
     mReadHandlers.ForEachActiveObject([fabricIndex](ReadHandler * handler) {
         if (handler->GetAccessingFabricIndex() == fabricIndex)
         {
-            ChipLogProgress(InteractionModel, "Fabric removed, deleting obsolete read handler with FabricIndex: %u", fabricIndex);
+            ChipLogProgress(InteractionModel, "Deleting expired ReadHandler for NodeId: " ChipLogFormatX64 ", FabricIndex: %u",
+                ChipLogValueX64(handler->GetInitiatorNodeId()), fabricIndex);
             handler->Close();
         }
 
@@ -1452,8 +1435,9 @@ void InteractionModelEngine::OnFabricRemoved(const FabricTable & fabricTable, Fa
         if (readClient->GetFabricIndex() == fabricIndex)
         {
             ChipLogProgress(InteractionModel, "Fabric removed, deleting obsolete read client with FabricIndex: %u", fabricIndex);
-            readClient->Close(CHIP_NO_ERROR, false);
             RemoveReadClient(readClient);
+            readClient->mpImEngine = nullptr;
+            readClient->Close(CHIP_ERROR_IM_FABRIC_DELETED, false);
         }
     }
 

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -144,12 +144,6 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
      */
     void ShutdownAllSubscriptions();
 
-    /**
-     * Expire active transactions and release related objects for the given fabric index.
-     * This is used for releasing transactions that won't be closed when a fabric is removed.
-     */
-    void CloseTransactionsFromFabricIndex(FabricIndex aFabricIndex);
-
     uint32_t GetNumActiveReadHandlers() const;
     uint32_t GetNumActiveReadHandlers(ReadHandler::InteractionType type) const;
 
@@ -290,6 +284,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
      */
     uint16_t GetMinGuaranteedSubscriptionsPerFabric() const;
 
+    // virtual method from FabricTable::Delegate
     void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override;
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST

diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
@@ -86,7 +86,7 @@ ReadClient::~ReadClient()
         // This won't be the case if the engine shut down before this destructor was called (in which case, mpImEngine
         // will point to null)
         //
-        if (mpImEngine && mpImEngine->InActiveReadClientList(this))
+        if (mpImEngine)
         {
             mpImEngine->RemoveReadClient(this);
         }

diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
@@ -385,7 +385,6 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
     // Gets called when a fabric is added/updated, but not necessarily committed to storage
     void OnFabricUpdated(const FabricTable & fabricTable, FabricIndex fabricIndex) override
     {
-        ChipLogProgress(InteractionModel, "OnFabricUpdated debugging from opertonal-credential!!!!!!!");
         NotifyFabricTableChanged();
     }
 

diff --git a/src/lib/core/CHIPError.cpp b/src/lib/core/CHIPError.cpp
@@ -731,6 +731,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
     case CHIP_ERROR_MISSING_URI_SEPARATOR.AsInteger():
         desc = "The URI separator is missing";
         break;
+    case CHIP_ERROR_IM_FABRIC_DELETED.AsInteger():
+        desc = "The fabric is deleted, and the corresponding IM resources are released";
+        break;
     }
 #endif // !CHIP_CONFIG_SHORT_ERROR_STR
 

diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h
@@ -2428,6 +2428,14 @@ using CHIP_ERROR = ::chip::ChipError;
  */
 #define CHIP_ERROR_MISSING_URI_SEPARATOR                      CHIP_CORE_ERROR(0xe0)
 
+/**
+ * @def CHIP_ERROR_IM_FABRIC_DELETED
+ *
+ * @brief
+ *   The fabric is deleted, and the corresponding IM resources are released
+ */
+#define CHIP_ERROR_IM_FABRIC_DELETED                            CHIP_CORE_ERROR(0xe1)
+
 // clang-format on
 
 // !!!!! IMPORTANT !!!!!  If you add new CHIP errors, please update the translation