Improve DeviceCommissioner::StopPairing behavior (#32233)

* Prefix DeviceCommissioner members with 'm'

* Move OnBasic* into DeviceCommissioner as private helpers

* Consistently check for presence of mDefaultCommissioner first

* Darwin: Reduce boilerplate in MTRError and use switch

* Darwin: Add MTRErrorCodeCancelled

* Darwin: Extend MTRPairingTests to cover cancellation

* Improve DeviceCommissioner::StopPairing behavior

Improve StopPairing behavior by cancelling the current read or invoke
interaction to avoid stray callbacks messing up a future commissioning attempt.

Rename SendCommand to SendCommissioningCommand.

* Make gLogFilter atomic to avoid races / TSAN failures

Note that most constrained platforms build with CHIP_LOG_FILTERING=0
so will not be incurring any extra runtime cost because of this change.

* Exclude Darwin code from PRI*64 lint

* Document InvokeCancelFn as internal-only

* Address review comments

- Log VerifyOrDie failures at Error level instead of Detail
- Use reset() for clearing UniquePtr

* Improve logging for commissioning

- Log when commissioning is complete (SendCommissioningCompleteCallbacks)
- Consolidate logging in AutoCommissioner::CommissioningStepFinished
- Also rely on VerifyOrDie in Variant instead of bug log message

* Another logging tweak

.github/workflows/lint.yml

@@ -203,7 +203,7 @@ jobs:
                   # TODO: TLVDebug should ideally not be excluded here.
                   # TODO: protocol_decoder.cpp should ideally not be excluded here.
                   # TODO: PersistentStorageMacros.h should ideally not be excluded here.
-                  git grep -I -n "PRI.64" -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)examples/chip-tool' ':(exclude)examples/tv-casting-app' ':(exclude)src/app/MessageDef/MessageDefHelper.cpp' ':(exclude)src/app/tests/integration/chip_im_initiator.cpp' ':(exclude)src/lib/core/TLVDebug.cpp' ':(exclude)src/lib/dnssd/tests/TestTxtFields.cpp' ':(exclude)src/lib/format/protocol_decoder.cpp' ':(exclude)src/lib/support/PersistentStorageMacros.h' ':(exclude)src/messaging/tests/echo/echo_requester.cpp' ':(exclude)src/platform/Linux' ':(exclude)src/platform/Ameba' ':(exclude)src/platform/ESP32' ':(exclude)src/platform/webos' ':(exclude)zzz_generated/chip-tool' ':(exclude)src/tools/chip-cert/Cmd_PrintCert.cpp' && exit 1 || exit 0
+                  git grep -I -n "PRI.64" -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)examples/chip-tool' ':(exclude)examples/tv-casting-app' ':(exclude)src/app/MessageDef/MessageDefHelper.cpp' ':(exclude)src/app/tests/integration/chip_im_initiator.cpp' ':(exclude)src/lib/core/TLVDebug.cpp' ':(exclude)src/lib/dnssd/tests/TestTxtFields.cpp' ':(exclude)src/lib/format/protocol_decoder.cpp' ':(exclude)src/lib/support/PersistentStorageMacros.h' ':(exclude)src/messaging/tests/echo/echo_requester.cpp' ':(exclude)src/platform/Linux' ':(exclude)src/platform/Ameba' ':(exclude)src/platform/ESP32' ':(exclude)src/platform/Darwin' ':(exclude)src/darwin' ':(exclude)src/platform/webos' ':(exclude)zzz_generated/chip-tool' ':(exclude)src/tools/chip-cert/Cmd_PrintCert.cpp' && exit 1 || exit 0
 
             # git grep exits with 0 if it finds a match, but we want
             # to fail (exit nonzero) on match.  And we want to exclude this file,

src/controller/AutoCommissioner.cpp

@@ -673,20 +673,10 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
 {
     CompletionStatus completionStatus;
     completionStatus.err = err;
-
-    if (err == CHIP_NO_ERROR)
-    {
-        ChipLogProgress(Controller, "Successfully finished commissioning step '%s'", StageToString(report.stageCompleted));
-    }
-    else
-    {
-        ChipLogProgress(Controller, "Error on commissioning step '%s': '%s'", StageToString(report.stageCompleted), err.AsString());
-    }
-
     if (err != CHIP_NO_ERROR)
     {
+        ChipLogError(Controller, "Error on commissioning step '%s': '%s'", StageToString(report.stageCompleted), err.AsString());
         completionStatus.failedStage = MakeOptional(report.stageCompleted);
-        ChipLogError(Controller, "Failed to perform commissioning step %d", static_cast<int>(report.stageCompleted));
         if (report.Is<AttestationErrorInfo>())
         {
             completionStatus.attestationResult = MakeOptional(report.Get<AttestationErrorInfo>().attestationResult);
@@ -727,19 +717,14 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
     }
     else
     {
+        ChipLogProgress(Controller, "Successfully finished commissioning step '%s'", StageToString(report.stageCompleted));
         switch (report.stageCompleted)
         {
         case CommissioningStage::kReadCommissioningInfo:
             break;
         case CommissioningStage::kReadCommissioningInfo2: {
-            if (!report.Is<ReadCommissioningInfo>())
-            {
-                ChipLogError(Controller,
-                             "[BUG] Should read commissioning info, but report is not ReadCommissioningInfo. THIS IS A BUG.");
-            }
-            ReadCommissioningInfo commissioningInfo = report.Get<ReadCommissioningInfo>();
-
             mDeviceCommissioningInfo = report.Get<ReadCommissioningInfo>();
+
             if (!mParams.GetFailsafeTimerSeconds().HasValue() && mDeviceCommissioningInfo.general.recommendedFailsafe > 0)
             {
                 mParams.SetFailsafeTimerSeconds(mDeviceCommissioningInfo.general.recommendedFailsafe);
@@ -751,11 +736,11 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
             // Don't send DST unless the device says it needs it
             mNeedsDST = false;
 
-            mParams.SetSupportsConcurrentConnection(commissioningInfo.supportsConcurrentConnection);
+            mParams.SetSupportsConcurrentConnection(mDeviceCommissioningInfo.supportsConcurrentConnection);
 
             if (mParams.GetCheckForMatchingFabric())
             {
-                chip::NodeId nodeId = commissioningInfo.remoteNodeId;
+                chip::NodeId nodeId = mDeviceCommissioningInfo.remoteNodeId;
                 if (nodeId != kUndefinedNodeId)
                 {
                     mParams.SetRemoteNodeId(nodeId);
@@ -764,7 +749,7 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
 
             if (mParams.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore)
             {
-                if (commissioningInfo.icd.isLIT && commissioningInfo.icd.checkInProtocolSupport)
+                if (mDeviceCommissioningInfo.icd.isLIT && mDeviceCommissioningInfo.icd.checkInProtocolSupport)
                 {
                     mNeedIcdRegistration = true;
                     ChipLogDetail(Controller, "AutoCommissioner: ICD supports the check-in protocol.");