Updates for Audio System#36
Conversation
And fix media keys after BT disabled change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughSfxEngine now uses the platform audio stream output instead of I2S, scales audio using cached system volume and enabled state, and removes app-level volume controls. Breakout and TamaTac no longer apply volume presets. MediaKeys centralizes Bluetooth cleanup, tracks device startup state, initializes devices through generic device APIs, responds to existing radio state, and auto-enables Bluetooth handling when shown. A new EspNowBridge application adds firmware validation, asynchronous co-processor OTA transfers, LVGL progress handling, WiFi lifecycle integration, build configuration, registration, and manifest metadata. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ced6a1c-6bda-48ab-bbe9-b7785c2b79fb
📒 Files selected for processing (7)
Apps/Breakout/main/Source/Breakout.cppApps/MediaKeys/main/Source/MediaKeys.cppApps/MediaKeys/main/Source/MediaKeys.hApps/TamaTac/main/Source/TamaTac.cppLibraries/SfxEngine/Include/SfxEngine.hLibraries/SfxEngine/README.mdLibraries/SfxEngine/Source/SfxEngine.cpp
💤 Files with no reviewable changes (2)
- Apps/TamaTac/main/Source/TamaTac.cpp
- Apps/Breakout/main/Source/Breakout.cpp
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Apps/EspNowBridge/main/Source/EspNowBridge.h (1)
88-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider using
std::functionfor UI dispatch.Using
std::function<void(EspNowBridge&)>instead of C-style callbacks allows callers to use capturing lambdas. This eliminates the need for manualvoid* contextmanagement, customfreeContextcleanup functions, and boilerplate static wrapper functions, significantly improving type safety and readability.♻️ Proposed refactor
Update the header to include
<functional>and change the signature:`#include` <optional> `#include` <string> +#include <functional> `#include` <freertos/FreeRTOS.h>/** Marshal a UI-touching closure onto the LVGL task. Only ever invoked if liveInstance_ is * still this instance (checked at dispatch time and again right before running, on the LVGL * task) and isShown_ is true (this app's widget tree exists). */ - void dispatchToUi(void (*work)(EspNowBridge&, void*), void* context, void (*freeContext)(void*)); + void dispatchToUi(std::function<void(EspNowBridge&)> work);For reference, the implementation in
EspNowBridge.cppcan be updated as follows:void EspNowBridge::dispatchToUi(std::function<void(EspNowBridge&)> work) { struct UiDispatchPayload { EspNowBridge* instance; std::function<void(EspNowBridge&)> work; }; auto* payload = new UiDispatchPayload{this, std::move(work)}; bool locked = tt_lvgl_lock(TT_LVGL_DEFAULT_LOCK_TIME); lv_result_t result = lv_async_call([](void* userData) { auto* payload = static_cast<UiDispatchPayload*>(userData); if (EspNowBridge::liveInstance_.load() == payload->instance && payload->instance->isShown_.load()) { payload->work(*payload->instance); } delete payload; }, payload); if (locked) { tt_lvgl_unlock(); } if (!locked || result != LV_RESULT_OK) { delete payload; } }This allows callers to write simple capturing lambdas like
dispatchToUi([parseError](EspNowBridge& app) { app.setStatus(parseError); });.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a3985e1-b760-4c68-abeb-17fb91681744
⛔ Files ignored due to path filters (1)
Apps/EspNowBridge/assets/espnow_bridge_slave_c6.binis excluded by!**/*.bin
📒 Files selected for processing (6)
Apps/EspNowBridge/CMakeLists.txtApps/EspNowBridge/main/CMakeLists.txtApps/EspNowBridge/main/Source/EspNowBridge.cppApps/EspNowBridge/main/Source/EspNowBridge.hApps/EspNowBridge/main/Source/main.cppApps/EspNowBridge/manifest.properties
|
Looks good, thanks! |
And fix media keys after BT disabled change.
Summary by CodeRabbit