From ccf0d1b79e2e1e3cf496031d1844a828e0386f5b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 13 Nov 2023 18:33:33 -0500 Subject: [PATCH] Fix operational advertising in chip-tool. chip-tool was never setting enableServerInteractions on the commissioner params, and it happened to work as long as we unconditionally advertised everything as long as server interactions were enabled on the factory, even if specific commissioners did not want to enable server interactions. Now that this is properly controller per-commissioner, we need to actually set the boolean we should have been setting all along. --- examples/chip-tool/commands/common/CHIPCommand.cpp | 1 + examples/chip-tool/commands/common/CHIPCommand.h | 2 +- examples/chip-tool/commands/common/Command.h | 11 ++++++++--- examples/chip-tool/commands/common/Commands.cpp | 8 ++++---- examples/chip-tool/commands/common/Commands.h | 5 +++-- .../commands/interactive/InteractiveCommands.cpp | 2 +- .../commands/interactive/InteractiveCommands.mm | 2 +- 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index 944360fb2eea3a..fc8295e00e887b 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/examples/chip-tool/commands/common/CHIPCommand.cpp @@ -460,6 +460,7 @@ CHIP_ERROR CHIPCommand::InitializeCommissioner(CommissionerIdentity & identity, commissionerParams.controllerICAC = icacSpan; commissionerParams.controllerNOC = nocSpan; commissionerParams.permitMultiControllerFabrics = true; + commissionerParams.enableServerInteractions = NeedsOperationalAdvertising(); } // TODO: Initialize IPK epoch key in ExampleOperationalCredentials issuer rather than relying on DefaultIpkValue diff --git a/examples/chip-tool/commands/common/CHIPCommand.h b/examples/chip-tool/commands/common/CHIPCommand.h index f1cd84439eae49..0b6124600eb415 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.h +++ b/examples/chip-tool/commands/common/CHIPCommand.h @@ -142,7 +142,7 @@ class CHIPCommand : public Command // If true, the controller will be created with server capabilities enabled, // such as advertising operational nodes over DNS-SD and accepting incoming // CASE sessions. - virtual bool NeedsOperationalAdvertising() { return false; } + virtual bool NeedsOperationalAdvertising() { return mAdvertiseOperational; } // Execute any deferred cleanups. Used when exiting interactive mode. static void ExecuteDeferredCleanups(intptr_t ignored); diff --git a/examples/chip-tool/commands/common/Command.h b/examples/chip-tool/commands/common/Command.h index cf8077b13c0546..26b80e54b6b322 100644 --- a/examples/chip-tool/commands/common/Command.h +++ b/examples/chip-tool/commands/common/Command.h @@ -267,10 +267,11 @@ class Command bool IsInteractive() { return mIsInteractive; } - CHIP_ERROR RunAsInteractive(const chip::Optional & interactiveStorageDirectory) + CHIP_ERROR RunAsInteractive(const chip::Optional & interactiveStorageDirectory, bool advertiseOperational) { - mStorageDirectory = interactiveStorageDirectory; - mIsInteractive = true; + mStorageDirectory = interactiveStorageDirectory; + mIsInteractive = true; + mAdvertiseOperational = advertiseOperational; return Run(); } @@ -280,6 +281,10 @@ class Command // mStorageDirectory lives here so we can just set it in RunAsInteractive. chip::Optional mStorageDirectory; + // mAdvertiseOperational lives here so we can just set it in + // RunAsInteractive; it's only used by CHIPCommand. + bool mAdvertiseOperational = false; + private: bool InitArgument(size_t argIndex, char * argValue); size_t AddArgument(const char * name, int64_t min, uint64_t max, void * out, ArgumentType type, const char * desc, diff --git a/examples/chip-tool/commands/common/Commands.cpp b/examples/chip-tool/commands/common/Commands.cpp index 17871e12cff2e6..8212656b9539ba 100644 --- a/examples/chip-tool/commands/common/Commands.cpp +++ b/examples/chip-tool/commands/common/Commands.cpp @@ -182,7 +182,7 @@ int Commands::Run(int argc, char ** argv) return (err == CHIP_NO_ERROR) ? EXIT_SUCCESS : EXIT_FAILURE; } -int Commands::RunInteractive(const char * command, const chip::Optional & storageDirectory) +int Commands::RunInteractive(const char * command, const chip::Optional & storageDirectory, bool advertiseOperational) { std::vector arguments; VerifyOrReturnValue(DecodeArgumentsFromInteractiveMode(command, arguments), EXIT_FAILURE); @@ -207,7 +207,7 @@ int Commands::RunInteractive(const char * command, const chip::Optional } ChipLogProgress(chipTool, "Command: %s", commandStr.c_str()); - auto err = RunCommand(argc, argv, true, storageDirectory); + auto err = RunCommand(argc, argv, true, storageDirectory, advertiseOperational); // Do not delete arg[0] for (auto i = 1; i < argc; i++) @@ -219,7 +219,7 @@ int Commands::RunInteractive(const char * command, const chip::Optional } CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive, - const chip::Optional & interactiveStorageDirectory) + const chip::Optional & interactiveStorageDirectory, bool interactiveAdvertiseOperational) { Command * command = nullptr; @@ -307,7 +307,7 @@ CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive, if (interactive) { - return command->RunAsInteractive(interactiveStorageDirectory); + return command->RunAsInteractive(interactiveStorageDirectory, interactiveAdvertiseOperational); } // Now that the command is initialized, get our storage from it as needed diff --git a/examples/chip-tool/commands/common/Commands.h b/examples/chip-tool/commands/common/Commands.h index 67c23b401c4147..bb6c62dcb035fe 100644 --- a/examples/chip-tool/commands/common/Commands.h +++ b/examples/chip-tool/commands/common/Commands.h @@ -42,7 +42,7 @@ class Commands Register(commandSetName, commandsList, helpText, false); } int Run(int argc, char ** argv); - int RunInteractive(const char * command, const chip::Optional & storageDirectory = chip::NullOptional); + int RunInteractive(const char * command, const chip::Optional & storageDirectory, bool advertiseOperational); private: struct CommandSet @@ -56,7 +56,8 @@ class Commands using CommandSetMap = std::map; CHIP_ERROR RunCommand(int argc, char ** argv, bool interactive = false, - const chip::Optional & interactiveStorageDirectory = chip::NullOptional); + const chip::Optional & interactiveStorageDirectory = chip::NullOptional, + bool interactiveAdvertiseOperational = false); CommandSetMap::iterator GetCommandSet(std::string commandSetName); Command * GetCommand(CommandsVector & commands, std::string commandName); diff --git a/examples/chip-tool/commands/interactive/InteractiveCommands.cpp b/examples/chip-tool/commands/interactive/InteractiveCommands.cpp index 6dae09ce392206..40b4431df116a6 100644 --- a/examples/chip-tool/commands/interactive/InteractiveCommands.cpp +++ b/examples/chip-tool/commands/interactive/InteractiveCommands.cpp @@ -369,7 +369,7 @@ bool InteractiveCommand::ParseCommand(char * command, int * status) ClearLine(); - *status = mHandler->RunInteractive(command, GetStorageDirectory()); + *status = mHandler->RunInteractive(command, GetStorageDirectory(), NeedsOperationalAdvertising()); return true; } diff --git a/examples/darwin-framework-tool/commands/interactive/InteractiveCommands.mm b/examples/darwin-framework-tool/commands/interactive/InteractiveCommands.mm index 6fe3522dcd909b..3242b5fa2c9022 100644 --- a/examples/darwin-framework-tool/commands/interactive/InteractiveCommands.mm +++ b/examples/darwin-framework-tool/commands/interactive/InteractiveCommands.mm @@ -380,7 +380,7 @@ el_status_t StopFunction() ClearLine(); - *status = mHandler->RunInteractive(command, GetStorageDirectory()); + *status = mHandler->RunInteractive(command, GetStorageDirectory(), true); return YES; }