Skip to content

Commit

Permalink
Fix operational advertising in chip-tool. (#30457)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Feb 6, 2024
1 parent 029252d commit 1226264
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 12 deletions.
1 change: 1 addition & 0 deletions examples/chip-tool/commands/common/CHIPCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/common/CHIPCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 8 additions & 3 deletions examples/chip-tool/commands/common/Command.h
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,11 @@ class Command

bool IsInteractive() { return mIsInteractive; }

CHIP_ERROR RunAsInteractive(const chip::Optional<char *> & interactiveStorageDirectory)
CHIP_ERROR RunAsInteractive(const chip::Optional<char *> & interactiveStorageDirectory, bool advertiseOperational)
{
mStorageDirectory = interactiveStorageDirectory;
mIsInteractive = true;
mStorageDirectory = interactiveStorageDirectory;
mIsInteractive = true;
mAdvertiseOperational = advertiseOperational;
return Run();
}

Expand All @@ -280,6 +281,10 @@ class Command
// mStorageDirectory lives here so we can just set it in RunAsInteractive.
chip::Optional<char *> 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,
Expand Down
8 changes: 4 additions & 4 deletions examples/chip-tool/commands/common/Commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<char *> & storageDirectory)
int Commands::RunInteractive(const char * command, const chip::Optional<char *> & storageDirectory, bool advertiseOperational)
{
std::vector<std::string> arguments;
VerifyOrReturnValue(DecodeArgumentsFromInteractiveMode(command, arguments), EXIT_FAILURE);
Expand All @@ -207,7 +207,7 @@ int Commands::RunInteractive(const char * command, const chip::Optional<char *>
}

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++)
Expand All @@ -219,7 +219,7 @@ int Commands::RunInteractive(const char * command, const chip::Optional<char *>
}

CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive,
const chip::Optional<char *> & interactiveStorageDirectory)
const chip::Optional<char *> & interactiveStorageDirectory, bool interactiveAdvertiseOperational)
{
Command * command = nullptr;

Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions examples/chip-tool/commands/common/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<char *> & storageDirectory = chip::NullOptional);
int RunInteractive(const char * command, const chip::Optional<char *> & storageDirectory, bool advertiseOperational);

private:
struct CommandSet
Expand All @@ -56,7 +56,8 @@ class Commands
using CommandSetMap = std::map<std::string, CommandSet>;

CHIP_ERROR RunCommand(int argc, char ** argv, bool interactive = false,
const chip::Optional<char *> & interactiveStorageDirectory = chip::NullOptional);
const chip::Optional<char *> & interactiveStorageDirectory = chip::NullOptional,
bool interactiveAdvertiseOperational = false);

CommandSetMap::iterator GetCommandSet(std::string commandSetName);
Command * GetCommand(CommandsVector & commands, std::string commandName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ el_status_t StopFunction()

ClearLine();

*status = mHandler->RunInteractive(command, GetStorageDirectory());
*status = mHandler->RunInteractive(command, GetStorageDirectory(), true);

return YES;
}

0 comments on commit 1226264

Please sign in to comment.