Add a chip-tool command line argument for the storage directory. (#26452

)

* Add a chip-tool command line argument for the storage directory.

1) Adds --storage-directory argument for chip-tool.
2) Changes our CI test script to create temp directory for the storage, so
   running that script locally does not interfere with existing chip-tool
   configuration.

* Make sure we don't pass the new storage args to darwin-framework-tool.

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

@@ -93,7 +93,7 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
     ReturnLogErrorOnFailure(chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(mBleAdapterId.ValueOr(0), true));
 #endif
 
-    ReturnLogErrorOnFailure(mDefaultStorage.Init());
+    ReturnLogErrorOnFailure(mDefaultStorage.Init(nullptr, GetStorageDirectory().ValueOr(nullptr)));
     ReturnLogErrorOnFailure(mOperationalKeystore.Init(&mDefaultStorage));
     ReturnLogErrorOnFailure(mOpCertStore.Init(&mDefaultStorage));
 
@@ -326,7 +326,7 @@ CHIP_ERROR CHIPCommand::GetIdentityNodeId(std::string identity, chip::NodeId * n
         return CHIP_NO_ERROR;
     }
 
-    ReturnLogErrorOnFailure(mCommissionerStorage.Init(identity.c_str()));
+    ReturnLogErrorOnFailure(mCommissionerStorage.Init(identity.c_str(), GetStorageDirectory().ValueOr(nullptr)));
 
     *nodeId = mCommissionerStorage.GetLocalNodeId();
 
@@ -419,7 +419,7 @@ CHIP_ERROR CHIPCommand::InitializeCommissioner(CommissionerIdentity & identity,
         // TODO - OpCreds should only be generated for pairing command
         //        store the credentials in persistent storage, and
         //        generate when not available in the storage.
-        ReturnLogErrorOnFailure(mCommissionerStorage.Init(identity.mName.c_str()));
+        ReturnLogErrorOnFailure(mCommissionerStorage.Init(identity.mName.c_str(), GetStorageDirectory().ValueOr(nullptr)));
         if (mUseMaxSizedCerts.HasValue())
         {
             auto option = CredentialIssuerCommands::CredentialIssuerOptions::kMaximizeCertificateSizes;

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

@@ -86,6 +86,8 @@ class CHIPCommand : public Command
         AddArgument("trace_decode", 0, 1, &mTraceDecode);
 #endif // CHIP_CONFIG_TRANSPORT_TRACE_ENABLED
         AddArgument("ble-adapter", 0, UINT16_MAX, &mBleAdapterId);
+        AddArgument("storage-directory", &mStorageDirectory,
+                    "Directory to place chip-tool's storage files in.  Defaults to $TMPDIR, with fallback to /tmp");
     }
 
     /////////// Command Interface /////////

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

@@ -260,12 +260,19 @@ class Command
 
     bool IsInteractive() { return mIsInteractive; }
 
-    CHIP_ERROR RunAsInteractive()
+    CHIP_ERROR RunAsInteractive(const chip::Optional<char *> & interactiveStorageDirectory)
     {
-        mIsInteractive = true;
+        mStorageDirectory = interactiveStorageDirectory;
+        mIsInteractive    = true;
         return Run();
     }
 
+    const chip::Optional<char *> & GetStorageDirectory() const { return mStorageDirectory; }
+
+protected:
+    // mStorageDirectory lives here so we can just set it in RunAsInteractive.
+    chip::Optional<char *> mStorageDirectory;
+
 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,

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

@@ -149,7 +149,7 @@ int Commands::Run(int argc, char ** argv)
     return (err == CHIP_NO_ERROR) ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
-int Commands::RunInteractive(const char * command)
+int Commands::RunInteractive(const char * command, const chip::Optional<char *> & storageDirectory)
 {
     std::vector<std::string> arguments;
     VerifyOrReturnValue(DecodeArgumentsFromInteractiveMode(command, arguments), EXIT_FAILURE);
@@ -174,7 +174,7 @@ int Commands::RunInteractive(const char * command)
     }
 
     ChipLogProgress(chipTool, "Command: %s", commandStr.c_str());
-    auto err = RunCommand(argc, argv, true);
+    auto err = RunCommand(argc, argv, true, storageDirectory);
 
     // Do not delete arg[0]
     for (auto i = 1; i < argc; i++)
@@ -185,7 +185,8 @@ int Commands::RunInteractive(const char * command)
     return (err == CHIP_NO_ERROR) ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
-CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive)
+CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive,
+                                const chip::Optional<char *> & interactiveStorageDirectory)
 {
     Command * command = nullptr;
 
@@ -271,7 +272,25 @@ CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive)
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    return interactive ? command->RunAsInteractive() : command->Run();
+    if (interactive)
+    {
+        return command->RunAsInteractive(interactiveStorageDirectory);
+    }
+
+    // Now that the command is initialized, get our storage from it as needed
+    // and set up our loging level.
+#ifdef CONFIG_USE_LOCAL_STORAGE
+    CHIP_ERROR err = mStorage.Init(nullptr, command->GetStorageDirectory().ValueOr(nullptr));
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(Controller, "Init Storage failure: %s", chip::ErrorStr(err));
+        return err;
+    }
+
+    chip::Logging::SetLogFilter(mStorage.GetLoggingLevel());
+#endif // CONFIG_USE_LOCAL_STORAGE
+
+    return command->Run();
 }
 
 Commands::ClusterMap::iterator Commands::GetCluster(std::string clusterName)

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

@@ -32,12 +32,13 @@ class Commands
 
     void Register(const char * clusterName, commands_list commandsList, const char * helpText = nullptr);
     int Run(int argc, char ** argv);
-    int RunInteractive(const char * command);
+    int RunInteractive(const char * command, const chip::Optional<char *> & storageDirectory = chip::NullOptional);
 
 private:
     using ClusterMap = std::map<std::string, std::pair<CommandsVector, const char *>>;
 
-    CHIP_ERROR RunCommand(int argc, char ** argv, bool interactive = false);
+    CHIP_ERROR RunCommand(int argc, char ** argv, bool interactive = false,
+                          const chip::Optional<char *> & interactiveStorageDirectory = chip::NullOptional);
 
     ClusterMap::iterator GetCluster(std::string clusterName);
     Command * GetCommand(CommandsVector & commands, std::string commandName);

examples/chip-tool/commands/interactive/InteractiveCommands.cpp

@@ -319,7 +319,7 @@ bool InteractiveCommand::ParseCommand(char * command, int * status)
 
     ClearLine();
 
-    *status = mHandler->RunInteractive(command);
+    *status = mHandler->RunInteractive(command, GetStorageDirectory());
 
     return true;
 }

scripts/tests/chiptest/test_definition.py

@@ -15,6 +15,8 @@
 
 import logging
 import os
+import shutil
+import tempfile
 import threading
 import time
 import typing
@@ -227,6 +229,7 @@ class TestRunTime(Enum):
     CHIP_TOOL_BUILTIN = auto()  # run via chip-tool built-in test commands
     CHIP_TOOL_PYTHON = auto()  # use the python yaml test parser with chip-tool
     CHIP_REPL_PYTHON = auto()       # use the python yaml test runner
+    DARWIN_FRAMEWORK_TOOL_BUILTIN = auto()  # run via darwin-framework-tool built-in test commands
 
 
 @dataclass
@@ -259,6 +262,8 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str,
         """
         runner.capture_delegate = ExecutionCapture()
 
+        tool_storage_dir = None
+
         try:
             if self.target == TestTarget.ALL_CLUSTERS:
                 target_app = paths.all_clusters_app
@@ -302,25 +307,22 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str,
 
             tool_cmd = paths.chip_tool if test_runtime != TestRunTime.CHIP_TOOL_PYTHON else paths.chip_tool_with_python_cmd
 
-            files_to_unlink = [
-                '/tmp/chip_tool_config.ini',
-                '/tmp/chip_tool_config.alpha.ini',
-                '/tmp/chip_tool_config.beta.ini',
-                '/tmp/chip_tool_config.gamma.ini',
-            ]
-
-            for f in files_to_unlink:
-                if os.path.exists(f):
-                    os.unlink(f)
+            tool_storage_dir = tempfile.mkdtemp()
+            tool_storage_args = ['--storage-directory', tool_storage_dir]
 
             # Only start and pair the default app
             app = apps_register.get('default')
             app.start()
             pairing_cmd = tool_cmd + ['pairing', 'code', TEST_NODE_ID, app.setupCode]
             test_cmd = tool_cmd + ['tests', self.run_name] + ['--PICS', pics_file]
             if test_runtime == TestRunTime.CHIP_TOOL_PYTHON:
-                pairing_cmd += ['--server_path'] + [paths.chip_tool[-1]]
-                test_cmd += ['--server_path'] + [paths.chip_tool[-1]]
+                server_args = ['--server_path', paths.chip_tool[-1]] + \
+                    ['--server_arguments', 'interactive server ' + ' '.join(tool_storage_args)]
+                pairing_cmd += server_args
+                test_cmd += server_args
+            elif test_runtime == TestRunTime.CHIP_TOOL_BUILTIN:
+                pairing_cmd += tool_storage_args
+                test_cmd += tool_storage_args
 
             if dry_run:
                 logging.info(" ".join(pairing_cmd))
@@ -348,3 +350,5 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str,
             apps_register.killAll()
             apps_register.factoryResetAll()
             apps_register.removeAll()
+            if tool_storage_dir is not None:
+                shutil.rmtree(tool_storage_dir, ignore_errors=True)

scripts/tests/run_test_suite.py

@@ -143,6 +143,8 @@ def main(context, dry_run, log_level, target, target_glob, target_skip_glob,
         runtime = TestRunTime.CHIP_REPL_PYTHON
     elif runner == 'chip_tool_python':
         runtime = TestRunTime.CHIP_TOOL_PYTHON
+    elif chip_tool is not None and os.path.basename(chip_tool) == "darwin-framework-tool":
+        runtime = TestRunTime.DARWIN_FRAMEWORK_TOOL_BUILTIN
 
     if chip_tool is None and not runtime == TestRunTime.CHIP_REPL_PYTHON:
         # non yaml tests REQUIRE chip-tool. Yaml tests should not require chip-tool

src/controller/ExamplePersistentStorage.cpp

@@ -39,37 +39,43 @@ constexpr const char kLocalNodeIdKey[]      = "LocalNodeId";
 constexpr const char kCommissionerCATsKey[] = "CommissionerCATs";
 constexpr LogCategory kDefaultLoggingLevel  = kLogCategory_Automation;
 
-std::string GetFilename(const char * name)
+std::string GetFilename(const char * directory, const char * name)
 {
-    const char * tmpdir = getenv("TMPDIR");
+    const char * dir = directory;
 
-    if (tmpdir == nullptr)
+    if (dir == nullptr)
     {
-        tmpdir = "/tmp";
+        dir = getenv("TMPDIR");
+    }
+
+    if (dir == nullptr)
+    {
+        dir = "/tmp";
     }
 
     if (name == nullptr)
     {
-        return std::string(tmpdir) + "/chip_tool_config.ini";
+        return std::string(dir) + "/chip_tool_config.ini";
     }
 
-    return std::string(tmpdir) + "/chip_tool_config." + std::string(name) + ".ini";
+    return std::string(dir) + "/chip_tool_config." + std::string(name) + ".ini";
 }
 
-CHIP_ERROR PersistentStorage::Init(const char * name)
+CHIP_ERROR PersistentStorage::Init(const char * name, const char * directory)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
 
     std::ifstream ifs;
-    ifs.open(GetFilename(name), std::ifstream::in);
+    ifs.open(GetFilename(directory, name), std::ifstream::in);
     if (!ifs.good())
     {
-        CommitConfig(name);
-        ifs.open(GetFilename(name), std::ifstream::in);
+        CommitConfig(directory, name);
+        ifs.open(GetFilename(directory, name), std::ifstream::in);
     }
     VerifyOrExit(ifs.is_open(), err = CHIP_ERROR_OPEN_FAILED);
 
-    mName = name;
+    mName      = name;
+    mDirectory = directory;
     mConfig.parse(ifs);
     ifs.close();
 
@@ -125,7 +131,7 @@ CHIP_ERROR PersistentStorage::SyncSetKeyValue(const char * key, const void * val
     }
 
     mConfig.sections[kDefaultSectionName] = section;
-    return CommitConfig(mName);
+    return CommitConfig(mDirectory, mName);
 }
 
 CHIP_ERROR PersistentStorage::SyncDeleteKeyValue(const char * key)
@@ -138,7 +144,7 @@ CHIP_ERROR PersistentStorage::SyncDeleteKeyValue(const char * key)
     section.erase(escapedKey);
 
     mConfig.sections[kDefaultSectionName] = section;
-    return CommitConfig(mName);
+    return CommitConfig(mDirectory, mName);
 }
 
 bool PersistentStorage::SyncDoesKeyExist(const char * key)
@@ -173,23 +179,23 @@ CHIP_ERROR PersistentStorage::SyncClearAll()
     auto section = mConfig.sections[kDefaultSectionName];
     section.clear();
     mConfig.sections[kDefaultSectionName] = section;
-    return CommitConfig(mName);
+    return CommitConfig(mDirectory, mName);
 }
 
-CHIP_ERROR PersistentStorage::CommitConfig(const char * name)
+CHIP_ERROR PersistentStorage::CommitConfig(const char * directory, const char * name)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
 
     std::ofstream ofs;
-    std::string tmpPath = GetFilename(name) + ".tmp";
+    std::string tmpPath = GetFilename(directory, name) + ".tmp";
     ofs.open(tmpPath, std::ofstream::out | std::ofstream::trunc);
     VerifyOrExit(ofs.good(), err = CHIP_ERROR_WRITE_FAILED);
 
     mConfig.generate(ofs);
     ofs.close();
     VerifyOrExit(ofs.good(), err = CHIP_ERROR_WRITE_FAILED);
 
-    VerifyOrExit(rename(tmpPath.c_str(), GetFilename(name).c_str()) == 0, err = CHIP_ERROR_WRITE_FAILED);
+    VerifyOrExit(rename(tmpPath.c_str(), GetFilename(directory, name).c_str()) == 0, err = CHIP_ERROR_WRITE_FAILED);
 
 exit:
     return err;

src/controller/ExamplePersistentStorage.h

@@ -26,7 +26,18 @@
 class PersistentStorage : public chip::PersistentStorageDelegate
 {
 public:
-    CHIP_ERROR Init(const char * name = nullptr);
+    /**
+     * name is the name of the storage to use.  If null, defaults to
+     * "chip_tool_config.ini".
+     *
+     * directory is the directory the storage file should be placed in.  If
+     * null, falls back to getenv("TMPDIR") and if that is not set falls back
+     * to /tmp.
+     *
+     * If non-null values are provided, the memory they point to is expected to
+     * outlive this object.
+     */
+    CHIP_ERROR Init(const char * name = nullptr, const char * directory = nullptr);
 
     /////////// PersistentStorageDelegate Interface /////////
     CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) override;
@@ -55,7 +66,8 @@ class PersistentStorage : public chip::PersistentStorageDelegate
     CHIP_ERROR SyncClearAll();
 
 private:
-    CHIP_ERROR CommitConfig(const char * name);
+    CHIP_ERROR CommitConfig(const char * directory, const char * name);
     inipp::Ini<char> mConfig;
     const char * mName;
+    const char * mDirectory;
 };