Stop storing breadcrumb in persistent storage.

Breadcrumb is not supposed to be stored persistently.

Also adds a bunch of tests and fixes various bugs where we were
setting breadcrumbs when we should not (e.g. in error conditions) and
not setting them when we should (e.g. when the fail-safe expires).

Fixes project-chip#17515

src/app/clusters/general-commissioning-server/general-commissioning-server.cpp

@@ -68,7 +68,6 @@ class GeneralCommissioningAttrAccess : public AttributeAccessInterface
     GeneralCommissioningAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), GeneralCommissioning::Id) {}
 
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
-    CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override;
 
 private:
     CHIP_ERROR ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &), AttributeValueEncoder & aEncoder);
@@ -100,37 +99,13 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
     case SupportsConcurrentConnection::Id: {
         return ReadSupportsConcurrentConnection(aEncoder);
     }
-    case Breadcrumb::Id: {
-        return aEncoder.Encode(DeviceLayer::DeviceControlServer::DeviceControlSvr().GetBreadcrumb());
-    }
     default: {
         break;
     }
     }
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR GeneralCommissioningAttrAccess::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
-{
-    // TODO: There was discussion about moving the breadcrumb to the attribute store, which would make this function obsolete
-
-    if (aPath.mClusterId != GeneralCommissioning::Id)
-    {
-        // We shouldn't have been called at all.
-        return CHIP_ERROR_INVALID_ARGUMENT;
-    }
-
-    switch (aPath.mAttributeId)
-    {
-    case Attributes::Breadcrumb::Id:
-        Attributes::Breadcrumb::TypeInfo::DecodableType value;
-        ReturnErrorOnFailure(aDecoder.Decode(value));
-        return DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(value);
-    default:
-        return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
-    }
-}
-
 CHIP_ERROR GeneralCommissioningAttrAccess::ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &),
                                                            AttributeValueEncoder & aEncoder)
 {
@@ -208,6 +183,8 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler *
         {
             // Force the timer to expire immediately.
             failSafeContext.ForceFailSafeTimerExpiry();
+            // Don't set the breadcrumb, since expiring the failsafe should
+            // reset it anyway.
             response.errorCode = CommissioningError::kOk;
             commandObj->AddResponse(commandPath, response);
         }
@@ -216,10 +193,10 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler *
             CheckSuccess(
                 failSafeContext.ArmFailSafe(accessingFabricIndex, System::Clock::Seconds16(commandData.expiryLengthSeconds)),
                 Failure);
+            Breadcrumb::Set(commandPath.mEndpointId, commandData.breadcrumb);
             response.errorCode = CommissioningError::kOk;
             commandObj->AddResponse(commandPath, response);
         }
-        DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(commandData.breadcrumb);
     }
     else
     {
@@ -265,11 +242,11 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
             CheckSuccess(server->CommissioningComplete(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex()),
                          Failure);
 
+            Breadcrumb::Set(commandPath.mEndpointId, 0);
             response.errorCode = CommissioningError::kOk;
         }
     }
 
-    DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(0);
     commandObj->AddResponse(commandPath, response);
 
     return true;
@@ -282,9 +259,8 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
     MATTER_TRACE_EVENT_SCOPE("SetRegulatoryConfig", "GeneralCommissioning");
     DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
 
-    CheckSuccess(server->SetRegulatoryConfig(to_underlying(commandData.newRegulatoryConfig), commandData.countryCode,
-                                             commandData.breadcrumb),
-                 Failure);
+    CheckSuccess(server->SetRegulatoryConfig(to_underlying(commandData.newRegulatoryConfig), commandData.countryCode), Failure);
+    Breadcrumb::Set(commandPath.mEndpointId, commandData.breadcrumb);
 
     Commands::SetRegulatoryConfigResponse::Type response;
     response.errorCode = CommissioningError::kOk;
@@ -293,8 +269,21 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
     return true;
 }
 
+namespace {
+void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
+{
+    if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
+    {
+        // Spec says to reset Breadcrumb attribute to 0.
+        Breadcrumb::Set(0, 0);
+    }
+}
+
+} // anonymous namespace
+
 void MatterGeneralCommissioningPluginServerInitCallback()
 {
-    DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(0);
+    Breadcrumb::Set(0, 0);
     registerAttributeAccessOverride(&gAttrAccess);
+    DeviceLayer::PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler);
 }

src/app/tests/suites/TestGeneralCommissioning.yaml

@@ -18,6 +18,12 @@ config:
     nodeId: 0x12344321
     cluster: "General Commissioning"
     endpoint: 0
+    discriminator:
+        type: INT16U
+        defaultValue: 3840
+    payload:
+        type: CHAR_STRING
+        defaultValue: "MT:-24J0AFN00KA0648G00" # This value needs to be generated automatically
 
 tests:
     - label: "Wait for the commissioned device to be retrieved"
@@ -52,6 +58,214 @@ tests:
       response:
           value: 81
 
+    - label: "Reboot to reset Breadcrumb"
+      cluster: "SystemCommands"
+      command: "Reboot"
+
+    - label: "Connect to the device again"
+      cluster: "DelayCommands"
+      command: "WaitForCommissionee"
+      arguments:
+          values:
+              - name: "nodeId"
+                value: nodeId
+
+    - label: "Read back Breadcrumb after reboot and ensure it was not persisted"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 0
+
+    - label: "Set Breadcrumb to nonzero value"
+      command: "writeAttribute"
+      attribute: "Breadcrumb"
+      arguments:
+          value: 1
+
+    - label: "Check Breadcrumb set worked"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 1
+
+    - label: "Send CommissioningComplete without armed fail-safe"
+      command: "CommissioningComplete"
+      response:
+          values:
+              - name: errorCode
+                value: 3 # NoFailSafe
+
+    - label: "Check Breadcrumb was not touched by invalid CommissioningComplete"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 1
+
+    - label: "Open Commissioning Window from alpha"
+      cluster: "AdministratorCommissioning"
+      command: "OpenBasicCommissioningWindow"
+      timedInteractionTimeoutMs: 10000
+      arguments:
+          values:
+              - name: "CommissioningTimeout"
+                value: 180
+
+    # Arming a fail-safe over CASE while a commissioning window is open should not work.
+    - label: "Try to arm fail-safe"
+      command: "ArmFailSafe"
+      arguments:
+          values:
+              - name: expiryLengthSeconds
+                value: 10
+              - name: breadcrumb
+                value: 5000
+      response:
+          values:
+              - name: errorCode
+                value: 4 # BusyWithOtherAdmin
+
+    - label:
+          "Check Breadcrumb was not touched by ArmFailSafe with commissioning
+          window open"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 1
+
+    - label: "Reset Breadcrumb to 0 so we can commission"
+      command: "writeAttribute"
+      attribute: "Breadcrumb"
+      arguments:
+          value: 0
+
+    - label: "Commission from beta"
+      identity: "beta"
+      cluster: "CommissionerCommands"
+      command: "PairWithQRCode"
+      arguments:
+          values:
+              - name: "nodeId"
+                value: 0x12345
+              - name: "payload"
+                value: payload
+
+    - label: "Wait for the commissioned device to be retrieved for beta"
+      identity: "beta"
+      cluster: "DelayCommands"
+      command: "WaitForCommissionee"
+      arguments:
+          values:
+              - name: "nodeId"
+                value: 0x12345
+
+    - label: "Arm fail-safe"
+      command: "ArmFailSafe"
+      arguments:
+          values:
+              - name: expiryLengthSeconds
+                value: 500
+              - name: breadcrumb
+                value: 2
+      response:
+          values:
+              - name: errorCode
+                value: 0 # OK
+
+    - label: "Check Breadcrumb was properly set by ArmFailSafe"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 2
+
+    - label: "Try to arm fail-safe from wrong fabric"
+      command: "ArmFailSafe"
+      identity: "beta"
+      arguments:
+          values:
+              - name: expiryLengthSeconds
+                value: 10
+              - name: breadcrumb
+                value: 5000
+      response:
+          values:
+              - name: errorCode
+                value: 4 # BusyWithOtherAdmin
+
+    - label:
+          "Check Breadcrumb was not touched by ArmFailSafe with existing
+          fail-safe armed"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 2
+
+    - label: "Send CommissioningComplete from wrong fabric"
+      command: "CommissioningComplete"
+      identity: "beta"
+      response:
+          values:
+              - name: errorCode
+                value: 2 # InvalidAuthentication
+
+    - label:
+          "Check Breadcrumb was not touched by CommissioningComplete from wrong
+          fabric"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 2
+
+    - label: "Close out the fail-safe gracefully"
+      command: "CommissioningComplete"
+      response:
+          values:
+              - name: errorCode
+                value: 0 # Ok
+
+    - label: "Check Breadcrumb was reset to 0 by CommissioningComplete"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 0
+
+    - label: "Arm fail-safe again"
+      command: "ArmFailSafe"
+      arguments:
+          values:
+              - name: expiryLengthSeconds
+                value: 500
+              - name: breadcrumb
+                value: 3
+      response:
+          values:
+              - name: errorCode
+                value: 0 # OK
+
+    - label: "Check Breadcrumb was set by arming fail-safe again"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 3
+
+    - label: "Force-expire the fail-safe"
+      command: "ArmFailSafe"
+      arguments:
+          values:
+              - name: expiryLengthSeconds
+                value: 0
+              - name: breadcrumb
+                value: 4
+      response:
+          values:
+              - name: errorCode
+                value: 0 # OK
+
+    - label: "Check Breadcrumb was reset by expiring the fail-safe"
+      command: "readAttribute"
+      attribute: "Breadcrumb"
+      response:
+          value: 0
+
     - label: "Validate presence of SupportsConcurrentConnection"
       command: "readAttribute"
       attribute: "SupportsConcurrentConnection"

src/darwin/Framework/CHIP/templates/tests/tests.js

@@ -272,7 +272,9 @@ function getTests()
     'TestDelayCommands',
     'TestDescriptorCluster',
     'TestBasicInformation',
-    'TestGeneralCommissioning',
+    // TestGeneralCommissioning does reboots that need to have side-effects, so
+    // can't be tested in this test framework.
+    //'TestGeneralCommissioning',
     'TestGroupsCluster',
     'TestGroupKeyManagementCluster',
     'TestIdentifyCluster',

src/include/platform/ConfigurationManager.h

@@ -109,7 +109,6 @@ class ConfigurationManager
 #endif
     virtual CHIP_ERROR GetRegulatoryLocation(uint8_t & location)                       = 0;
     virtual CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen)    = 0;
-    virtual CHIP_ERROR GetBreadcrumb(uint64_t & breadcrumb)                            = 0;
     virtual CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen)  = 0;
     virtual CHIP_ERROR StorePrimaryWiFiMACAddress(const uint8_t * buf)                 = 0;
     virtual CHIP_ERROR StorePrimary802154MACAddress(const uint8_t * buf)               = 0;
@@ -118,7 +117,6 @@ class ConfigurationManager
     virtual CHIP_ERROR StoreHardwareVersion(uint16_t hardwareVer)                      = 0;
     virtual CHIP_ERROR StoreRegulatoryLocation(uint8_t location)                       = 0;
     virtual CHIP_ERROR StoreCountryCode(const char * code, size_t codeLen)             = 0;
-    virtual CHIP_ERROR StoreBreadcrumb(uint64_t breadcrumb)                            = 0;
     virtual CHIP_ERROR GetRebootCount(uint32_t & rebootCount)                          = 0;
     virtual CHIP_ERROR StoreRebootCount(uint32_t rebootCount)                          = 0;
     virtual CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours)      = 0;

src/include/platform/DeviceControlServer.h

@@ -89,14 +89,12 @@ class DeviceControlServer final
     // ===== Members for internal use by other Device Layer components.
 
     CHIP_ERROR CommissioningComplete(NodeId peerNodeId, FabricIndex accessingFabricIndex);
-    CHIP_ERROR SetRegulatoryConfig(uint8_t location, const CharSpan & countryCode, uint64_t breadcrumb);
+    CHIP_ERROR SetRegulatoryConfig(uint8_t location, const CharSpan & countryCode);
     CHIP_ERROR ConnectNetworkForOperational(ByteSpan networkID);
 
     void SetSwitchDelegate(SwitchDeviceControlDelegate * delegate) { mSwitchDelegate = delegate; }
     SwitchDeviceControlDelegate * GetSwitchDelegate() const { return mSwitchDelegate; }
     FailSafeContext & GetFailSafeContext() { return mFailSafeContext; }
-    CHIP_ERROR SetBreadcrumb(uint64_t breadcrumb);
-    uint64_t GetBreadcrumb();
 
     static DeviceControlServer & DeviceControlSvr();
 

src/include/platform/internal/GenericConfigurationManagerImpl.h

@@ -98,8 +98,6 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR StoreRegulatoryLocation(uint8_t location) override;
     CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen) override;
     CHIP_ERROR StoreCountryCode(const char * code, size_t codeLen) override;
-    CHIP_ERROR GetBreadcrumb(uint64_t & breadcrumb) override;
-    CHIP_ERROR StoreBreadcrumb(uint64_t breadcrumb) override;
     CHIP_ERROR GetRebootCount(uint32_t & rebootCount) override;
     CHIP_ERROR StoreRebootCount(uint32_t rebootCount) override;
     CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours) override;