Do not use CommissioningComplete to signal failsafe timer expired (#1…

…6454) * Do not use CommissioningComplete to signal failsafe timer exipred * Add PeerFabricIndex field back * Update src/include/platform/CHIPDeviceEvent.h Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Mar 19, 2022 · 01503be · 01503be
1 parent a3390fd
commit 01503be
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 62 deletions.
diff --git a/examples/tv-casting-app/linux/main.cpp b/examples/tv-casting-app/linux/main.cpp
@@ -507,12 +507,6 @@ void DeviceEventCallback(const DeviceLayer::ChipDeviceEvent * event, intptr_t ar
     }
     else if (event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete)
     {
-        if (event->CommissioningComplete.Status != CHIP_NO_ERROR)
-        {
-            ChipLogError(AppServer, "Commissioning is not successfully Complete");
-            return;
-        }
-
         ReturnOnFailure(gTargetVideoPlayerInfo.Initialize(event->CommissioningComplete.PeerNodeId,
                                                           event->CommissioningComplete.PeerFabricIndex));
 

diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp
@@ -85,8 +85,7 @@ CHIP_ERROR Instance::Init()
 {
     ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->RegisterCommandHandler(this));
     VerifyOrReturnError(registerAttributeAccessOverride(this), CHIP_ERROR_INCORRECT_STATE);
-    ReturnErrorOnFailure(
-        DeviceLayer::PlatformMgrImpl().AddEventHandler(_OnCommissioningComplete, reinterpret_cast<intptr_t>(this)));
+    ReturnErrorOnFailure(DeviceLayer::PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this)));
     ReturnErrorOnFailure(mpBaseDriver->Init(this));
     mLastNetworkingStatusValue.SetNull();
     mLastConnectErrorValue.SetNull();
@@ -489,29 +488,36 @@ void Instance::OnFinished(Status status, CharSpan debugText, WiFiScanResponseIte
     }
 }
 
-void Instance::_OnCommissioningComplete(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
+void Instance::OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
 {
     Instance * this_ = reinterpret_cast<Instance *>(arg);
-    VerifyOrReturn(event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete);
-    this_->OnCommissioningComplete(event->CommissioningComplete.Status);
-}
-
-void Instance::OnCommissioningComplete(CHIP_ERROR err)
-{
-    VerifyOrReturn(mpWirelessDriver != nullptr);
 
-    if (err == CHIP_NO_ERROR)
+    if (event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete)
     {
-        ChipLogDetail(Zcl, "Commissioning complete, notify platform driver to persist network credentials.");
-        mpWirelessDriver->CommitConfiguration();
+        this_->OnCommissioningComplete();
     }
-    else
+    else if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
     {
-        ChipLogDetail(Zcl, "Failsafe timeout, tell platform driver to revert network credentials.");
-        mpWirelessDriver->RevertConfiguration();
+        this_->OnFailSafeTimerExpired();
     }
 }
 
+void Instance::OnCommissioningComplete()
+{
+    VerifyOrReturn(mpWirelessDriver != nullptr);
+
+    ChipLogDetail(Zcl, "Commissioning complete, notify platform driver to persist network credentials.");
+    mpWirelessDriver->CommitConfiguration();
+}
+
+void Instance::OnFailSafeTimerExpired()
+{
+    VerifyOrReturn(mpWirelessDriver != nullptr);
+
+    ChipLogDetail(Zcl, "Failsafe timeout, tell platform driver to revert network credentials.");
+    mpWirelessDriver->RevertConfiguration();
+}
+
 bool NullNetworkDriver::GetEnabled()
 {
     // Disable the interface and it cannot be enabled since there are no physical interfaces.

diff --git a/src/app/clusters/network-commissioning/network-commissioning.h b/src/app/clusters/network-commissioning/network-commissioning.h
@@ -72,8 +72,9 @@ class Instance : public CommandHandlerInterface,
                     DeviceLayer::NetworkCommissioning::ThreadScanResponseIterator * networks) override;
 
 private:
-    static void _OnCommissioningComplete(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
-    void OnCommissioningComplete(CHIP_ERROR err);
+    static void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
+    void OnCommissioningComplete();
+    void OnFailSafeTimerExpired();
 
     const BitFlags<NetworkCommissioningFeature> mFeatureFlags;
 

diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
@@ -267,12 +267,12 @@ void FailSafeCleanup(const chip::DeviceLayer::ChipDeviceEvent * event)
 {
     emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Call to FailSafeCleanup");
 
-    FabricIndex fabricIndex = event->CommissioningComplete.PeerFabricIndex;
+    FabricIndex fabricIndex = event->FailSafeTimerExpired.PeerFabricIndex;
 
     // If an AddNOC or UpdateNOC command has been successfully invoked, terminate all CASE sessions associated with the Fabric
     // whose Fabric Index is recorded in the Fail-Safe context (see ArmFailSafe Command) by clearing any associated Secure
     // Session Context at the Server.
-    if (event->CommissioningComplete.AddNocCommandHasBeenInvoked || event->CommissioningComplete.UpdateNocCommandHasBeenInvoked)
+    if (event->FailSafeTimerExpired.AddNocCommandHasBeenInvoked || event->FailSafeTimerExpired.UpdateNocCommandHasBeenInvoked)
     {
         CASESessionManager * caseSessionManager = Server::GetInstance().GetCASESessionManager();
         if (caseSessionManager)
@@ -289,28 +289,25 @@ void FailSafeCleanup(const chip::DeviceLayer::ChipDeviceEvent * event)
     // If an AddNOC command had been successfully invoked, achieve the equivalent effect of invoking the RemoveFabric command
     // against the Fabric Index stored in the Fail-Safe Context for the Fabric Index that was the subject of the AddNOC
     // command.
-    if (event->CommissioningComplete.AddNocCommandHasBeenInvoked)
+    if (event->FailSafeTimerExpired.AddNocCommandHasBeenInvoked)
     {
         Server::GetInstance().GetFabricTable().Delete(fabricIndex);
     }
 
     // If an UpdateNOC command had been successfully invoked, revert the state of operational key pair, NOC and ICAC for that
     // Fabric to the state prior to the Fail-Safe timer being armed, for the Fabric Index that was the subject of the UpdateNOC
     // command.
-    if (event->CommissioningComplete.UpdateNocCommandHasBeenInvoked)
+    if (event->FailSafeTimerExpired.UpdateNocCommandHasBeenInvoked)
     {
         // TODO: Revert the state of operational key pair, NOC and ICAC
     }
 }
 
 void OnPlatformEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
 {
-    if (event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete)
+    if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
     {
-        if (event->CommissioningComplete.Status != CHIP_NO_ERROR)
-        {
-            FailSafeCleanup(event);
-        }
+        FailSafeCleanup(event);
     }
 }
 

diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp
@@ -55,17 +55,13 @@ void CommissioningWindowManager::OnPlatformEvent(const DeviceLayer::ChipDeviceEv
 {
     if (event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete)
     {
-        if (event->CommissioningComplete.Status == CHIP_NO_ERROR)
-        {
-            ChipLogProgress(AppServer, "Commissioning completed successfully");
-            Cleanup();
-        }
-        else
-        {
-            ChipLogError(AppServer, "Commissioning failed with error %" CHIP_ERROR_FORMAT,
-                         event->CommissioningComplete.Status.Format());
-            OnSessionEstablishmentError(event->CommissioningComplete.Status);
-        }
+        ChipLogProgress(AppServer, "Commissioning completed successfully");
+        Cleanup();
+    }
+    else if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
+    {
+        ChipLogError(AppServer, "Failsafe timer expired");
+        OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT);
     }
     else if (event->Type == DeviceLayer::DeviceEventType::kOperationalNetworkEnabled)
     {

diff --git a/src/include/platform/CHIPDeviceEvent.h b/src/include/platform/CHIPDeviceEvent.h
@@ -216,10 +216,16 @@ enum PublicEventTypes
     kInterfaceIpAddressChanged,
 
     /**
-     * Commissioning has completed either through timer expiry or by a call to the general commissioning cluster command.
+     * Commissioning has completed by a call to the general commissioning cluster command.
      */
     kCommissioningComplete,
 
+    /**
+     * Signals that the fail-safe timer expired before the CommissioningComplete command was
+     * successfully invoked.
+     */
+    kFailSafeTimerExpired,
+
     /**
      *
      */
@@ -446,12 +452,16 @@ struct ChipDeviceEvent final
 
         struct
         {
-            CHIP_ERROR Status;
             uint64_t PeerNodeId;
             FabricIndex PeerFabricIndex;
+        } CommissioningComplete;
+
+        struct
+        {
+            FabricIndex PeerFabricIndex;
             bool AddNocCommandHasBeenInvoked;
             bool UpdateNocCommandHasBeenInvoked;
-        } CommissioningComplete;
+        } FailSafeTimerExpired;
 
         struct
         {

diff --git a/src/include/platform/FailSafeContext.h b/src/include/platform/FailSafeContext.h
@@ -88,7 +88,7 @@ class FailSafeContext
     // TODO:: Track the state of what was mutated during fail-safe.
 
     static void HandleArmFailSafe(System::Layer * layer, void * aAppState);
-    void CommissioningFailedTimerComplete();
+    void FailSafeTimerExpired();
 };
 
 } // namespace DeviceLayer

diff --git a/src/platform/DeviceControlServer.cpp b/src/platform/DeviceControlServer.cpp
@@ -40,12 +40,9 @@ CHIP_ERROR DeviceControlServer::CommissioningComplete(NodeId peerNodeId, FabricI
 
     ChipDeviceEvent event;
 
-    event.Type                                                 = DeviceEventType::kCommissioningComplete;
-    event.CommissioningComplete.PeerNodeId                     = peerNodeId;
-    event.CommissioningComplete.PeerFabricIndex                = accessingFabricIndex;
-    event.CommissioningComplete.AddNocCommandHasBeenInvoked    = mFailSafeContext.AddNocCommandHasBeenInvoked();
-    event.CommissioningComplete.UpdateNocCommandHasBeenInvoked = mFailSafeContext.UpdateNocCommandHasBeenInvoked();
-    event.CommissioningComplete.Status                         = CHIP_NO_ERROR;
+    event.Type                                  = DeviceEventType::kCommissioningComplete;
+    event.CommissioningComplete.PeerNodeId      = peerNodeId;
+    event.CommissioningComplete.PeerFabricIndex = accessingFabricIndex;
 
     return PlatformMgr().PostEvent(&event);
 }

diff --git a/src/platform/FailSafeContext.cpp b/src/platform/FailSafeContext.cpp
@@ -30,18 +30,17 @@ namespace DeviceLayer {
 void FailSafeContext::HandleArmFailSafe(System::Layer * layer, void * aAppState)
 {
     FailSafeContext * context = reinterpret_cast<FailSafeContext *>(aAppState);
-    context->CommissioningFailedTimerComplete();
+    context->FailSafeTimerExpired();
 }
 
-void FailSafeContext::CommissioningFailedTimerComplete()
+void FailSafeContext::FailSafeTimerExpired()
 {
     ChipDeviceEvent event;
-    event.Type                                                 = DeviceEventType::kCommissioningComplete;
-    event.CommissioningComplete.PeerFabricIndex                = mFabricIndex;
-    event.CommissioningComplete.AddNocCommandHasBeenInvoked    = mAddNocCommandHasBeenInvoked;
-    event.CommissioningComplete.UpdateNocCommandHasBeenInvoked = mUpdateNocCommandHasBeenInvoked;
-    event.CommissioningComplete.Status                         = CHIP_ERROR_TIMEOUT;
-    CHIP_ERROR status                                          = PlatformMgr().PostEvent(&event);
+    event.Type                                                = DeviceEventType::kFailSafeTimerExpired;
+    event.FailSafeTimerExpired.PeerFabricIndex                = mFabricIndex;
+    event.FailSafeTimerExpired.AddNocCommandHasBeenInvoked    = mAddNocCommandHasBeenInvoked;
+    event.FailSafeTimerExpired.UpdateNocCommandHasBeenInvoked = mUpdateNocCommandHasBeenInvoked;
+    CHIP_ERROR status                                         = PlatformMgr().PostEvent(&event);
 
     mFailSafeArmed                  = false;
     mAddNocCommandHasBeenInvoked    = false;