From 7c415e6d98cac69470e3d04cd8de216fb14ea55d Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 14 May 2024 12:42:59 -0700 Subject: [PATCH 1/3] Run interactive mode by default --- .../interactive/InteractiveCommands.cpp | 47 +++++++++++++++++-- examples/fabric-admin/main.cpp | 3 +- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp index 9ef07e79450300..adfd6dd7ab4a09 100644 --- a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp +++ b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp @@ -31,6 +31,27 @@ constexpr char kInteractiveModeStopCommand[] = "quit()"; namespace { +// File pointer for the log file +FILE * logFile = nullptr; + +void OpenLogFile(const char * filePath) +{ + logFile = fopen(filePath, "a"); + if (logFile == nullptr) + { + perror("Failed to open log file"); + } +} + +void CloseLogFile() +{ + if (logFile != nullptr) + { + fclose(logFile); + logFile = nullptr; + } +} + void ClearLine() { printf("\r\x1B[0J"); // Move cursor to the beginning of the line and clear from cursor to end of the screen @@ -38,9 +59,25 @@ void ClearLine() void ENFORCE_FORMAT(3, 0) LoggingCallback(const char * module, uint8_t category, const char * msg, va_list args) { - ClearLine(); - chip::Logging::Platform::LogV(module, category, msg, args); - ClearLine(); + struct timeval tv; + + // Should not fail per man page of gettimeofday(), but failed to get time is not a fatal error in log. The bad time value will + // indicate the error occurred during getting time. + gettimeofday(&tv, nullptr); + + FILE * outputStream = (logFile == nullptr) ? stdout : logFile; + // Lock outputStream, so a single log line will not be corrupted in case + // where multiple threads are using logging subsystem at the same time. + flockfile(outputStream); + + fprintf(outputStream, "[%llu.%06llu][%lld:%lld] CHIP:%s: ", static_cast(tv.tv_sec), + static_cast(tv.tv_usec), static_cast(syscall(SYS_getpid)), + static_cast(syscall(SYS_gettid)), module); + vfprintf(outputStream, msg, args); + fprintf(outputStream, "\n"); + fflush(outputStream); + + funlockfile(outputStream); } } // namespace @@ -90,6 +127,8 @@ CHIP_ERROR InteractiveStartCommand::RunCommand() { read_history(GetHistoryFilePath().c_str()); + OpenLogFile("/tmp/fabric_admin.log"); + // Logs needs to be redirected in order to refresh the screen appropriately when something // is dumped to stdout while the user is typing a command. chip::Logging::SetLogRedirectCallback(LoggingCallback); @@ -112,6 +151,8 @@ CHIP_ERROR InteractiveStartCommand::RunCommand() } SetCommandExitStatus(CHIP_NO_ERROR); + CloseLogFile(); + return CHIP_NO_ERROR; } diff --git a/examples/fabric-admin/main.cpp b/examples/fabric-admin/main.cpp index e517c67f6b403f..d3e774c6c9ba78 100644 --- a/examples/fabric-admin/main.cpp +++ b/examples/fabric-admin/main.cpp @@ -28,6 +28,7 @@ // ================================================================================ int main(int argc, char * argv[]) { + const char * args[] = { argv[0], "interactive", "start" }; ExampleCredentialIssuerCommands credIssuerCommands; Commands commands; @@ -36,5 +37,5 @@ int main(int argc, char * argv[]) registerClusters(commands, &credIssuerCommands); registerCommandsSubscriptions(commands, &credIssuerCommands); - return commands.Run(argc, argv); + return commands.Run(3, const_cast(args)); } From 0c54647cba602b5802779d2c7dd73cec899284fe Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 15 May 2024 12:43:28 -0700 Subject: [PATCH 2/3] Address review comments --- .../commands/common/CHIPCommand.h | 4 ++ .../interactive/InteractiveCommands.cpp | 53 ++++++++++--------- examples/fabric-admin/main.cpp | 26 ++++++++- 3 files changed, 56 insertions(+), 27 deletions(-) diff --git a/examples/fabric-admin/commands/common/CHIPCommand.h b/examples/fabric-admin/commands/common/CHIPCommand.h index 856a4daa48753d..af833f5f718652 100644 --- a/examples/fabric-admin/commands/common/CHIPCommand.h +++ b/examples/fabric-admin/commands/common/CHIPCommand.h @@ -67,6 +67,9 @@ class CHIPCommand : public Command CHIPCommand(const char * commandName, CredentialIssuerCommands * credIssuerCmds, const char * helpText = nullptr) : Command(commandName, helpText), mCredIssuerCmds(credIssuerCmds) { + AddArgument("log-file-path", &mLogFilePath, + "Path to the log file where the output is redirected. Can be absolute or relative to the current working " + "directory."); AddArgument("paa-trust-store-path", &mPaaTrustStorePath, "Path to directory holding PAA certificate information. Can be absolute or relative to the current working " "directory."); @@ -156,6 +159,7 @@ class CHIPCommand : public Command // identity without shutting it down or something in between... PersistentStorage mCommissionerStorage; #endif // CONFIG_USE_LOCAL_STORAGE + chip::Optional mLogFilePath; chip::PersistentStorageOperationalKeystore mOperationalKeystore; chip::Credentials::PersistentStorageOpCertStore mOpCertStore; static chip::Crypto::RawKeySessionKeystore sSessionKeystore; diff --git a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp index adfd6dd7ab4a09..a624ebdcc6fc81 100644 --- a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp +++ b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp @@ -19,9 +19,12 @@ #include "InteractiveCommands.h" #include +#include #include +#include +#include #include #include @@ -32,12 +35,12 @@ constexpr char kInteractiveModeStopCommand[] = "quit()"; namespace { // File pointer for the log file -FILE * logFile = nullptr; +FILE * sLogFile = nullptr; void OpenLogFile(const char * filePath) { - logFile = fopen(filePath, "a"); - if (logFile == nullptr) + sLogFile = fopen(filePath, "a"); + if (sLogFile == nullptr) { perror("Failed to open log file"); } @@ -45,10 +48,10 @@ void OpenLogFile(const char * filePath) void CloseLogFile() { - if (logFile != nullptr) + if (sLogFile != nullptr) { - fclose(logFile); - logFile = nullptr; + fclose(sLogFile); + sLogFile = nullptr; } } @@ -59,25 +62,23 @@ void ClearLine() void ENFORCE_FORMAT(3, 0) LoggingCallback(const char * module, uint8_t category, const char * msg, va_list args) { - struct timeval tv; + uint64_t timeMs = chip::System::SystemClock().GetMonotonicMilliseconds64().count(); + uint64_t seconds = timeMs / 1000; + uint64_t milliseconds = timeMs % 1000; - // Should not fail per man page of gettimeofday(), but failed to get time is not a fatal error in log. The bad time value will - // indicate the error occurred during getting time. - gettimeofday(&tv, nullptr); + flockfile(sLogFile); - FILE * outputStream = (logFile == nullptr) ? stdout : logFile; - // Lock outputStream, so a single log line will not be corrupted in case - // where multiple threads are using logging subsystem at the same time. - flockfile(outputStream); + // Portable thread and process identifiers + auto pid = static_cast(getpid()); + auto tid = static_cast(pthread_self()); - fprintf(outputStream, "[%llu.%06llu][%lld:%lld] CHIP:%s: ", static_cast(tv.tv_sec), - static_cast(tv.tv_usec), static_cast(syscall(SYS_getpid)), - static_cast(syscall(SYS_gettid)), module); - vfprintf(outputStream, msg, args); - fprintf(outputStream, "\n"); - fflush(outputStream); + fprintf(sLogFile, "[%llu.%06llu][%lu:%lu] CHIP:%s: ", static_cast(seconds), + static_cast(milliseconds), pid, tid, module); + vfprintf(sLogFile, msg, args); + fprintf(sLogFile, "\n"); + fflush(sLogFile); - funlockfile(outputStream); + funlockfile(sLogFile); } } // namespace @@ -127,11 +128,13 @@ CHIP_ERROR InteractiveStartCommand::RunCommand() { read_history(GetHistoryFilePath().c_str()); - OpenLogFile("/tmp/fabric_admin.log"); + if (mLogFilePath.HasValue()) + { + OpenLogFile(mLogFilePath.Value()); - // Logs needs to be redirected in order to refresh the screen appropriately when something - // is dumped to stdout while the user is typing a command. - chip::Logging::SetLogRedirectCallback(LoggingCallback); + // Redirect logs to the custom logging callback + chip::Logging::SetLogRedirectCallback(LoggingCallback); + } char * command = nullptr; int status; diff --git a/examples/fabric-admin/main.cpp b/examples/fabric-admin/main.cpp index d3e774c6c9ba78..194edfd3e5d6b7 100644 --- a/examples/fabric-admin/main.cpp +++ b/examples/fabric-admin/main.cpp @@ -23,12 +23,34 @@ #include "commands/pairing/Commands.h" #include +#include +#include +#include + // ================================================================================ // Main Code // ================================================================================ int main(int argc, char * argv[]) { - const char * args[] = { argv[0], "interactive", "start" }; + // Convert command line arguments to a vector of strings for easier manipulation + std::vector args(argv, argv + argc); + + // Check if "interactive" and "start" are not in the arguments + if (args.size() < 3 || args[1] != "interactive" || args[2] != "start") + { + // Insert "interactive" and "start" after the executable name + args.insert(args.begin() + 1, "interactive"); + args.insert(args.begin() + 2, "start"); + + // Update argc and argv to reflect the new arguments + argc = static_cast(args.size()); + for (size_t i = 0; i < args.size(); ++i) + { + argv[i] = const_cast(args[i].c_str()); + } + } + + // const char * args[] = { argv[0], "interactive", "start" }; ExampleCredentialIssuerCommands credIssuerCommands; Commands commands; @@ -37,5 +59,5 @@ int main(int argc, char * argv[]) registerClusters(commands, &credIssuerCommands); registerCommandsSubscriptions(commands, &credIssuerCommands); - return commands.Run(3, const_cast(args)); + return commands.Run(argc, argv); } From b662d53f8ea6d1bf06038c37c50f8e331cfd8346 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 15 May 2024 19:01:53 -0700 Subject: [PATCH 3/3] allocate make a std::vector and pass .data() of that vector to commands.Run() --- .../commands/interactive/InteractiveCommands.cpp | 13 +++++++------ examples/fabric-admin/main.cpp | 16 +++++++--------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp index a624ebdcc6fc81..aaf3c36461cffb 100644 --- a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp +++ b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp @@ -62,18 +62,19 @@ void ClearLine() void ENFORCE_FORMAT(3, 0) LoggingCallback(const char * module, uint8_t category, const char * msg, va_list args) { + if (sLogFile == nullptr) + { + return; + } + uint64_t timeMs = chip::System::SystemClock().GetMonotonicMilliseconds64().count(); uint64_t seconds = timeMs / 1000; uint64_t milliseconds = timeMs % 1000; flockfile(sLogFile); - // Portable thread and process identifiers - auto pid = static_cast(getpid()); - auto tid = static_cast(pthread_self()); - - fprintf(sLogFile, "[%llu.%06llu][%lu:%lu] CHIP:%s: ", static_cast(seconds), - static_cast(milliseconds), pid, tid, module); + fprintf(sLogFile, "[%llu.%06llu] CHIP:%s: ", static_cast(seconds), + static_cast(milliseconds), module); vfprintf(sLogFile, msg, args); fprintf(sLogFile, "\n"); fflush(sLogFile); diff --git a/examples/fabric-admin/main.cpp b/examples/fabric-admin/main.cpp index 194edfd3e5d6b7..cf1122bda1ec8a 100644 --- a/examples/fabric-admin/main.cpp +++ b/examples/fabric-admin/main.cpp @@ -41,16 +41,8 @@ int main(int argc, char * argv[]) // Insert "interactive" and "start" after the executable name args.insert(args.begin() + 1, "interactive"); args.insert(args.begin() + 2, "start"); - - // Update argc and argv to reflect the new arguments - argc = static_cast(args.size()); - for (size_t i = 0; i < args.size(); ++i) - { - argv[i] = const_cast(args[i].c_str()); - } } - // const char * args[] = { argv[0], "interactive", "start" }; ExampleCredentialIssuerCommands credIssuerCommands; Commands commands; @@ -59,5 +51,11 @@ int main(int argc, char * argv[]) registerClusters(commands, &credIssuerCommands); registerCommandsSubscriptions(commands, &credIssuerCommands); - return commands.Run(argc, argv); + std::vector c_args; + for (auto & arg : args) + { + c_args.push_back(const_cast(arg.c_str())); + } + + return commands.Run(static_cast(c_args.size()), c_args.data()); }