Update Pigweed; switch tokenized logging to its own handler function (#…

…25351) * Add linker search directory for pw_tokenizer Specify a linker search directory (-L) for pw_tokenizer's linker script, since an upcoming Pigweed commit splits the linker script into two files. * Update pigweed repo This commit updates the Pigweed submodule to 73cac22f, which requires some changes. Pigweed's pw_tokenizer:global_handler_with_payload facade is deprecated. Each pw_tokenizer should define its own handler function instead. This gives each pw_tokenizer user more control over the handler function and allows multiple subsystems to use pw_tokenizer in whichever way works best. This change updates the tokenized version of the ChipInternalLogImpl macro to function as described at https://pigweed.dev/pw_tokenizer/#tokenize-a-message-with-arguments-in-a-custom-macro. * Update darwin logic for zap-cli moving --------- Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Jun 30, 2023 · 1020280 · 1020280
1 parent d3815cd
commit 1020280
Show file tree

Hide file tree

Showing 13 changed files with 45 additions and 17 deletions.
diff --git a/.github/workflows/darwin.yaml b/.github/workflows/darwin.yaml
@@ -68,7 +68,7 @@ jobs:
 
             - name: Block zap-cli from being used
               # xcodebuild is NOT expected to require zap-cli
-              run: scripts/run_in_build_env.sh 'mv $PW_ENVIRONMENT_ROOT/cipd/packages/zap/zap-cli $PW_ENVIRONMENT_ROOT/cipd/packages/zap/zap-cli.moved'
+              run: scripts/run_in_build_env.sh 'D=$(dirname $(which zap-cli)) && mv $D/zap-cli $D/zap-cli.moved'
             - name: Validate zap-cli is NOT available
               # run_in_build_env.sh is used to ensure PATH is set to something that would otherwise find zap-cli
               run: scripts/run_in_build_env.sh '(zap-cli --version && exit 1) || exit 0'
@@ -112,7 +112,7 @@ jobs:
               run: xcodebuild clean
               working-directory: src/darwin/Framework
             - name: Make zap-cli work again
-              run: scripts/run_in_build_env.sh 'mv $PW_ENVIRONMENT_ROOT/cipd/packages/zap/zap-cli.moved $PW_ENVIRONMENT_ROOT/cipd/packages/zap/zap-cli'
+              run: scripts/run_in_build_env.sh 'D=$(dirname $(which zap-cli.moved)) && mv $D/zap-cli.moved $D/zap-cli'
             - name: Validate zap-cli is again available
               run: scripts/run_in_build_env.sh 'zap-cli --version'
             - name: Build example All Clusters Server

diff --git a/config/mbed/CMakeLists.txt b/config/mbed/CMakeLists.txt
@@ -209,6 +209,10 @@ list(APPEND CHIP_DEFINES
     PW_RPC_USE_GLOBAL_MUTEX=0
 )
 
+# TODO: Update this to use target_link_libraries instead of
+# target_include_directories. target_include_directories never should be used to
+# access other libraries since it does not add source files to the build graph
+# and does not support transitive dependencies.
 target_include_directories(${APP_TARGET} PRIVATE
                     ${PIGWEED_ROOT}/pw_sys_io/public
                     ${PIGWEED_ROOT}/pw_assert/public
@@ -241,6 +245,7 @@ target_include_directories(${APP_TARGET} PRIVATE
                     ${PIGWEED_ROOT}/pw_function/public
                     ${PIGWEED_ROOT}/pw_preprocessor/public
                     ${PIGWEED_ROOT}/pw_rpc/system_server/public
+                    ${PIGWEED_ROOT}/pw_toolchain/public
                     ${PIGWEED_ROOT}/third_party/fuchsia/repo/sdk/lib/fit/include
                     ${PIGWEED_ROOT}/third_party/fuchsia/repo/sdk/lib/stdcompat/include
                     ${CHIP_ROOT}/third_party/nanopb/repo

diff --git a/examples/all-clusters-app/esp32/main/CMakeLists.txt b/examples/all-clusters-app/esp32/main/CMakeLists.txt
@@ -249,6 +249,7 @@ target_link_libraries(${COMPONENT_LIB} PUBLIC
 target_link_options(${COMPONENT_LIB}
   PUBLIC
     "-T${PIGWEED_ROOT}/pw_tokenizer/pw_tokenizer_linker_sections.ld"
+    "-L${PIGWEED_ROOT}/pw_tokenizer"
 )
 
 target_compile_options(${COMPONENT_LIB} PRIVATE

diff --git a/examples/all-clusters-minimal-app/esp32/main/CMakeLists.txt b/examples/all-clusters-minimal-app/esp32/main/CMakeLists.txt
@@ -244,6 +244,7 @@ target_link_libraries(${COMPONENT_LIB} PUBLIC
 target_link_options(${COMPONENT_LIB}
   PUBLIC
     "-T${PIGWEED_ROOT}/pw_tokenizer/pw_tokenizer_linker_sections.ld"
+    "-L${PIGWEED_ROOT}/pw_tokenizer"
 )
 
 target_compile_options(${COMPONENT_LIB} PRIVATE

diff --git a/examples/chef/esp32/main/CMakeLists.txt b/examples/chef/esp32/main/CMakeLists.txt
@@ -209,6 +209,7 @@ target_link_libraries(${COMPONENT_LIB} PUBLIC
 target_link_options(${COMPONENT_LIB}
   PUBLIC
     "-T${PIGWEED_ROOT}/pw_tokenizer/pw_tokenizer_linker_sections.ld"
+    "-L${PIGWEED_ROOT}/pw_tokenizer"
 )
 
 target_compile_options(${COMPONENT_LIB} PRIVATE

diff --git a/examples/lighting-app/esp32/main/CMakeLists.txt b/examples/lighting-app/esp32/main/CMakeLists.txt
@@ -220,6 +220,7 @@ target_link_libraries(${COMPONENT_LIB} PUBLIC
 target_link_options(${COMPONENT_LIB}
   PUBLIC
     "-T${PIGWEED_ROOT}/pw_tokenizer/pw_tokenizer_linker_sections.ld"
+    "-L${PIGWEED_ROOT}/pw_tokenizer"
 )
 
 target_compile_options(${COMPONENT_LIB} PRIVATE

diff --git a/examples/ota-requestor-app/esp32/main/CMakeLists.txt b/examples/ota-requestor-app/esp32/main/CMakeLists.txt
@@ -185,6 +185,7 @@ target_link_libraries(${COMPONENT_LIB} PUBLIC
 target_link_options(${COMPONENT_LIB}
   PUBLIC
     "-T${PIGWEED_ROOT}/pw_tokenizer/pw_tokenizer_linker_sections.ld"
+    "-L${PIGWEED_ROOT}/pw_tokenizer"
 )
 
 target_compile_options(${COMPONENT_LIB} PRIVATE

diff --git a/examples/temperature-measurement-app/esp32/main/CMakeLists.txt b/examples/temperature-measurement-app/esp32/main/CMakeLists.txt
@@ -176,6 +176,7 @@ target_link_libraries(${COMPONENT_LIB} PUBLIC
 target_link_options(${COMPONENT_LIB}
   PUBLIC
     "-T${PIGWEED_ROOT}/pw_tokenizer/pw_tokenizer_linker_sections.ld"
+    "-L${PIGWEED_ROOT}/pw_tokenizer"
 )
 
 target_compile_options(${COMPONENT_LIB} PRIVATE

diff --git a/examples/window-app/telink/CMakeLists.txt b/examples/window-app/telink/CMakeLists.txt
@@ -241,6 +241,7 @@ target_link_libraries(app PRIVATE
 target_link_options(app
   PUBLIC
     "-T${PIGWEED_ROOT}/pw_tokenizer/pw_tokenizer_linker_sections.ld"
+    "-L${PIGWEED_ROOT}/pw_tokenizer"
 )
 
 endif(CONFIG_CHIP_PW_RPC)
diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn
@@ -216,7 +216,7 @@ static_library("support") {
   }
 
   if (chip_pw_tokenizer_logging) {
-    public_deps += [ "${dir_pw_tokenizer}:global_handler_with_payload" ]
+    public_deps += [ "${dir_pw_tokenizer}" ]
   }
 
   if (chip_config_memory_debug_dmalloc) {

diff --git a/src/lib/support/logging/CHIPLogging.cpp b/src/lib/support/logging/CHIPLogging.cpp
@@ -38,33 +38,45 @@
 
 #include <atomic>
 
+#if CHIP_PW_TOKENIZER_LOGGING
+#include "pw_tokenizer/encode_args.h"
+#endif
+
+namespace chip {
+namespace Logging {
+
+#if _CHIP_USE_LOGGING
+
 #if CHIP_PW_TOKENIZER_LOGGING
 
-extern "C" void pw_tokenizer_HandleEncodedMessageWithPayload(uintptr_t levels, const uint8_t encoded_message[], size_t size_bytes)
+void HandleTokenizedLog(uint32_t levels, pw_tokenizer_Token token, pw_tokenizer_ArgTypes types, ...)
 {
+    uint8_t encoded_message[PW_TOKENIZER_CFG_ENCODING_BUFFER_SIZE_BYTES];
+
+    va_list args;
+    va_start(args, types);
+    // Use the C argument encoding API, since the C++ API requires C++17.
+    const size_t encoded_size = pw_tokenizer_EncodeArgs(types, args, encoded_message, sizeof(encoded_message));
+    va_end(args);
+
     uint8_t log_category = levels >> 8 & 0xFF;
     uint8_t log_module   = levels & 0xFF;
-    char * buffer        = (char *) chip::Platform::MemoryAlloc(2 * size_bytes + 1);
+    char * buffer        = (char *) chip::Platform::MemoryAlloc(2 * encoded_size + 1);
 
     if (buffer)
     {
-        for (int i = 0; i < size_bytes; i++)
+        for (int i = 0; i < encoded_size; i++)
         {
             sprintf(buffer + 2 * i, "%02x", encoded_message[i]);
         }
-        buffer[2 * size_bytes] = '\0';
-        chip::Logging::Log(log_module, log_category, "%s", buffer);
+        buffer[2 * encoded_size] = '\0';
+        Log(log_module, log_category, "%s", buffer);
         chip::Platform::MemoryFree(buffer);
     }
 }
 
 #endif
 
-namespace chip {
-namespace Logging {
-
-#if _CHIP_USE_LOGGING
-
 namespace {
 
 std::atomic<LogRedirectCallback_t> sLogRedirectCallback{ nullptr };

diff --git a/src/lib/support/logging/CHIPLogging.h b/src/lib/support/logging/CHIPLogging.h
@@ -50,7 +50,7 @@
 #endif
 
 #if CHIP_PW_TOKENIZER_LOGGING
-#include "pw_tokenizer/tokenize_to_global_handler_with_payload.h"
+#include "pw_tokenizer/tokenize.h"
 #endif
 
 /**
@@ -423,13 +423,17 @@ void LogV(uint8_t module, uint8_t category, const char * msg, va_list args) ENFO
 #endif // CHIP_SYSTEM_CONFIG_PLATFORM_LOG
 
 #if CHIP_PW_TOKENIZER_LOGGING
+
+void HandleTokenizedLog(uint32_t levels, pw_tokenizer_Token token, pw_tokenizer_ArgTypes, ...);
+
 #define ChipInternalLogImpl(MOD, CAT, MSG, ...)                                                                                    \
     do                                                                                                                             \
     {                                                                                                                              \
         if (chip::Logging::IsCategoryEnabled(CAT))                                                                                 \
         {                                                                                                                          \
-            PW_TOKENIZE_TO_GLOBAL_HANDLER_WITH_PAYLOAD((pw_tokenizer_Payload)((CAT << 8) | chip::Logging::kLogModule_##MOD), MSG,  \
-                                                       __VA_ARGS__);                                                               \
+            PW_TOKENIZE_FORMAT_STRING(PW_TOKENIZER_DEFAULT_DOMAIN, UINT32_MAX, MSG, __VA_ARGS__);                                  \
+            ::chip::Logging::HandleTokenizedLog((uint32_t)((CAT << 8) | chip::Logging::kLogModule_##MOD), _pw_tokenizer_token,     \
+                                                PW_TOKENIZER_ARG_TYPES(__VA_ARGS__) PW_COMMA_ARGS(__VA_ARGS__));                   \
         }                                                                                                                          \
     } while (0)
 #else // CHIP_PW_TOKENIZER_LOGGING

diff --git a/third_party/pigweed/repo b/third_party/pigweed/repo