diff --git a/scripts/tests/chiptest/test_definition.py b/scripts/tests/chiptest/test_definition.py index 694f8c7e75feb1..a5dc5fe79ede87 100644 --- a/scripts/tests/chiptest/test_definition.py +++ b/scripts/tests/chiptest/test_definition.py @@ -71,9 +71,15 @@ def stop(self): return False def factoryReset(self): + wasRunning = (not self.killed) and self.stop() + for kvs in self.kvsPathSet: if os.path.exists(kvs): os.unlink(kvs) + + if wasRunning: + return self.start() + return True def waitForAnyAdvertisement(self): diff --git a/src/app/tests/suites/TestSystemCommands.yaml b/src/app/tests/suites/TestSystemCommands.yaml index f4ad90f14e42ac..0a7001fdb62158 100644 --- a/src/app/tests/suites/TestSystemCommands.yaml +++ b/src/app/tests/suites/TestSystemCommands.yaml @@ -21,6 +21,9 @@ config: payload: type: char_string defaultValue: "MT:-24J0IX4122-.548G00" # This value needs to be generated + secondNodeId: + type: int64u + defaultValue: 0xDEADBEEF tests: - label: "Wait for the commissioned device to be retrieved" @@ -114,7 +117,7 @@ tests: arguments: values: - name: "nodeId" - value: 0xDEADBEEF + value: secondNodeId - name: "payload" value: payload @@ -125,7 +128,7 @@ tests: arguments: values: - name: "nodeId" - value: 0xDEADBEEF + value: secondNodeId - label: "Stop the second accessory" command: "Stop" @@ -147,6 +150,29 @@ tests: - name: "registerKey" value: "chip-lock-app" + # NOTE: Since we have a new KVS, this is basically a factory reset, so we can commission again. + - label: "Commission second accessory with new KVS from alpha" + identity: "alpha" + cluster: "CommissionerCommands" + command: "PairWithCode" + arguments: + values: + - name: "nodeId" + value: secondNodeId + - name: "payload" + value: payload + + - label: + "Wait for the second commissioned device with new KVS to be retrieved + for alpha" + identity: "alpha" + cluster: "DelayCommands" + command: "WaitForCommissionee" + arguments: + values: + - name: "nodeId" + value: secondNodeId + - label: "Reboot the default accessory" command: "Reboot" @@ -180,3 +206,25 @@ tests: values: - name: "registerKey" value: "chip-lock-app" + + - label: "Commission the now-reset second accessory from alpha" + identity: "alpha" + cluster: "CommissionerCommands" + command: "PairWithCode" + arguments: + values: + - name: "nodeId" + value: secondNodeId + - name: "payload" + value: payload + + - label: + "Wait for the second commissioned device (after reset) to be retrieved + for alpha" + identity: "alpha" + cluster: "DelayCommands" + command: "WaitForCommissionee" + arguments: + values: + - name: "nodeId" + value: secondNodeId diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 3333a6554f3c59..37536ff469c8df 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -2764,6 +2764,15 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio case CommissioningStage::kFindOperational: { // If there is an error, CommissioningStageComplete will be called from OnDeviceConnectionFailureFn. auto scopedPeerId = GetPeerScopedId(proxy->GetDeviceId()); + + // If we ever had a commissioned device with this node ID before, we may + // have stale sessions to it. Make sure we don't re-use any of those, + // because clearly they are not related to this new device we are + // commissioning. We only care about sessions we might reuse, so just + // clearing the ones associated with our fabric index is good enough and + // we don't need to worry about ExpireAllSessionsOnLogicalFabric. + mSystemState->SessionMgr()->ExpireAllSessions(scopedPeerId); + mSystemState->CASESessionMgr()->FindOrEstablishSession(scopedPeerId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback #if CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES diff --git a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h index a9fee22507a367..30fefeece04e5d 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h @@ -146820,6 +146820,7 @@ class TestSystemCommands : public TestCommandBridge { AddArgument("cluster", &mCluster); AddArgument("endpoint", 0, UINT16_MAX, &mEndpoint); AddArgument("payload", &mPayload); + AddArgument("secondNodeId", 0, UINT64_MAX, &mSecondNodeId); AddArgument("timeout", 0, UINT16_MAX, &mTimeout); } // NOLINTEND(clang-analyzer-nullability.NullPassedToNonnull) @@ -146914,28 +146915,46 @@ class TestSystemCommands : public TestCommandBridge { err = TestStartASecondAccessoryWithDifferentKvs_15(); break; case 16: - ChipLogProgress(chipTool, " ***** Test Step 16 : Reboot the default accessory\n"); - err = TestRebootTheDefaultAccessory_16(); + ChipLogProgress(chipTool, " ***** Test Step 16 : Commission second accessory with new KVS from alpha\n"); + err = TestCommissionSecondAccessoryWithNewKvsFromAlpha_16(); break; case 17: - ChipLogProgress(chipTool, " ***** Test Step 17 : Reboot the default accessory by key\n"); - err = TestRebootTheDefaultAccessoryByKey_17(); + ChipLogProgress( + chipTool, " ***** Test Step 17 : Wait for the second commissioned device with new KVS to be retrieved for alpha\n"); + err = TestWaitForTheSecondCommissionedDeviceWithNewKvsToBeRetrievedForAlpha_17(); break; case 18: - ChipLogProgress(chipTool, " ***** Test Step 18 : Reboot the second accessory\n"); - err = TestRebootTheSecondAccessory_18(); + ChipLogProgress(chipTool, " ***** Test Step 18 : Reboot the default accessory\n"); + err = TestRebootTheDefaultAccessory_18(); break; case 19: - ChipLogProgress(chipTool, " ***** Test Step 19 : Factory Reset the default accessory\n"); - err = TestFactoryResetTheDefaultAccessory_19(); + ChipLogProgress(chipTool, " ***** Test Step 19 : Reboot the default accessory by key\n"); + err = TestRebootTheDefaultAccessoryByKey_19(); break; case 20: - ChipLogProgress(chipTool, " ***** Test Step 20 : Factory Reset the default accessory by key\n"); - err = TestFactoryResetTheDefaultAccessoryByKey_20(); + ChipLogProgress(chipTool, " ***** Test Step 20 : Reboot the second accessory\n"); + err = TestRebootTheSecondAccessory_20(); break; case 21: - ChipLogProgress(chipTool, " ***** Test Step 21 : Factory Reset the second accessory\n"); - err = TestFactoryResetTheSecondAccessory_21(); + ChipLogProgress(chipTool, " ***** Test Step 21 : Factory Reset the default accessory\n"); + err = TestFactoryResetTheDefaultAccessory_21(); + break; + case 22: + ChipLogProgress(chipTool, " ***** Test Step 22 : Factory Reset the default accessory by key\n"); + err = TestFactoryResetTheDefaultAccessoryByKey_22(); + break; + case 23: + ChipLogProgress(chipTool, " ***** Test Step 23 : Factory Reset the second accessory\n"); + err = TestFactoryResetTheSecondAccessory_23(); + break; + case 24: + ChipLogProgress(chipTool, " ***** Test Step 24 : Commission the now-reset second accessory from alpha\n"); + err = TestCommissionTheNowResetSecondAccessoryFromAlpha_24(); + break; + case 25: + ChipLogProgress(chipTool, + " ***** Test Step 25 : Wait for the second commissioned device (after reset) to be retrieved for alpha\n"); + err = TestWaitForTheSecondCommissionedDeviceAfterResetToBeRetrievedForAlpha_25(); break; } @@ -147014,6 +147033,18 @@ class TestSystemCommands : public TestCommandBridge { case 21: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); break; + case 22: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; + case 23: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; + case 24: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; + case 25: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; } // Go on to the next test. @@ -147027,12 +147058,13 @@ class TestSystemCommands : public TestCommandBridge { private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 22; + const uint16_t mTestCount = 26; chip::Optional mNodeId; chip::Optional mCluster; chip::Optional mEndpoint; chip::Optional mPayload; + chip::Optional mSecondNodeId; chip::Optional mTimeout; CHIP_ERROR TestWaitForTheCommissionedDeviceToBeRetrieved_0() @@ -147152,7 +147184,7 @@ class TestSystemCommands : public TestCommandBridge { { chip::app::Clusters::CommissionerCommands::Commands::PairWithCode::Type value; - value.nodeId = 3735928559ULL; + value.nodeId = mSecondNodeId.HasValue() ? mSecondNodeId.Value() : 3735928559ULL; value.payload = mPayload.HasValue() ? mPayload.Value() : chip::Span("MT:-24J0IX4122-.548G00", 22); return PairWithCode("alpha", value); } @@ -147161,7 +147193,7 @@ class TestSystemCommands : public TestCommandBridge { { chip::app::Clusters::DelayCommands::Commands::WaitForCommissionee::Type value; - value.nodeId = 3735928559ULL; + value.nodeId = mSecondNodeId.HasValue() ? mSecondNodeId.Value() : 3735928559ULL; return WaitForCommissionee("alpha", value); } @@ -147189,14 +147221,31 @@ class TestSystemCommands : public TestCommandBridge { return Start("alpha", value); } - CHIP_ERROR TestRebootTheDefaultAccessory_16() + CHIP_ERROR TestCommissionSecondAccessoryWithNewKvsFromAlpha_16() + { + + chip::app::Clusters::CommissionerCommands::Commands::PairWithCode::Type value; + value.nodeId = mSecondNodeId.HasValue() ? mSecondNodeId.Value() : 3735928559ULL; + value.payload = mPayload.HasValue() ? mPayload.Value() : chip::Span("MT:-24J0IX4122-.548G00", 22); + return PairWithCode("alpha", value); + } + + CHIP_ERROR TestWaitForTheSecondCommissionedDeviceWithNewKvsToBeRetrievedForAlpha_17() + { + + chip::app::Clusters::DelayCommands::Commands::WaitForCommissionee::Type value; + value.nodeId = mSecondNodeId.HasValue() ? mSecondNodeId.Value() : 3735928559ULL; + return WaitForCommissionee("alpha", value); + } + + CHIP_ERROR TestRebootTheDefaultAccessory_18() { chip::app::Clusters::SystemCommands::Commands::Reboot::Type value; return Reboot("alpha", value); } - CHIP_ERROR TestRebootTheDefaultAccessoryByKey_17() + CHIP_ERROR TestRebootTheDefaultAccessoryByKey_19() { chip::app::Clusters::SystemCommands::Commands::Reboot::Type value; @@ -147205,7 +147254,7 @@ class TestSystemCommands : public TestCommandBridge { return Reboot("alpha", value); } - CHIP_ERROR TestRebootTheSecondAccessory_18() + CHIP_ERROR TestRebootTheSecondAccessory_20() { chip::app::Clusters::SystemCommands::Commands::Reboot::Type value; @@ -147214,14 +147263,14 @@ class TestSystemCommands : public TestCommandBridge { return Reboot("alpha", value); } - CHIP_ERROR TestFactoryResetTheDefaultAccessory_19() + CHIP_ERROR TestFactoryResetTheDefaultAccessory_21() { chip::app::Clusters::SystemCommands::Commands::FactoryReset::Type value; return FactoryReset("alpha", value); } - CHIP_ERROR TestFactoryResetTheDefaultAccessoryByKey_20() + CHIP_ERROR TestFactoryResetTheDefaultAccessoryByKey_22() { chip::app::Clusters::SystemCommands::Commands::FactoryReset::Type value; @@ -147230,7 +147279,7 @@ class TestSystemCommands : public TestCommandBridge { return FactoryReset("alpha", value); } - CHIP_ERROR TestFactoryResetTheSecondAccessory_21() + CHIP_ERROR TestFactoryResetTheSecondAccessory_23() { chip::app::Clusters::SystemCommands::Commands::FactoryReset::Type value; @@ -147238,6 +147287,23 @@ class TestSystemCommands : public TestCommandBridge { value.registerKey.Value() = chip::Span("chip-lock-appgarbage: not in length on purpose", 13); return FactoryReset("alpha", value); } + + CHIP_ERROR TestCommissionTheNowResetSecondAccessoryFromAlpha_24() + { + + chip::app::Clusters::CommissionerCommands::Commands::PairWithCode::Type value; + value.nodeId = mSecondNodeId.HasValue() ? mSecondNodeId.Value() : 3735928559ULL; + value.payload = mPayload.HasValue() ? mPayload.Value() : chip::Span("MT:-24J0IX4122-.548G00", 22); + return PairWithCode("alpha", value); + } + + CHIP_ERROR TestWaitForTheSecondCommissionedDeviceAfterResetToBeRetrievedForAlpha_25() + { + + chip::app::Clusters::DelayCommands::Commands::WaitForCommissionee::Type value; + value.nodeId = mSecondNodeId.HasValue() ? mSecondNodeId.Value() : 3735928559ULL; + return WaitForCommissionee("alpha", value); + } }; class TestBinding : public TestCommandBridge {