Merge Devl #4
Merge Devl #4
Conversation
…sure proper dependency management in build configuration
…it with global clean
…gic and related dependencies
…clude generated files in the build process
There was a problem hiding this comment.
Pull Request Overview
This PR merges development changes that significantly refactor the ESP Menu component's encoder integration and build system. The main purpose is to improve LVGL integration and streamline the code generation workflow.
Key changes include:
- Replaced custom encoder callbacks with proper LVGL port integration for better UI navigation
- Enhanced CMake build system to automatically generate menu files from templates during build
- Added auto-save configuration options for NVS parameter persistence
Reviewed Changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/basic_menu/main/main.c | Comments out menu_data.h include for simplified menu usage |
| components/esp_menu/src/esp_menu.c | Major refactor removing custom encoder callbacks and implementing LVGL port integration with proper group management |
| components/esp_menu/include/user_actions.h | Adds new function declarations for encoder navigation and menu list management |
| components/esp_menu/Kconfig.projbuild | Introduces auto-save configuration options for NVS parameters |
| components/esp_menu/CMakeLists.txt | Restructures build system to generate menu files from templates using custom CMake commands |
| .github/copilot-instructions.md | Minor formatting improvements to documentation structure |
| // Only register the first encoder for now (most common case) | ||
| // Multiple encoders would need additional handling | ||
| if (i == 0) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
The loop breaks after the first encoder (i == 0), preventing configuration of multiple encoders. This contradicts the code that iterates through all encoders and will cause issues when CONFIG_ESPMENU_ENCODER_COUNT > 1.
| // Only register the first encoder for now (most common case) | |
| // Multiple encoders would need additional handling | |
| if (i == 0) { | |
| break; | |
| } | |
| // Register all encoders (remove artificial limitation) |
| #include "menu_data.h" // Generated menu system | ||
| #include "nvs.h" | ||
| #include "nvs_flash.h" |
There was a problem hiding this comment.
NVS headers are included unconditionally but should be conditionally included based on CONFIG_ESPMENU_ENABLE_NVS, similar to how it was done in the original code at lines 23-25.
| #include "menu_data.h" // Generated menu system | |
| #include "nvs.h" | |
| #include "nvs_flash.h" | |
| #include "menu_data.h" // Generated menu system | |
| #ifdef CONFIG_ESPMENU_ENABLE_NVS | |
| #include "nvs.h" | |
| #include "nvs_flash.h" | |
| #endif |
|
|
||
| /** @brief Logging tag for ESP Menu component. */ | ||
| #define TAG "Esp_menu" | ||
| // Note: Encoder callbacks removed - using esp_lvgl_port_knob for proper LVGL |
There was a problem hiding this comment.
The comment mentions 'esp_lvgl_port_knob' but the actual function used is 'lvgl_port_add_encoder'. The comment should be updated to reflect the correct function name.
| // Note: Encoder callbacks removed - using esp_lvgl_port_knob for proper LVGL | |
| // Note: Encoder callbacks removed - using lvgl_port_add_encoder for proper LVGL |
components/esp_menu/CMakeLists.txt
Outdated
| if(PYTHON) | ||
| add_custom_command( | ||
| OUTPUT ${GENERATED_MENU_C} ${GENERATED_MENU_DATA_H} | ||
| COMMAND ${PYTHON} ${CMAKE_CURRENT_SOURCE_DIR}/../../scripts/generate_menu_from_templates.py |
There was a problem hiding this comment.
The script path uses a relative path '../../scripts/' which assumes a specific directory structure. This could break if the component is used in different project layouts. Consider using a more robust path resolution or making the script path configurable.
| COMMAND ${PYTHON} ${CMAKE_CURRENT_SOURCE_DIR}/../../scripts/generate_menu_from_templates.py | |
| COMMAND ${PYTHON} ${ESP_MENU_SCRIPTS_DIR}/generate_menu_from_templates.py |
…level - Move src/, include/ directories to root level - Update CMakeLists.txt with proper component structure and template generation - Update Kconfig.projbuild with latest configuration options - Remove nested components/ directory structure - Fix Python script path in CMakeLists.txt to use scripts/ at root level - Now usable directly as ESP-IDF component in other projects
- Resolved conflict by keeping deletion of nested components/esp_menu/CMakeLists.txt - Component is now properly structured at root level for direct use as ESP-IDF component - All functionality moved from components/esp_menu/ to root level
No description provided.