Develop/nn#5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds LVGL demo support and modifies the logging utility API. The changes include adding multiple LVGL demo files (widgets, stress, music, keypad_encoder, benchmark), updating CMakeLists.txt to enable demos, and refactoring the logging utility to use better naming conventions.
Changes:
- Added LVGL demo files including widgets, stress, music, keypad_encoder, and benchmark demos
- Updated CMakeLists.txt to enable LVGL demos and link them properly
- Refactored logging utility to use single underscore prefix instead of double underscores (better C standard compliance)
Reviewed changes
Copilot reviewed 32 out of 82 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| L3_Middlewares/LVGL/demos/* | New LVGL demo files (headers, implementations, READMEs, and assets) |
| L3_Middlewares/CMakeLists.txt | Enabled LVGL demos and updated linking configuration |
| L2_Core/utils/inc/ek_log.h | Changed logging API: renamed macros/functions and modified function signature |
| L2_Core/utils/src/ek_log.c | Updated logging implementation to match new API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # endif /* EK_LOG_BUFFER_SIZE */ | ||
|
|
||
| # define EK_LOG_FILE_TAG(tag) static const char *__EK_LOG_TAG__ = tag; | ||
| # define EK_LOG_FILE_TAG(tag) static const char *_EK_LOG_TAG_ = tag; |
There was a problem hiding this comment.
The macro name has been changed from __EK_LOG_TAG__ to _EK_LOG_TAG_ (double underscores to single underscores). This is a breaking change for any code that defines file tags using the old naming convention. While the new naming is better (avoiding reserved identifiers), this change needs to be documented and existing code must be updated.
| # define EK_LOG_FILE_TAG(tag) static const char *_EK_LOG_TAG_ = tag; | |
| # define EK_LOG_FILE_TAG(tag) \ | |
| static const char *_EK_LOG_TAG_ = tag; \ | |
| static const char *__EK_LOG_TAG__ = tag; |
| */ | ||
| void _ek_log_printf(const char *tag, uint32_t line, ek_log_type_t type, const char *fmt, ...); | ||
| void _ek_log_printf(const char *tag, uint32_t line, ek_log_type_t type, uint32_t tick, const char *fmt, ...); | ||
| uint32_t _ek_log_get_tick(void); |
There was a problem hiding this comment.
The function signature of _ek_log_printf has been changed to add a tick parameter. This is a breaking API change that could affect existing code calling this function directly. The tick parameter is now passed explicitly instead of being called internally, which changes the function's interface. Consider whether this breaking change is intentional and if so, ensure all call sites are updated accordingly.
| static char ek_log_buffer[EK_LOG_BUFFER_SIZE]; | ||
|
|
||
| __WEAK uint32_t __ek_log_get_tick(void) | ||
| __WEAK uint32_t _ek_log_get_tick(void) |
There was a problem hiding this comment.
The variable name has been changed from __ek_log_get_tick to _ek_log_get_tick (removing one underscore). While this follows better naming conventions (avoiding double underscores which are reserved by the C standard), this is a breaking change. Any code that overrides this weak function will need to be updated to match the new name.
| lwvsnprintf(ek_log_buffer, EK_LOG_BUFFER_SIZE - 1, fmt, args); | ||
| va_end(args); |
There was a problem hiding this comment.
The removal of the explicit null termination after lwvsnprintf may cause issues. The lwvsnprintf function typically null-terminates the buffer, but explicitly setting ek_log_buffer[length] = '\0' was a safety measure. Removing this line assumes that lwvsnprintf always properly null-terminates, which may not be guaranteed in all implementations. Consider keeping the explicit null termination for safety.
| tag, | ||
| line, | ||
| __ek_log_get_tick()); | ||
| _ek_log_get_tick()); |
There was a problem hiding this comment.
There's an inconsistency in the logging implementation. On line 57, _ek_log_get_tick() is still being called within _ek_log_printf, but the function signature was changed to accept tick as a parameter (line 44). This means the tick is calculated twice - once by the caller and once inside the function. Either remove the internal call and use the parameter, or revert the signature change.
| _ek_log_get_tick()); | |
| tick); |
No description provided.