From 0fc3e957b4383805671a24e418721d3c22554e08 Mon Sep 17 00:00:00 2001 From: Wyatt Hepler <255@users.noreply.github.com> Date: Fri, 7 Apr 2023 13:27:50 -0700 Subject: [PATCH] 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 --- .github/workflows/darwin.yaml | 4 +-- config/mbed/CMakeLists.txt | 5 +++ .../esp32/main/CMakeLists.txt | 1 + .../esp32/main/CMakeLists.txt | 1 + examples/chef/esp32/main/CMakeLists.txt | 1 + .../lighting-app/esp32/main/CMakeLists.txt | 1 + .../esp32/main/CMakeLists.txt | 1 + .../esp32/main/CMakeLists.txt | 1 + examples/window-app/telink/CMakeLists.txt | 1 + src/lib/support/BUILD.gn | 2 +- src/lib/support/logging/CHIPLogging.cpp | 32 +++++++++++++------ src/lib/support/logging/CHIPLogging.h | 10 ++++-- third_party/pigweed/repo | 2 +- 13 files changed, 45 insertions(+), 17 deletions(-) diff --git a/.github/workflows/darwin.yaml b/.github/workflows/darwin.yaml index 40ec4df82cc88d..2a7c3642919323 100644 --- 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 index 2a0eb84dda52b3..a7006062f12ace 100644 --- 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 index 144dbd4892af33..4b2f6a464e2933 100644 --- 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 index e2ed8315f4223f..9587e26f048d7d 100644 --- 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 index 0099a8547c78c2..e1940af232fd8d 100644 --- 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 index 9a7f8c26a16a34..6856a3582349dc 100644 --- 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 index 011d4a95c4d570..6a3633f442ac64 100644 --- 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 index 73636f69b2a9e9..6136d1583709bb 100644 --- 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 index 73d2a710037b54..ad23d3201a455b 100644 --- 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 index d006b42bf1b38e..724e029cbbc379 100644 --- 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 index ca04f60388d6b8..27b93e802f7991 100644 --- a/src/lib/support/logging/CHIPLogging.cpp +++ b/src/lib/support/logging/CHIPLogging.cpp @@ -38,33 +38,45 @@ #include +#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 sLogRedirectCallback{ nullptr }; diff --git a/src/lib/support/logging/CHIPLogging.h b/src/lib/support/logging/CHIPLogging.h index 5bb01ff1ed5c6b..3d0dd96d2f5f60 100644 --- 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 index 71ba37c8a6cc65..73cac22f49069a 160000 --- a/third_party/pigweed/repo +++ b/third_party/pigweed/repo @@ -1 +1 @@ -Subproject commit 71ba37c8a6cc65c8084078fb000e8f470cff8597 +Subproject commit 73cac22f49069a18fbec3bb60cb5f762b32ce20b