Fix discriminator handling in chip-tool pairing commands. (project-chip#34115)

Not all commands initialize the discriminator value, and the ones that don't should not use it.

Fixes project-chip#34096

Author: bzbarsky-apple

examples/chip-tool/commands/pairing/PairingCommand.cpp

@@ -216,7 +216,12 @@ CHIP_ERROR PairingCommand::PairWithCode(NodeId remoteId)
 
 CHIP_ERROR PairingCommand::Pair(NodeId remoteId, PeerAddress address)
 {
-    auto params = RendezvousParameters().SetSetupPINCode(mSetupPINCode).SetDiscriminator(mDiscriminator).SetPeerAddress(address);
+    VerifyOrDieWithMsg(mSetupPINCode.has_value(), chipTool, "Using mSetupPINCode in a mode when we have not gotten one");
+    auto params = RendezvousParameters().SetSetupPINCode(mSetupPINCode.value()).SetPeerAddress(address);
+    if (mDiscriminator.has_value())
+    {
+        params.SetDiscriminator(mDiscriminator.value());
+    }
 
     CHIP_ERROR err = CHIP_NO_ERROR;
     if (mPaseOnly.ValueOr(false))
@@ -236,9 +241,11 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndex(NodeId remoteId, uint16_t in
 #if CHIP_DEVICE_LAYER_TARGET_DARWIN
     VerifyOrReturnError(IsInteractive(), CHIP_ERROR_INCORRECT_STATE);
 
+    VerifyOrDieWithMsg(mSetupPINCode.has_value(), chipTool, "Using mSetupPINCode in a mode when we have not gotten one");
+
     RendezvousParameters params;
     ReturnErrorOnFailure(GetDeviceScanner().Get(index, params));
-    params.SetSetupPINCode(mSetupPINCode);
+    params.SetSetupPINCode(mSetupPINCode.value());
 
     CHIP_ERROR err = CHIP_NO_ERROR;
     if (mPaseOnly.ValueOr(false))
@@ -258,6 +265,10 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndex(NodeId remoteId, uint16_t in
 
 CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndexWithCode(NodeId remoteId, uint16_t index)
 {
+    // We might or might not have a setup code.  We don't know yet, but if we
+    // do, we'll emplace it at that point.
+    mSetupPINCode.reset();
+
 #if CHIP_DEVICE_LAYER_TARGET_DARWIN
     VerifyOrReturnError(IsInteractive(), CHIP_ERROR_INCORRECT_STATE);
 
@@ -281,7 +292,7 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndexWithCode(NodeId remoteId, uin
             VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT);
         }
 
-        mSetupPINCode = payload.setUpPINCode;
+        mSetupPINCode.emplace(payload.setUpPINCode);
         return PairWithMdnsOrBleByIndex(remoteId, index);
     }
 

examples/chip-tool/commands/pairing/PairingCommand.h

@@ -26,6 +26,8 @@
 #include <lib/support/Span.h>
 #include <lib/support/ThreadOperationalDataset.h>
 
+#include <optional>
+
 enum class PairingMode
 {
     None,
@@ -107,32 +109,32 @@ class PairingCommand : public CHIPCommand,
             break;
         case PairingMode::Ble:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
-            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
-            AddArgument("discriminator", 0, 4096, &mDiscriminator);
+            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
+            AddArgument("discriminator", 0, 4096, &mDiscriminator.emplace());
             break;
         case PairingMode::OnNetwork:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
-            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
+            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
             AddArgument("pase-only", 0, 1, &mPaseOnly);
             break;
         case PairingMode::SoftAP:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
-            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
-            AddArgument("discriminator", 0, 4096, &mDiscriminator);
+            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
+            AddArgument("discriminator", 0, 4096, &mDiscriminator.emplace());
             AddArgument("device-remote-ip", &mRemoteAddr);
             AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
             AddArgument("pase-only", 0, 1, &mPaseOnly);
             break;
         case PairingMode::AlreadyDiscovered:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
-            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
+            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
             AddArgument("device-remote-ip", &mRemoteAddr);
             AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
             AddArgument("pase-only", 0, 1, &mPaseOnly);
             break;
         case PairingMode::AlreadyDiscoveredByIndex:
             AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
-            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
+            AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
             AddArgument("index", 0, UINT16_MAX, &mIndex);
             AddArgument("pase-only", 0, 1, &mPaseOnly);
             break;
@@ -252,8 +254,14 @@ class PairingCommand : public CHIPCommand,
         mComplex_DSTOffsets;
 
     uint16_t mRemotePort;
-    uint16_t mDiscriminator;
-    uint32_t mSetupPINCode;
+    // mDiscriminator is only used for some situations, but in those situations
+    // it's mandatory.  Track whether we're actually using it; the cases that do
+    // will emplace this optional.
+    std::optional<uint16_t> mDiscriminator;
+    // mSetupPINCode is only used for some situations, but in those situations
+    // it's mandatory.  Track whether we're actually using it; the cases that do
+    // will emplace this optional.
+    std::optional<uint32_t> mSetupPINCode;
     uint16_t mIndex;
     chip::ByteSpan mOperationalDataset;
     chip::ByteSpan mSSID;