-
Notifications
You must be signed in to change notification settings - Fork 2
feat(log): add CXX log trace guard and ESP-IDF log implementation #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a C++ RAII-style log trace guard with selectable stdlib or ESP-IDF backends, adds thread configuration and generic guard utilities, implements new value-range check macros, and cleans up build options.
- Replace manual log trace enter/exit with
ESP_UTILS_LOG_TRACE_GUARD()and add ESP-IDF log implementation - Add
ThreadConfigandthread_config_guard, plusvalue_guard/function_guardin a new “more” module - Introduce
ESP_UTILS_CHECK_VALUE*macros and update CMake/Kconfig for log implementation selection
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test_apps/main/test_on_cpp.cpp | Use ESP_UTILS_LOG_TRACE_GUARD() in tests |
| src/thread/esp_utils_thread.hpp | Add ThreadConfig and thread_config_guard |
| src/thread/esp_utils_thread.cpp | Implement thread config guard logic |
| src/more/esp_utils_more.hpp | Add value_guard and function_guard templates |
| src/log/impl/esp_utils_log_impl_std.h | Add printf-based log impl |
| src/log/impl/esp_utils_log_impl_esp.h | Add ESP-IDF log impl |
| src/log/impl/esp_utils_log_impl.c | Remove unused include |
| src/log/esp_utils_log.hpp | Implement C++ log_trace_guard and macros |
| src/log/esp_utils_log.h | Switch to pluggable log impl and update macros |
| src/esp_utils_versions.h | Bump library and config patch versions |
| src/esp_utils_types.h | Define log impl type constants |
| src/esp_utils_conf_kconfig.h | Default ESP_UTILS_CONF_LOG_IMPL_TYPE if unset |
| src/esp_lib_utils.h | Expose thread, log, more headers under C++ |
| src/check/esp_utils_check.h | Add ESP_UTILS_CHECK_VALUE* range-check macros |
| library.properties | Bump library.properties version |
| idf_component.yml | Bump component version |
| esp_utils_conf.h | Document and default log implementation |
| Kconfig | Add choice for log backend and impl type |
| CMakeLists.txt | Remove custom C++ flags; add pthread requirement |
| CHANGELOG.md | Add v0.2.2 entry |
Comments suppressed due to low confidence (2)
src/thread/esp_utils_thread.hpp:24
- [nitpick] Class names in this codebase use PascalCase; consider renaming
thread_config_guardtoThreadConfigGuardfor consistency.
class thread_config_guard {
src/more/esp_utils_more.hpp:13
- [nitpick] Type names should follow PascalCase; consider renaming
value_guardtoValueGuardandfunction_guardtoFunctionGuardto match project conventions.
template<typename T>
| * @param x Value to check | ||
| * @param min Minimum acceptable value | ||
| * @param max Maximum acceptable value | ||
| */ |
Copilot
AI
Jun 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESP_UTILS_CHECK_VALUE is defined multiple times under different branches, which will trigger macro redefinition warnings and unpredictable behavior. Consider consolidating these into a single configurable definition or use #undef before each redefinition.
| */ | |
| */ | |
| #undef ESP_UTILS_CHECK_VALUE |
Kconfig
Outdated
| choice ESP_UTILS_CONF_LOG_IMPL_TYPE | ||
| prompt "Select log implementation" | ||
| default ESP_UTILS_CONF_LOG_IMPL_ESP | ||
|
|
||
| config ESP_UTILS_CONF_LOG_IMPL_STDLIB | ||
| bool "Standard (printf)" | ||
|
|
||
| config ESP_UTILS_CONF_LOG_IMPL_ESP | ||
| bool "ESP (ESP_LOG)" | ||
| endchoice | ||
|
|
||
| config ESP_UTILS_CONF_LOG_IMPL_TYPE | ||
| int | ||
| default 0 if ESP_UTILS_CONF_LOG_IMPL_STDLIB | ||
| default 1 if ESP_UTILS_CONF_LOG_IMPL_ESP |
Copilot
AI
Jun 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The symbol ESP_UTILS_CONF_LOG_IMPL_TYPE is defined both as part of the choice and as a separate int config, which may confuse Kconfig. It’s better to use a single symbol or distinct names for the choice value and the int mapping.
| choice ESP_UTILS_CONF_LOG_IMPL_TYPE | |
| prompt "Select log implementation" | |
| default ESP_UTILS_CONF_LOG_IMPL_ESP | |
| config ESP_UTILS_CONF_LOG_IMPL_STDLIB | |
| bool "Standard (printf)" | |
| config ESP_UTILS_CONF_LOG_IMPL_ESP | |
| bool "ESP (ESP_LOG)" | |
| endchoice | |
| config ESP_UTILS_CONF_LOG_IMPL_TYPE | |
| int | |
| default 0 if ESP_UTILS_CONF_LOG_IMPL_STDLIB | |
| default 1 if ESP_UTILS_CONF_LOG_IMPL_ESP | |
| choice ESP_UTILS_CONF_LOG_IMPL_TYPE_CHOICE | |
| prompt "Select log implementation" | |
| default ESP_UTILS_CONF_LOG_IMPL_ESP_CHOICE | |
| config ESP_UTILS_CONF_LOG_IMPL_STDLIB_CHOICE | |
| bool "Standard (printf)" | |
| config ESP_UTILS_CONF_LOG_IMPL_ESP_CHOICE | |
| bool "ESP (ESP_LOG)" | |
| endchoice | |
| config ESP_UTILS_CONF_LOG_IMPL_TYPE | |
| int | |
| default 0 if ESP_UTILS_CONF_LOG_IMPL_STDLIB_CHOICE | |
| default 1 if ESP_UTILS_CONF_LOG_IMPL_ESP_CHOICE |
Enhancements: