-
-
Notifications
You must be signed in to change notification settings - Fork 8
V070 preps #79
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
V070 preps #79
Conversation
|
Caution Review failedThe pull request is closed. Note
|
| Cohort / File(s) | Summary |
|---|---|
Documentation & UI docs/moonbase/inputoutput.md, docs/index.md, docs/moonlight/effects.md, docs/gettingstarted/installer.md, docs/network/sta.md, README.md, misc/misc.txt |
Cosmetic and content updates: emoji-enhanced pin labels, release/version text and media thumbnails updated, installer/docs date/version bumps, example flash command filename updated. |
Pin usage enum & presets src/MoonBase/Modules/ModuleIO.h |
Replaced pin_Button_LightsOn with pin_Button_Push_LightsOn and pin_Button_Toggle_LightsOn; added pin_Exposed; updated human-readable control labels (emoji variants) and board preset mappings to new enum members. |
Pin usage propagation src/MoonLight/Modules/ModuleLightsControl.h |
Updated readPins conditional from pin_Button_LightsOn → pin_Button_Push_LightsOn. |
ArtNet driver refactor src/MoonLight/Nodes/Drivers/D_ArtnetIn.h |
Renamed public member view → layer; updated logic, indexing and config binding to use layer. |
WLED effect fix src/MoonLight/Nodes/Effects/E_WLED.h |
In BouncingBallsEffect::onSizeChanged, changed allocation/size use from layer->size.x → layer->size.y. |
Version & installer manifests firmware/installer/*.json, firmware/installer/manifest.json, platformio.ini, interface/package.json |
Bumped versions from 0.6.x → 0.7.0 and updated referenced webflash binary filenames; updated APP_VERSION/APP_DATE. |
Frontend small change interface/src/lib/components/moonbase/FieldRenderer.svelte |
Removed target="_blank" from mdnsName anchor so the link opens in the same tab. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Check all usages/serializations referencing the old
pin_Button_LightsOnare updated (presets, saved configs). - Verify
pin_Exposedenum position doesn't break serialization/ABI assumptions. - Confirm ArtNet
view→layerrename is reflected in config bindings and saved settings; validate indexing offsets for virtual layers. - Validate WLED dimension change produces correct behavior for multiple layer sizes.
- Spot-check manifest/package/version bumps and the frontend anchor change.
Possibly related PRs
- Update board definitions #73 — Overlapping changes to
IO_PinUsageEnumand board preset definitions (pin usage remapping). - Board presets more presets #71 — Related edits to
D_ArtnetIn(renaming/logic aroundview/layer). - Board presets p4 #74 — Overlaps ModuleIO enum edits and board preset/LED pin mapping updates.
Suggested labels
enhancement
Poem
🐰 I hopped through enums and gave pins a name,
Buttons split in two and LEDs gained fame.
Views now call themselves a proper layer,
WLED counts by height — the pixels cheer and stare.
Hop, hop, release! A carrot-code hooray 🥕🎉
Pre-merge checks and finishing touches
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
|
| Title check | ❓ Inconclusive | The title 'V070 preps' is vague and generic, using abbreviation without conveying meaningful information about the specific changes in the changeset. | Consider a more descriptive title that captures the main change, such as 'Bump version to 0.7.0 with UI enhancements and API updates' or similar. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
firmware/installer/manifest_esp32-d0-16MB.jsonfirmware/installer/manifest_esp32-s3-stephanelec-16p.json
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 @coderabbitai help to get the list of available commands and usage tips.
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MoonLight/Modules/ModuleLightsControl.h (1)
72-100: Add handling for the toggle button variant or document why it's not needed.The code checks for
pin_Button_Push_LightsOnbut leavespin_Button_Toggle_LightsOn(defined at ModuleIO.h:36) unhandled. Add a corresponding conditional branch if toggle behavior is intended, or clarify in a comment whether this variant is a planned future feature or intentionally unsupported for now.
🧹 Nitpick comments (1)
src/MoonBase/Modules/ModuleIO.h (1)
19-67: Refine implementation: Toggle button variant is unused and future migration strategy needed.The enum defines
pin_Button_Toggle_LightsOn(line 36) but it is never referenced anywhere in the codebase—only the Push variant is implemented (ModuleLightsControl.h:89). Either remove the unused enum value or implement its corresponding functionality.More broadly, enum values are persisted as integers in
_state.data["pins"][i]["usage"](ModuleIO.h:599, 221), making the configuration brittle to enum reordering. The newpin_Exposedentry shifts all subsequent enum values; if this firmware is deployed and users create pin configurations, a future enum change will silently map existing stored values to incorrect pin usages. Establish a versioning or migration strategy for persisted pin configurations before production release.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/moonbase/inputoutput.mdsrc/MoonBase/Modules/ModuleIO.hsrc/MoonLight/Modules/ModuleLightsControl.hsrc/MoonLight/Nodes/Drivers/D_ArtnetIn.hsrc/MoonLight/Nodes/Effects/E_WLED.h
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-18T15:15:57.828Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:15:57.828Z
Learning: In DripEffect (E_WLED.h), dropsSize must be set to layer->size.y (not layer->size.x) in onSizeChanged(), because the drops array is allocated as [layer->size.y][maxNumDrops]. Using layer->size.x causes out-of-bounds access when iterating over y-coordinates.
Applied to files:
src/MoonLight/Nodes/Drivers/D_ArtnetIn.hsrc/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:30:02.486Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:30:02.486Z
Learning: In DripEffect (E_WLED.h), the bounce at line 1669 uses heavy damping (`vel = -vel / 4`), which makes the bounce too weak to be visible. Reducing the damping factor from 4 to 2 (or lower) makes the bounce more pronounced and visible.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:59:30.556Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:59:30.556Z
Learning: In DripEffect (E_WLED.h), the bounce fails because the damped velocity is too weak to keep the drop at pos > 0 for multiple frames. When the drop returns to pos <= 0 while still in the bouncing state (colIndex == bouncing), it immediately resets to init (line 1663-1664) before any visible bounce occurs. The bounce velocity must be strong enough (divide by 1.1 or less, or use minimal damping like *= 0.9) so the drop stays airborne long enough for a visible bounce arc before the eventual second ground hit triggers the reset.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:46:48.319Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:46:48.319Z
Learning: In DripEffect (E_WLED.h), the bounce damping at line 1669 needs to be `/1.5` or less (not `/4` or `/2`) to create a visible bounce effect. The continuous gravity application every frame (line 1652) requires a strong initial bounce velocity to overcome it. Using `/1.5` gives approximately 67% energy return, making the bounce visible and lasting for ~33 frames.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
🧬 Code graph analysis (1)
src/MoonLight/Nodes/Drivers/D_ArtnetIn.h (1)
src/MoonBase/Module.cpp (2)
addControl(361-370)addControl(361-361)
🪛 markdownlint-cli2 (0.18.1)
docs/moonbase/inputoutput.md
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
36-36: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
37-37: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
src/MoonLight/Nodes/Effects/E_WLED.h (1)
33-42: LGTM! Correct dimension fix for ball allocation.The change to use
layer->size.yforballsSizeis correct and aligns with the allocation at line 34, which useslayer->size.y. This prevents potential out-of-bounds access in the loop at line 59 that iterates over the y-dimension.Based on learnings, this same pattern applies to other effects in this file (e.g., DripEffect).
src/MoonLight/Nodes/Drivers/D_ArtnetIn.h (4)
24-24: LGTM! Improved variable naming.Renaming
viewtolayerimproves code clarity and aligns with the domain terminology (Physical layer vs. Virtual layer). The semantic meaning is preserved.
29-43: LGTM! Control binding updated consistently.The control name was updated from "view" to "layer" to match the renamed member variable, maintaining consistency between the data model and UI.
110-142: LGTM! Conditional logic updated consistently.All conditionals using the renamed
layervariable have been updated correctly. The logic remains unchanged:layer == 0targets the physical layer, whilelayer > 0targets virtual layers with proper index adjustment (layer - 1).
144-170: LGTM! DDP handler updated consistently.The DDP packet handler mirrors the ArtNet handler's logic, correctly using the renamed
layervariable with the same conditional patterns.src/MoonBase/Modules/ModuleIO.h (4)
147-194: LGTM! UI labels updated with emoji variants.The control labels have been enhanced with emoji icons, improving visual identification in the UI. Changes are cosmetic and consistent with the documentation updates.
284-302: LGTM! Board preset updated consistently.The SE16V1 board preset correctly uses the new
pin_Button_Push_LightsOnenum value (line 290), aligning with the enum changes.
340-375: LGTM! Board presets updated with new enum values.The Dig2Go and commented Serg Mini Shield board presets have been updated to use:
pin_Button_Push_LightsOn(lines 343, 381)pin_Exposedfor exposed pins (lines 352-353)Changes are consistent with the enum modifications.
393-416: LGTM! Additional board preset updated.The Serg Universal Shield v5 preset correctly uses
pin_Button_Push_LightsOn(line 402), maintaining consistency with the enum changes.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
50-50: Consider cleaning up the duplicate lightning emoji.Line 50 contains
⚡️️(two lightning bolt emojis) instead of a single⚡️. While this doesn't affect functionality, cleaning it up would improve consistency.🔎 Proposed fix
-- ⚡️️ **High Performance** - 12K LEDs at 100 FPS with FastLED + parallel drivers +- ⚡️ **High Performance** - 12K LEDs at 100 FPS with FastLED + parallel drivers
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (30)
firmware/installer/MoonLight_esp32-c3-devkitm-1_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-c3-devkitm-1_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-c3-supermini_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-c3-supermini_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-16mb_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-16mb_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-moonbase_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-moonbase_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-wrover-moonbase_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-wrover-moonbase_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-wrover_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0-wrover_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-d0_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-p4-nano_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-p4-nano_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-p4-olimex_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-p4-olimex_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-atoms3r_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-atoms3r_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-devkitc-1-n16r8v_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-devkitc-1-n16r8v_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-devkitc-1-n8r8v_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-devkitc-1-n8r8v_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-devkitc-1_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-devkitc-1_0-7-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-stephanelec-16p_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-zero-n4r2_0-6-0_webflash.binis excluded by!**/*.binfirmware/installer/MoonLight_esp32-s3-zero-n4r2_0-7-0_webflash.binis excluded by!**/*.bininterface/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
README.mddocs/gettingstarted/installer.mddocs/index.mddocs/moonlight/effects.mddocs/network/sta.mdfirmware/installer/manifest.jsonfirmware/installer/manifest_esp32-c3-devkitm-1.jsonfirmware/installer/manifest_esp32-c3-supermini.jsonfirmware/installer/manifest_esp32-d0-16MB.jsonfirmware/installer/manifest_esp32-d0-moonbase.jsonfirmware/installer/manifest_esp32-d0-wrover-moonbase.jsonfirmware/installer/manifest_esp32-d0-wrover.jsonfirmware/installer/manifest_esp32-d0.jsonfirmware/installer/manifest_esp32-p4-nano.jsonfirmware/installer/manifest_esp32-p4-olimex.jsonfirmware/installer/manifest_esp32-s3-atoms3r.jsonfirmware/installer/manifest_esp32-s3-devkitc-1-n16r8v.jsonfirmware/installer/manifest_esp32-s3-devkitc-1-n8r8v.jsonfirmware/installer/manifest_esp32-s3-stephanelec-16p.jsonfirmware/installer/manifest_esp32-s3-zero-n4r2.jsoninterface/package.jsoninterface/src/lib/components/moonbase/FieldRenderer.sveltelib/framework/WWWData.hmisc/misc.txtplatformio.ini
✅ Files skipped from review due to trivial changes (3)
- docs/gettingstarted/installer.md
- docs/network/sta.md
- docs/moonlight/effects.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/interface/package.json : Define frontend dependencies and build scripts in `interface/package.json`
Applied to files:
interface/package.json
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/**/platformio.ini : Use PlatformIO to build and upload the ESP32 backend C++ code
Applied to files:
platformio.ini
🪛 markdownlint-cli2 (0.18.1)
README.md
41-41: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (23)
firmware/installer/manifest_esp32-s3-zero-n4r2.json (1)
3-3: Version bump is consistent and binary file exists.The version update from 0.6.0 to 0.7.0 is correctly reflected in both the manifest version field and the binary filename. The referenced binary file
MoonLight_esp32-s3-zero-n4r2_0-7-0_webflash.binis present at the expected path with appropriate file size.interface/package.json (1)
3-3: LGTM! Version bump is consistent with the release.The frontend package version has been correctly updated to 0.7.0, aligning with the project-wide version bump for this release.
firmware/installer/manifest_esp32-c3-supermini.json (1)
3-3: LGTM! Manifest version and binary reference updated consistently.Both the manifest version and the webflash binary reference have been updated to reflect the 0.7.0 release.
Also applies to: 14-14
misc/misc.txt (1)
16-16: LGTM! Documentation updated to reflect the new binary name.The esptool flash command example has been correctly updated to reference the 0-7-0 webflash binary.
firmware/installer/manifest_esp32-d0-16MB.json (1)
3-3: LGTM! Manifest and binary reference updated consistently.Also applies to: 14-14
firmware/installer/manifest_esp32-d0-wrover.json (1)
3-3: LGTM! Version and binary reference updated correctly.Also applies to: 14-14
firmware/installer/manifest_esp32-c3-devkitm-1.json (1)
3-3: LGTM! Consistent version bump and binary reference update.Also applies to: 14-14
firmware/installer/manifest_esp32-p4-olimex.json (1)
3-3: LGTM! Manifest updated consistently with other device variants.Also applies to: 14-14
firmware/installer/manifest.json (1)
3-3: LGTM! All manifest entries updated consistently across device variants.The main manifest has been updated with the 0.7.0 version, and all 9 device variant binary references have been consistently updated from 0-6-0 to 0-7-0. platformio.ini correctly sets APP_VERSION to 0.7.0. No old 0-6-0 references remain.
platformio.ini (1)
58-59: LGTM! Version bump to 0.7.0 is consistent across the PR.The APP_VERSION and APP_DATE updates align with the release preparation objectives and follow the expected format.
interface/src/lib/components/moonbase/FieldRenderer.svelte (1)
110-110: Verify the intentional removal oftarget="_blank".The mdnsName link now opens in the same tab instead of a new tab. This changes the user experience—previously, users could keep their current page open while exploring the linked device. Confirm this is the intended behavior.
firmware/installer/manifest_esp32-s3-atoms3r.json (1)
3-3: LGTM! Manifest version and binary path are synchronized.The version bump to 0.7.0 and corresponding binary filename update are consistent with the release preparation.
Also applies to: 14-14
firmware/installer/manifest_esp32-p4-nano.json (1)
3-3: LGTM! Version and binary path updates are consistent.The manifest correctly reflects the 0.7.0 release with synchronized version and binary filename.
Also applies to: 14-14
firmware/installer/manifest_esp32-s3-devkitc-1-n16r8v.json (1)
3-3: LGTM! Manifest correctly updated for 0.7.0 release.Version and binary path are properly synchronized.
Also applies to: 14-14
firmware/installer/manifest_esp32-s3-devkitc-1-n8r8v.json (1)
3-3: LGTM! Version bump properly applied.The manifest version and binary path are correctly updated for the 0.7.0 release.
Also applies to: 14-14
docs/index.md (2)
35-47: LGTM! Video content updated for 0.7.0 release.The documentation now includes dedicated video sections for both 0.6.0 and 0.7.0 releases with appropriate thumbnails and links.
143-144: LGTM! Roadmap section updated for 0.7.0 release.The version section correctly reflects the new 0.7.0 release with its focus on Art-Net, board presets, and ESP32-P4 support.
README.md (2)
29-41: LGTM! README updated with 0.7.0 release content.The video sections and links are properly updated to showcase both 0.6.0 and 0.7.0 releases.
137-138: LGTM! Roadmap updated for 0.7.0 release.The version roadmap correctly reflects the new release focus on Art-Net, board presets, and ESP32-P4 support.
firmware/installer/manifest_esp32-d0.json (1)
3-3: LGTM! Version bump to 0.7.0 is consistent and binary file exists.The version update and corresponding binary path change are correctly aligned, with the referenced binary file
MoonLight_esp32-d0_0-7-0_webflash.binpresent in the repository.firmware/installer/manifest_esp32-d0-wrover-moonbase.json (1)
3-3: LGTM! Version bump to 0.7.0 is consistent.The version update and corresponding binary path change are correctly aligned. The referenced binary file exists at firmware/installer/MoonLight_esp32-d0-wrover-moonbase_0-7-0_webflash.bin.
Also applies to: 14-14
firmware/installer/manifest_esp32-d0-moonbase.json (1)
3-3: LGTM! Version bump to 0.7.0 is consistent.The version update and corresponding binary path change are correctly aligned. The referenced binary file exists in the repository.
firmware/installer/manifest_esp32-s3-stephanelec-16p.json (1)
3-3: LGTM! Version bump is consistent.The version increment from 0.6.0 to 0.7.0 follows semantic versioning and aligns with the PR objectives.
Summary by CodeRabbit
New Features
Bug Fixes
UI Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.