[KenVanHoeylandt]

Summary by CodeRabbit

• New Features
  • Added Midi Controller with BLE and USB MIDI support, offering keyboards, chord and arpeggiator tools, modulation controls, touch-based controllers, and rhythm sequencers.
  • Added optional 8Encoder and LCD accessory support for compatible modes.
• Compatibility
  • Expanded platform targeting for several apps, including BusyBox vi, HelloWorld, Neatvi, Tone, and Satellite Tracker.
• Documentation
  • Added Midi Controller setup and usage guides covering modes, scales, hardware, and optional accessories.
• Updates
  • Updated version numbers for multiple apps.

[coderabbitai]

📝 Walkthrough

The pull request adds a Midi Controller app with BLE and USB MIDI transport, multiple instrument and sequencer views, and optional 8Encoder and LCD Unit support. It also adds app build and hardware documentation. Several existing app manifests receive version increments, and selected manifests change their platform target to all. Release JSON conversion maps a sole all target to an empty platform list.

Estimated code review effort

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 9e376

Merge Risk: 🟡 Moderate · up to 9e376

The new Midi Controller app can access deleted UI objects when it exits, which risks a crash. Fix this before merging. The remaining issues are minor edge cases: held notes, timing drift, label contrast, and one out-of-bounds read in the ARP view. The manifest and platform-target changes carry no concrete risk.

Pre-merge checks: 3 · 1 · 1

Failed checks: 1 warning · 1 inconclusive

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 34.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 376 functions across 28 files. (37 skippe…	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check		The title names two changes in the changeset: updates for new platform naming and added macOS support. The raw summary mainly shows manifests that move from ESP32-specific platform lists to all platfo…	Revise the title to name the main change. For example, state that the PR adds the MidiController app and updates app manifests for new platform naming. If macOS support is part of this PR, name it. The summary does not show macOS-specific c…

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 34.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 376 functions across 28 files. (37 skipped: 37 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Title check

Explanation

The title names two changes in the changeset: updates for new platform naming and added macOS support. The raw summary mainly shows manifests that move from ESP32-specific platform lists to all platforms, plus a release script change that maps ["all"] to an empty platform list. It also adds the MidiController app, which the title does not mention. The title does not name the main change, which is the large new MidiController app. macOS support is not visible in the summary, so the title is only partly related to the changeset.

Resolution

Revise the title to name the main change. For example, state that the PR adds the MidiController app and updates app manifests for new platform naming. If macOS support is part of this PR, name it. The summary does not show macOS-specific changes, so confirm that the macOS change is in the diff.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 9

🧹 Nitpick comments: 1

Apps/MidiController/main/Source/ViewEuclid.cpp: 1

70-155: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

Avoid rebuilding the widget tree on every sequencer step.

cbEuclidTimer and cbTb3poTimer call their draw functions after each scheduled step. Each draw function calls lv_obj_clean and recreates the complete widget tree. This adds allocation and cleanup work to every step.

The documented pattern rebuilds EUCLID and TB3PO widgets when the pattern changes. Keep the widgets persistent, update playhead styles and positions on timer steps, and rebuild only when the pattern or step configuration changes.

---

ℹ️ Review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c24cfb37-c7a5-495c-8be3-60ecb6b603f7

📥 Commits

Reviewing files that changed from the base of the PR and between 2ed8c36 and a5341b7.

⛔ Files ignored due to path filters: 1

• Apps/MidiController/docs/MIDI-Control-Change-Messages_Another-Producer.pdf is excluded by !**/*.pdf

📒 Files selected for processing: 65

• Apps/Brainfuck/manifest.properties
• Apps/Breakout/manifest.properties
• Apps/BusyBoxVi/manifest.properties
• Apps/Calculator/manifest.properties
• Apps/Diceware/manifest.properties
• Apps/Doom/manifest.properties
• Apps/EpubReader/manifest.properties
• Apps/EspNowBridge/manifest.properties
• Apps/GPIO/manifest.properties
• Apps/GraphicsDemo/manifest.properties
• Apps/HelloWorld/manifest.properties
• Apps/M5UnitTest/manifest.properties
• Apps/Magic8Ball/manifest.properties
• Apps/MediaKeys/manifest.properties
• Apps/MidiController/CMakeLists.txt
• Apps/MidiController/docs/README.md
• Apps/MidiController/docs/architecture.md
• Apps/MidiController/docs/encoder.md
• Apps/MidiController/docs/hardware.md
• Apps/MidiController/docs/lcd.md
• Apps/MidiController/docs/modes.md
• Apps/MidiController/docs/scales.md
• Apps/MidiController/docs/units.md
• Apps/MidiController/main/CMakeLists.txt
• Apps/MidiController/main/Source/EncoderInput.cpp
• Apps/MidiController/main/Source/GroveLookup.cpp
• Apps/MidiController/main/Source/GroveLookup.h
• Apps/MidiController/main/Source/LcdDisplay.cpp
• Apps/MidiController/main/Source/LcdDisplay.h
• Apps/MidiController/main/Source/MidiController.cpp
• Apps/MidiController/main/Source/MidiController.h
• Apps/MidiController/main/Source/MidiScales.h
• Apps/MidiController/main/Source/Sequencer.cpp
• Apps/MidiController/main/Source/Sequencer.h
• Apps/MidiController/main/Source/UiKit.h
• Apps/MidiController/main/Source/ViewArp.cpp
• Apps/MidiController/main/Source/ViewBaby8.cpp
• Apps/MidiController/main/Source/ViewBeats.cpp
• Apps/MidiController/main/Source/ViewChord.cpp
• Apps/MidiController/main/Source/ViewDimensions.cpp
• Apps/MidiController/main/Source/ViewDrop.cpp
• Apps/MidiController/main/Source/ViewEuclid.cpp
• Apps/MidiController/main/Source/ViewGrid.cpp
• Apps/MidiController/main/Source/ViewKeys.cpp
• Apps/MidiController/main/Source/ViewLfo.cpp
• Apps/MidiController/main/Source/ViewRaga.cpp
• Apps/MidiController/main/Source/ViewRng.cpp
• Apps/MidiController/main/Source/ViewTb3po.cpp
• Apps/MidiController/main/Source/ViewXYPad.cpp
• Apps/MidiController/main/Source/ViewZen.cpp
• Apps/MidiController/main/Source/main.cpp
• Apps/MidiController/manifest.properties
• Apps/MystifyDemo/manifest.properties
• Apps/Neatvi/manifest.properties
• Apps/Rcc700/manifest.properties
• Apps/SatelliteTracker/manifest.properties
• Apps/SerialConsole/manifest.properties
• Apps/Snake/manifest.properties
• Apps/TamaTac/manifest.properties
• Apps/TodoList/manifest.properties
• Apps/Tone/manifest.properties
• Apps/TwoEleven/manifest.properties
• Apps/WebServer/manifest.properties
• Apps/Xcc700/manifest.properties
• Scripts/release.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.