Have chip-tool generate a random remote node id every time it pairs. (#…

…8450)

This way multiple things we pair don't step on each other's toes as
far as mdns advertisements go.

Fixes #8439

.github/workflows/tests.yaml

@@ -77,7 +77,8 @@ jobs:
             - name: Build chip-tool
               timeout-minutes: 5
               run: |
-                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ is_tsan=${USE_TSAN} config_use_separate_eventloop=${USE_SEPARATE_EVENTLOOP}
+                  # config_pair_with_random_id=false so CI runs faster.
+                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ is_tsan=${USE_TSAN} config_use_separate_eventloop=${USE_SEPARATE_EVENTLOOP} config_pair_with_random_id=false
             - name: Copy objdir
               run: |
                   # The idea is to not upload our objdir unless builds have
@@ -162,7 +163,8 @@ jobs:
             - name: Build chip-tool
               timeout-minutes: 5
               run: |
-                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ is_tsan=${USE_TSAN} config_use_separate_eventloop=${USE_SEPARATE_EVENTLOOP}
+                  # config_pair_with_random_id=false so CI runs faster.
+                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ is_tsan=${USE_TSAN} config_use_separate_eventloop=${USE_SEPARATE_EVENTLOOP} config_pair_with_random_id=false
             - name: Copy objdir
               run: |
                   # The idea is to not upload our objdir unless builds have

examples/chip-tool/BUILD.gn

@@ -22,6 +22,11 @@ assert(chip_build_tools)
 declare_args() {
   # Use a separate eventloop for CHIP tasks
   config_use_separate_eventloop = true
+
+  # Generate a new node id on every pairing.  This significantly slows
+  # down running a lot of pairings (because of all the disk traffic for
+  # saving the config file), so we disable it in some configurations.
+  config_pair_with_random_id = true
 }
 
 executable("chip-tool") {
@@ -40,7 +45,10 @@ executable("chip-tool") {
     "main.cpp",
   ]
 
-  defines = [ "CONFIG_USE_SEPARATE_EVENTLOOP=${config_use_separate_eventloop}" ]
+  defines = [
+    "CONFIG_USE_SEPARATE_EVENTLOOP=${config_use_separate_eventloop}",
+    "CONFIG_PAIR_WITH_RANDOM_ID=${config_pair_with_random_id}",
+  ]
 
   deps = [
     "${chip_root}/src/controller/data_model",

examples/chip-tool/commands/common/Commands.cpp

@@ -38,11 +38,13 @@ void Commands::Register(const char * clusterName, commands_list commandsList)
     }
 }
 
-int Commands::Run(NodeId localId, NodeId remoteId, int argc, char ** argv)
+int Commands::Run(int argc, char ** argv)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     chip::Controller::CommissionerInitParams initParams;
     Command * command = nullptr;
+    NodeId localId;
+    NodeId remoteId;
 
     err = chip::Platform::MemoryInit();
     VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Controller, "Init Memory failure: %s", chip::ErrorStr(err)));
@@ -56,6 +58,11 @@ int Commands::Run(NodeId localId, NodeId remoteId, int argc, char ** argv)
     VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Controller, "Init Storage failure: %s", chip::ErrorStr(err)));
 
     chip::Logging::SetLogFilter(mStorage.GetLoggingLevel());
+    localId  = mStorage.GetLocalNodeId();
+    remoteId = mStorage.GetRemoteNodeId();
+
+    ChipLogProgress(Controller, "Read local id 0x" ChipLogFormatX64 ", remote id 0x" ChipLogFormatX64, ChipLogValueX64(localId),
+                    ChipLogValueX64(remoteId));
 
     initParams.storageDelegate = &mStorage;
 

examples/chip-tool/commands/common/Commands.h

@@ -30,7 +30,7 @@ class Commands
     using CommandsVector = ::std::vector<std::unique_ptr<Command>>;
 
     void Register(const char * clusterName, commands_list commandsList);
-    int Run(NodeId localId, NodeId remoteId, int argc, char ** argv);
+    int Run(int argc, char ** argv);
 
 private:
     // *ranCommand will be set to the command we ran if we get as far as running

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

@@ -18,7 +18,9 @@
 
 #include "PairingCommand.h"
 #include "platform/PlatformManager.h"
+#include <crypto/CHIPCryptoPAL.h>
 #include <lib/core/CHIPSafeCasts.h>
+#include <support/logging/CHIPLogging.h>
 
 #include <setup_payload/ManualSetupPayloadParser.h>
 #include <setup_payload/QRCodeSetupPayloadParser.h>
@@ -35,6 +37,18 @@ CHIP_ERROR PairingCommand::Run()
     GetExecContext()->commissioner->RegisterDeviceAddressUpdateDelegate(this);
     GetExecContext()->commissioner->RegisterPairingDelegate(this);
 
+#if CONFIG_PAIR_WITH_RANDOM_ID
+    // Generate a random remote id so we don't end up reusing the same node id
+    // for different nodes.
+    NodeId randomId;
+    DRBG_get_bytes(reinterpret_cast<uint8_t *>(&randomId), sizeof(randomId));
+    ChipLogProgress(Controller, "Generated random node id: 0x" ChipLogFormatX64, ChipLogValueX64(randomId));
+    if (GetExecContext()->storage->SetRemoteNodeId(randomId) == CHIP_NO_ERROR)
+    {
+        GetExecContext()->remoteId = randomId;
+    }
+#endif // CONFIG_PAIR_WITH_RANDOM_ID
+
     err = RunInternal(GetExecContext()->remoteId);
     VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init Failure! PairDevice: %s", ErrorStr(err)));
 

examples/chip-tool/commands/reporting/Commands.h

@@ -60,54 +60,55 @@ class Listen : public ReportingCommand
     void AddReportCallbacks(uint8_t endpointId) override
     {
         chip::app::CHIPDeviceCallbacksMgr & callbacksMgr = chip::app::CHIPDeviceCallbacksMgr::GetInstance();
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x000F, 0x0055,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x000F, 0x0055,
                                        onReportBinaryInputBasicPresentValueCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x000F, 0x006F,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x000F, 0x006F,
                                        onReportBinaryInputBasicStatusFlagsCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0300, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0300, 0x0000,
                                        onReportColorControlCurrentHueCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0300, 0x0001,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0300, 0x0001,
                                        onReportColorControlCurrentSaturationCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0300, 0x0003,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0300, 0x0003,
                                        onReportColorControlCurrentXCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0300, 0x0004,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0300, 0x0004,
                                        onReportColorControlCurrentYCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0300, 0x0007,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0300, 0x0007,
                                        onReportColorControlColorTemperatureCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0101, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0101, 0x0000,
                                        onReportDoorLockLockStateCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0008, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0008, 0x0000,
                                        onReportLevelControlCurrentLevelCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0406, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0406, 0x0000,
                                        onReportOccupancySensingOccupancyCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0006, 0x0000, onReportOnOffOnOffCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0403, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0006, 0x0000,
+                                       onReportOnOffOnOffCallback->Cancel());
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0403, 0x0000,
                                        onReportPressureMeasurementMeasuredValueCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0200, 0x0013,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0200, 0x0013,
                                        onReportPumpConfigurationAndControlCapacityCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0405, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0405, 0x0000,
                                        onReportRelativeHumidityMeasurementMeasuredValueCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x003B, 0x0001,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x003B, 0x0001,
                                        onReportSwitchCurrentPositionCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0402, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0402, 0x0000,
                                        onReportTemperatureMeasurementMeasuredValueCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0201, 0x0000,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0201, 0x0000,
                                        onReportThermostatLocalTemperatureCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x0008,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x0008,
                                        onReportWindowCoveringCurrentPositionLiftPercentageCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x0009,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x0009,
                                        onReportWindowCoveringCurrentPositionTiltPercentageCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x000A,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x000A,
                                        onReportWindowCoveringOperationalStatusCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x000B,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x000B,
                                        onReportWindowCoveringTargetPositionLiftPercent100thsCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x000C,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x000C,
                                        onReportWindowCoveringTargetPositionTiltPercent100thsCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x000E,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x000E,
                                        onReportWindowCoveringCurrentPositionLiftPercent100thsCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x000F,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x000F,
                                        onReportWindowCoveringCurrentPositionTiltPercent100thsCallback->Cancel());
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, 0x0102, 0x001A,
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, 0x0102, 0x001A,
                                        onReportWindowCoveringSafetyStatusCallback->Cancel());
     }
 

examples/chip-tool/config/PersistentStorage.cpp

@@ -17,6 +17,8 @@
  */
 #include "PersistentStorage.h"
 
+#include <lib/core/CHIPEncoding.h>
+#include <protocols/secure_channel/PASESession.h>
 #include <support/Base64.h>
 
 #include <fstream>
@@ -34,6 +36,8 @@ constexpr const char kFilename[]           = "/tmp/chip_tool_config.ini";
 constexpr const char kDefaultSectionName[] = "Default";
 constexpr const char kPortKey[]            = "ListenPort";
 constexpr const char kLoggingKey[]         = "LoggingLevel";
+constexpr const char kLocalNodeIdKey[]     = "LocalNodeId";
+constexpr const char kRemoteNodeIdKey[]    = "RemoteNodeId";
 constexpr LogCategory kDefaultLoggingLevel = kLogCategory_Detail;
 
 namespace {
@@ -204,3 +208,44 @@ LogCategory PersistentStorage::GetLoggingLevel()
 
     return chipLogLevel;
 }
+
+NodeId PersistentStorage::GetNodeId(const char * key, NodeId defaultVal)
+{
+    CHIP_ERROR err = CHIP_NO_ERROR;
+
+    uint64_t nodeId;
+    uint16_t size = static_cast<uint16_t>(sizeof(nodeId));
+    err           = SyncGetKeyValue(key, &nodeId, size);
+    if (err == CHIP_NO_ERROR)
+    {
+        return static_cast<NodeId>(Encoding::LittleEndian::HostSwap64(nodeId));
+    }
+
+    return defaultVal;
+}
+
+NodeId PersistentStorage::GetLocalNodeId()
+{
+    return GetNodeId(kLocalNodeIdKey, kTestControllerNodeId);
+}
+
+NodeId PersistentStorage::GetRemoteNodeId()
+{
+    return GetNodeId(kRemoteNodeIdKey, kTestDeviceNodeId);
+}
+
+CHIP_ERROR PersistentStorage::SetNodeId(const char * key, NodeId value)
+{
+    uint64_t nodeId = Encoding::LittleEndian::HostSwap64(value);
+    return SyncSetKeyValue(key, &nodeId, sizeof(nodeId));
+}
+
+CHIP_ERROR PersistentStorage::SetLocalNodeId(NodeId nodeId)
+{
+    return SetNodeId(kLocalNodeIdKey, nodeId);
+}
+
+CHIP_ERROR PersistentStorage::SetRemoteNodeId(NodeId nodeId)
+{
+    return SetNodeId(kRemoteNodeIdKey, nodeId);
+}

examples/chip-tool/config/PersistentStorage.h

@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <app/util/basic-types.h>
 #include <controller/CHIPDeviceController.h>
 #include <inipp/inipp.h>
 #include <support/logging/CHIPLogging.h>
@@ -35,7 +36,19 @@ class PersistentStorage : public chip::PersistentStorageDelegate
     uint16_t GetListenPort();
     chip::Logging::LogCategory GetLoggingLevel();
 
+    // Return the stored node ids, or the default ones if nothing is stored.
+    chip::NodeId GetLocalNodeId();
+    chip::NodeId GetRemoteNodeId();
+
+    // Store node ids.
+    CHIP_ERROR SetLocalNodeId(chip::NodeId nodeId);
+    CHIP_ERROR SetRemoteNodeId(chip::NodeId nodeId);
+
 private:
+    // Helpers for node ids.
+    chip::NodeId GetNodeId(const char * key, chip::NodeId defaultVal);
+    CHIP_ERROR SetNodeId(const char * key, chip::NodeId value);
+
     CHIP_ERROR CommitConfig();
     inipp::Ini<char> mConfig;
 };

examples/chip-tool/main.cpp

@@ -25,8 +25,6 @@
 #include "commands/reporting/Commands.h"
 #include "commands/tests/Commands.h"
 
-#include <protocols/secure_channel/PASESession.h>
-
 // ================================================================================
 // Main Code
 // ================================================================================
@@ -40,5 +38,5 @@ int main(int argc, char * argv[])
     registerCommandsTests(commands);
     registerClusters(commands);
 
-    return commands.Run(chip::kTestControllerNodeId, chip::kTestDeviceNodeId, argc, argv);
+    return commands.Run(argc, argv);
 }

examples/chip-tool/templates/reporting-commands.zapt

@@ -31,7 +31,7 @@ public:
 {{#chip_clusters}}
 {{#chip_server_cluster_attributes}}
 {{#if isReportableAttribute}}
-        callbacksMgr.AddReportCallback(chip::kTestDeviceNodeId, endpointId, {{asHex parent.code 4}}, {{asHex code 4}}, onReport{{asCamelCased parent.name false}}{{asCamelCased name false}}Callback->Cancel());
+        callbacksMgr.AddReportCallback(GetExecContext()->storage->GetRemoteNodeId(), endpointId, {{asHex parent.code 4}}, {{asHex code 4}}, onReport{{asCamelCased parent.name false}}{{asCamelCased name false}}Callback->Cancel());
 {{/if}}
 {{/chip_server_cluster_attributes}}
 {{/chip_clusters}}