Improve chip-tool help for "synthetic" clusters.

1. Makes it clear which things are actual clusters and which are command sets. 2. Makes sure all command sets have overall help text that describes what the commands in the set do. 3. Adds some help text to some specific commands.
project-chip · Jul 6, 2023 · 474344c · 474344c
1 parent 0d1f9c3
commit 474344c
Show file tree

Hide file tree

Showing 27 changed files with 294 additions and 244 deletions.
diff --git a/examples/chip-tool/commands/clusters/SubscriptionsCommands.h b/examples/chip-tool/commands/clusters/SubscriptionsCommands.h
@@ -104,5 +104,5 @@ void registerCommandsSubscriptions(Commands & commands, CredentialIssuerCommands
         make_unique<ShutdownAllSubscriptions>(credsIssuerConfig),     //
     };
 
-    commands.Register(clusterName, clusterCommands, "Commands for shutting down subscriptions.");
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for shutting down subscriptions.");
 }
diff --git a/examples/chip-tool/commands/common/Commands.cpp b/examples/chip-tool/commands/common/Commands.cpp
@@ -119,12 +119,13 @@ static void DetectAndLogMismatchedDoubleQuotes(int argc, char ** argv)
 
 } // namespace
 
-void Commands::Register(const char * clusterName, commands_list commandsList, const char * helpText)
+void Commands::Register(const char * clusterName, commands_list commandsList, const char * helpText, bool isSynthetic)
 {
-    mClusters[clusterName].second = helpText;
+    mClusters[clusterName].isSynthetic = isSynthetic;
+    mClusters[clusterName].helpText    = helpText;
     for (auto & command : commandsList)
     {
-        mClusters[clusterName].first.push_back(std::move(command));
+        mClusters[clusterName].commands.push_back(std::move(command));
     }
 }
 
@@ -192,21 +193,21 @@ CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive,
 
     if (argc <= 1)
     {
-        ChipLogError(chipTool, "Missing cluster name");
+        ChipLogError(chipTool, "Missing cluster or command set name");
         ShowClusters(argv[0]);
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
     auto clusterIter = GetCluster(argv[1]);
     if (clusterIter == mClusters.end())
     {
-        ChipLogError(chipTool, "Unknown cluster: %s", argv[1]);
+        ChipLogError(chipTool, "Unknown cluster or command set: %s", argv[1]);
         ShowClusters(argv[0]);
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    auto & commandList = clusterIter->second.first;
-    auto * clusterHelp = clusterIter->second.second;
+    auto & commandList = clusterIter->second.commands;
+    auto * clusterHelp = clusterIter->second.helpText;
 
     if (argc <= 2)
     {
@@ -350,21 +351,41 @@ bool Commands::IsGlobalCommand(std::string commandName) const
     return IsAttributeCommand(commandName) || IsEventCommand(commandName);
 }
 
+void Commands::ShowClusterOverview(std::string clusterName, const ClusterData & clusterData)
+{
+    std::transform(clusterName.begin(), clusterName.end(), clusterName.begin(), [](unsigned char c) { return std::tolower(c); });
+    fprintf(stderr, "  | * %-82s|\n", clusterName.c_str());
+    ShowHelpText(clusterData.helpText);
+}
+
 void Commands::ShowClusters(std::string executable)
 {
     fprintf(stderr, "Usage:\n");
     fprintf(stderr, "  %s cluster_name command_name [param1 param2 ...]\n", executable.c_str());
+    fprintf(stderr, "or:\n");
+    fprintf(stderr, "  %s command_set_name command_name [param1 param2 ...]\n", executable.c_str());
     fprintf(stderr, "\n");
     fprintf(stderr, "  +-------------------------------------------------------------------------------------+\n");
     fprintf(stderr, "  | Clusters:                                                                           |\n");
     fprintf(stderr, "  +-------------------------------------------------------------------------------------+\n");
     for (auto & cluster : mClusters)
     {
-        std::string clusterName(cluster.first);
-        std::transform(clusterName.begin(), clusterName.end(), clusterName.begin(),
-                       [](unsigned char c) { return std::tolower(c); });
-        fprintf(stderr, "  | * %-82s|\n", clusterName.c_str());
-        ShowHelpText(cluster.second.second);
+        if (!cluster.second.isSynthetic)
+        {
+            ShowClusterOverview(cluster.first, cluster.second);
+        }
+    }
+    fprintf(stderr, "  +-------------------------------------------------------------------------------------+\n");
+    fprintf(stderr, "\n");
+    fprintf(stderr, "  +-------------------------------------------------------------------------------------+\n");
+    fprintf(stderr, "  | Command sets:                                                                       |\n");
+    fprintf(stderr, "  +-------------------------------------------------------------------------------------+\n");
+    for (auto & cluster : mClusters)
+    {
+        if (cluster.second.isSynthetic)
+        {
+            ShowClusterOverview(cluster.first, cluster.second);
+        }
     }
     fprintf(stderr, "  +-------------------------------------------------------------------------------------+\n");
 }
@@ -558,7 +579,7 @@ bool Commands::DecodeArgumentsFromBase64EncodedJson(const char * json, std::vect
     VerifyOrReturnValue(clusterIter != mClusters.end(), false,
                         ChipLogError(chipTool, "Cluster '%s' is not supported.", clusterName.c_str()));
 
-    auto & commandList = clusterIter->second.first;
+    auto & commandList = clusterIter->second.commands;
 
     auto command = GetCommand(commandList, commandName);
 

diff --git a/examples/chip-tool/commands/common/Commands.h b/examples/chip-tool/commands/common/Commands.h
@@ -30,12 +30,30 @@ class Commands
 public:
     using CommandsVector = ::std::vector<std::unique_ptr<Command>>;
 
-    void Register(const char * clusterName, commands_list commandsList, const char * helpText = nullptr);
+    void RegisterCluster(const char * clusterName, commands_list commandsList)
+    {
+        Register(clusterName, commandsList, nullptr, false);
+    }
+    // Command sets represent chip-tool functionality that is not actually
+    // XML-defined clusters.  All command sets should have help text explaining
+    // what sort of commands one should expect to find in the set.
+    void RegisterCommandSet(const char * clusterName, commands_list commandsList, const char * helpText)
+    {
+        Register(clusterName, commandsList, helpText, true);
+    }
     int Run(int argc, char ** argv);
     int RunInteractive(const char * command, const chip::Optional<char *> & storageDirectory = chip::NullOptional);
 
 private:
-    using ClusterMap = std::map<std::string, std::pair<CommandsVector, const char *>>;
+    struct ClusterData
+    {
+        CommandsVector commands;
+        bool isSynthetic      = false;
+        const char * helpText = nullptr;
+    };
+    // The tuple contains the commands, whether it's a synthetic cluster, and
+    // the help text for the cluster (which may be null).
+    using ClusterMap = std::map<std::string, ClusterData>;
 
     CHIP_ERROR RunCommand(int argc, char ** argv, bool interactive = false,
                           const chip::Optional<char *> & interactiveStorageDirectory = chip::NullOptional);
@@ -48,6 +66,7 @@ class Commands
     bool IsGlobalCommand(std::string commandName) const;
 
     void ShowClusters(std::string executable);
+    static void ShowClusterOverview(std::string clusterName, const ClusterData & clusterData);
     void ShowCluster(std::string executable, std::string clusterName, CommandsVector & commands, const char * helpText);
     void ShowClusterAttributes(std::string executable, std::string clusterName, std::string commandName, CommandsVector & commands);
     void ShowClusterEvents(std::string executable, std::string clusterName, std::string commandName, CommandsVector & commands);
@@ -60,6 +79,8 @@ class Commands
     // helpText may be null, in which case it's not shown.
     static void ShowHelpText(const char * helpText);
 
+    void Register(const char * clusterName, commands_list commandsList, const char * helpText, bool isSynthetic);
+
     ClusterMap mClusters;
 #ifdef CONFIG_USE_LOCAL_STORAGE
     PersistentStorage mStorage;

diff --git a/examples/chip-tool/commands/delay/Commands.h b/examples/chip-tool/commands/delay/Commands.h
@@ -30,5 +30,5 @@ void registerCommandsDelay(Commands & commands, CredentialIssuerCommands * creds
         make_unique<WaitForCommissioneeCommand>(credsIssuerConfig), //
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for waiting for something to happen.");
 }
diff --git a/examples/chip-tool/commands/delay/WaitForCommissioneeCommand.h b/examples/chip-tool/commands/delay/WaitForCommissioneeCommand.h
@@ -26,8 +26,8 @@ class WaitForCommissioneeCommand : public CHIPCommand
 {
 public:
     WaitForCommissioneeCommand(CredentialIssuerCommands * credIssuerCommands) :
-        CHIPCommand("wait-for-commissionee", credIssuerCommands), mOnDeviceConnectedCallback(OnDeviceConnectedFn, this),
-        mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this)
+        CHIPCommand("wait-for-commissionee", credIssuerCommands, "Establish a CASE session to the provided node id."),
+        mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this)
     {
         AddArgument("nodeId", 0, UINT64_MAX, &mNodeId);
         AddArgument("expire-existing-session", 0, 1, &mExpireExistingSession);

diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h
@@ -85,5 +85,5 @@ void registerCommandsDiscover(Commands & commands, CredentialIssuerCommands * cr
         make_unique<DiscoverCommissionersCommand>(credsIssuerConfig),
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for device discovery.");
 }
diff --git a/examples/chip-tool/commands/group/Commands.h b/examples/chip-tool/commands/group/Commands.h
@@ -361,5 +361,6 @@ void registerCommandsGroup(Commands & commands, CredentialIssuerCommands * creds
         make_unique<RemoveKeySet>(credsIssuerConfig),
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands,
+                                "Commands for manipulating group keys and memberships for chip-tool itself.");
 }
diff --git a/examples/chip-tool/commands/interactive/Commands.h b/examples/chip-tool/commands/interactive/Commands.h
@@ -33,5 +33,5 @@ void registerCommandsInteractive(Commands & commands, CredentialIssuerCommands *
 #endif // CONFIG_USE_INTERACTIVE_MODE
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for starting long-lived interactive modes.");
 }
diff --git a/examples/chip-tool/commands/interactive/InteractiveCommands.h b/examples/chip-tool/commands/interactive/InteractiveCommands.h
@@ -29,8 +29,10 @@ class Commands;
 class InteractiveCommand : public CHIPCommand
 {
 public:
-    InteractiveCommand(const char * name, Commands * commandsHandler, CredentialIssuerCommands * credsIssuerConfig) :
-        CHIPCommand(name, credsIssuerConfig), mHandler(commandsHandler)
+    InteractiveCommand(const char * name, Commands * commandsHandler, const char * helpText,
+                       CredentialIssuerCommands * credsIssuerConfig) :
+        CHIPCommand(name, credsIssuerConfig, helpText),
+        mHandler(commandsHandler)
     {
         AddArgument("advertise-operational", 0, 1, &mAdvertiseOperational,
                     "Advertise operational node over DNS-SD and accept incoming CASE sessions.");
@@ -51,7 +53,8 @@ class InteractiveStartCommand : public InteractiveCommand
 {
 public:
     InteractiveStartCommand(Commands * commandsHandler, CredentialIssuerCommands * credsIssuerConfig) :
-        InteractiveCommand("start", commandsHandler, credsIssuerConfig)
+        InteractiveCommand("start", commandsHandler, "Start an interactive shell that can then run other commands.",
+                           credsIssuerConfig)
     {}
 
     /////////// CHIPCommand Interface /////////
@@ -62,7 +65,8 @@ class InteractiveServerCommand : public InteractiveCommand, public WebSocketServ
 {
 public:
     InteractiveServerCommand(Commands * commandsHandler, CredentialIssuerCommands * credsIssuerConfig) :
-        InteractiveCommand("server", commandsHandler, credsIssuerConfig)
+        InteractiveCommand("server", commandsHandler, "Start a websocket server that can receive commands sent by another process.",
+                           credsIssuerConfig)
     {
         AddArgument("port", 0, UINT16_MAX, &mPort, "Port the websocket will listen to. Defaults to 9002.");
     }

diff --git a/examples/chip-tool/commands/pairing/Commands.h b/examples/chip-tool/commands/pairing/Commands.h
@@ -255,5 +255,5 @@ void registerCommandsPairing(Commands & commands, CredentialIssuerCommands * cre
         make_unique<IssueNOCChainCommand>(credsIssuerConfig),
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for commissioning devices.");
 }
diff --git a/examples/chip-tool/commands/payload/Commands.h b/examples/chip-tool/commands/payload/Commands.h
@@ -36,5 +36,5 @@ void registerCommandsPayload(Commands & commands)
         make_unique<SetupPayloadVerhoeffGenerate>(),          //
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for parsing and generating setup payloads.");
 }
diff --git a/examples/chip-tool/commands/session-management/Commands.h b/examples/chip-tool/commands/session-management/Commands.h
@@ -30,5 +30,5 @@ void registerCommandsSessionManagement(Commands & commands, CredentialIssuerComm
         make_unique<EvictLocalCASESessionsCommand>(credsIssuerConfig),
     };
 
-    commands.Register(clusterName, clusterCommands, "Commands for managing CASE and PASE session state");
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for managing CASE and PASE session state.");
 }
diff --git a/examples/chip-tool/commands/storage/Commands.h b/examples/chip-tool/commands/storage/Commands.h
@@ -27,5 +27,5 @@ void registerCommandsStorage(Commands & commands)
 
     commands_list clusterCommands = { make_unique<StorageClearAll>() };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for managing persistent data stored by chip-tool.");
 }
diff --git a/examples/chip-tool/templates/commands.zapt b/examples/chip-tool/templates/commands.zapt
@@ -117,7 +117,7 @@ void registerCluster{{asUpperCamelCase name}}(Commands & commands, CredentialIss
         {{/zcl_events}}
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCluster(clusterName, clusterCommands);
 }
 {{/zcl_clusters}}
 
@@ -136,7 +136,7 @@ void registerClusterAny(Commands & commands, CredentialIssuerCommands * credsIss
         make_unique<SubscribeAll>(credsIssuerConfig),         //
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for sending IM messages based on cluster id, not cluster name.");
 }
 
 void registerClusters(Commands & commands, CredentialIssuerCommands * credsIssuerConfig)

diff --git a/examples/chip-tool/templates/tests/commands.zapt b/examples/chip-tool/templates/tests/commands.zapt
@@ -58,5 +58,5 @@ void registerCommandsTests(Commands & commands, CredentialIssuerCommands * creds
 #endif // CONFIG_ENABLE_YAML_TESTS
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for running YAML tests.");
 }
diff --git a/examples/darwin-framework-tool/commands/discover/Commands.h b/examples/darwin-framework-tool/commands/discover/Commands.h
@@ -32,5 +32,5 @@ void registerCommandsDiscover(Commands & commands)
         make_unique<DiscoverCommissionablesListCommand>(),
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for device discovery.");
 }
diff --git a/examples/darwin-framework-tool/commands/interactive/Commands.h b/examples/darwin-framework-tool/commands/interactive/Commands.h
@@ -35,5 +35,5 @@ void registerCommandsInteractive(Commands & commands)
 #endif // CONFIG_USE_INTERACTIVE_MODE
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for starting long-lived interactive modes.");
 }
diff --git a/examples/darwin-framework-tool/commands/pairing/Commands.h b/examples/darwin-framework-tool/commands/pairing/Commands.h
@@ -102,5 +102,5 @@ void registerCommandsPairing(Commands & commands)
         make_unique<GetCommissionerNodeIdCommand>(),
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for commissioning devices.");
 }
diff --git a/examples/darwin-framework-tool/commands/payload/Commands.h b/examples/darwin-framework-tool/commands/payload/Commands.h
@@ -27,5 +27,5 @@ void registerCommandsPayload(Commands & commands)
         make_unique<SetupPayloadParseCommand>(), //
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for parsing and generating setup payloads.");
 }
diff --git a/examples/darwin-framework-tool/commands/provider/Commands.h b/examples/darwin-framework-tool/commands/provider/Commands.h
@@ -10,5 +10,5 @@ void registerClusterOtaSoftwareUpdateProviderInteractive(Commands & commands)
         make_unique<OTASoftwareUpdateSetParams>(),   //
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Command for configuring darwin-framework-tool as an OTA provider.");
 }
diff --git a/examples/darwin-framework-tool/commands/storage/Commands.h b/examples/darwin-framework-tool/commands/storage/Commands.h
@@ -27,5 +27,6 @@ void registerCommandsStorage(Commands & commands)
 
     commands_list clusterCommands = { make_unique<StorageClearAll>() };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands,
+                                "Commands for managing persistent data stored by darwin-framework-tool.");
 }
diff --git a/examples/darwin-framework-tool/templates/commands.zapt b/examples/darwin-framework-tool/templates/commands.zapt
@@ -304,7 +304,7 @@ void registerCluster{{asUpperCamelCase name}}(Commands & commands)
         {{/zcl_events}}
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCluster(clusterName, clusterCommands);
 }
 {{/if}}
 {{/zcl_clusters}}
@@ -323,7 +323,7 @@ void registerClusterAny(Commands & commands)
         make_unique<SubscribeEvent>(),     //
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for sending IM messages based on cluster id, not cluster name.");
 }
 
 void registerClusters(Commands & commands)

diff --git a/examples/darwin-framework-tool/templates/tests/commands.zapt b/examples/darwin-framework-tool/templates/tests/commands.zapt
@@ -59,5 +59,5 @@ void registerCommandsTests(Commands & commands)
 #endif // CONFIG_ENABLE_YAML_TESTS
     };
 
-    commands.Register(clusterName, clusterCommands);
+    commands.RegisterCommandSet(clusterName, clusterCommands, "Commands for running YAML tests.");
 }