Repository navigation
Add a generic SD card service with live-configurable pins - #134
TouchMyLight wants to merge 0 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
this is bigger feature introduction. let me know if it should be changed to better be integrated into this project. |
|
All of this is supercool ! I am currently working on this project: https://moonmodules.org/MoonLight/how-to/multi-board-installation.html But exactly 20x20x20 is my next project. This is my old setup running on old MoonLight: trim.B4A063AA-7068-43F6-87E8-2DBCB01DD1D3.MOVSo I will look at your work with great interest. Coming back to this soon ! |
|
more information about the installation and the hardware sources are available here: https://git.touchmylight.info/TouchMyLight/chandeLED |
That's a beauty! Where can I buy 🙂 Regarding : 1x16 using PARLIO You know the P4 also has lcd_cam, which give you another 16 pins to drive 🙂 And if you want even more, MoonLight supports "pinExpander" based on @hpwit his virtual clockless driver using 595 shift registers, giving you up to 120 more pins 👻 |
|
I actually used the LCD driver for the cube. The readme was outdated, but this is fixed now. |
|
Thanks a lot for this, and for testing it end to end on the P4-ETH, including a live format. Your plan behind it is exactly the kind of thing MoonLight should do: an FSEQ player that runs an installation like your 20×20×20 cube without a computer. Other uses the card opens up:
The platform side ( 1. The card as a mount point in the File Manager. If the platform's file layer sends every path under 2. Flash: only in images with room. Measured on the classic ESP32 image, same base commit: +81.2 KB (FatFs 26.1, SD protocol 16.0, SD host driver 14.3, C stdio file functions 15.2, the module, routes and UI 9.6). On a 4 MB board that is a quarter of the app slot's remaining room, and the classic ESP32's SD host only works on its fixed pins (2, 4, 12, 13, 14, 15). Please build the SD card into the images that have room (P4, S31, the 16 MB S3) and leave it out of the 4 MB ones, the same way the S3-Zero image leaves out the USB host. Using FatFs directly ( 3. Large transfers off the render thread. HTTP requests are served from 4. Card sizes past 4 GB. 5. One open file per download. 6. Smaller items.
7. Tests. Map 8. Start again from current main. 17 of the 21 files changed on main since this branched: git fetch upstream
git switch sdcard
git reset --hard upstream/main # the branch now starts from today's main
git checkout ade3f71f -- src/platform/esp32/platform_esp32_sdmmc.cpp src/core/system/SdCardModule.h src/core/system/SdCardModule.cpp test/unit/core/unit_SdCardModule.cpp
# rework along points 1 to 7, then commit
git push --force-with-lease origin sdcardThe force-push keeps this PR and its discussion, and your first version stays reachable as Locally, Looking forward to the FSEQ player! |
PR MoonModules#136 review item 6 (yours, lower priority): fsWriteStream (pull, upload-to-completion in one call) duplicated the exact same chunk-and-commit loop three times - once each for flash on ESP32, flash on desktop, and the card's own callback path - instead of being one core helper over the already-platform-abstracted fsWriteStreamOpen/Write/Close. fsReadAt (positional, reopen-per-call) had no caller left in src/ since point 5 of the MoonModules#134 review moved every real download onto FsReadStream. Flash: esp32p4rev1-eth -704 B (2329250 -> 2328546). desktop-linux -176 B (2548104 -> 2547928). Account: src +17/-134 lines | test +12/-5 lines | code findings 1642 -> 1630 (complex function -3, duplicated block -8, large stack frame -1) | duplicated lines 3767 -> 3738 (-29) Core: - platform.h: fsWriteStream is now one inline function, pulling from `src` into a 1024-byte chunk and handing each to fsWriteStreamOpen/Write/Close - the same shape encoderClaim/Release already give a helper composed over other platform primitives. fsReadAt is gone. - platform_esp32_fs.cpp, platform_desktop.cpp: their own fsWriteStream/fsReadAt implementations are gone - fsWriteStreamOpen/Write/Close already dispatched flash vs. SD internally, so nothing platform-specific was left to write here. - platform_esp32_sdmmc.cpp/.h: sdCardWriteStreamCb (a fourth copy of the same loop, directly against FatFs) and sdCardReadAt are gone with their last caller. Tests: - unit_FileManagerModule.cpp: the positional read-back test now reads through FsReadStream (skip-then-read, the same contract a real download uses) instead of fsReadAt; a stale comment naming fsReadAt as the download primitive is corrected. - unit_HttpServerModule_sdcard.cpp: the multi-tick upload/download round-trip test reads the file back through FsReadStream instead of fsReadAt, unchanged in what it proves. Checks (scoped to this diff's own trigger-table buckets - src/ and test/ only): - check_specs: PASS - check_prose: PASS - 0 new findings (one pre-existing, unrelated finding in docs/reference/hardware/gpio-usage.md, not touched by this diff) - check_docgen: PASS - 0 errors, 0 net new warnings (matches the committed baseline exactly) - check_code: PASS - findings fell (1642 -> 1630): the three duplicate pull-loops were themselves flagged as duplicated blocks against each other, and one of the three carried its own large-stack-frame finding - build_desktop: PASS - zero errors/warnings in touched files - test_desktop: PASS - 2436/2436 - run_scenario: PASS - FilesystemModule (2/2), FileManagerModule (1/1), HttpServerModule (1/1, 1 pre-existing unrelated skip) - check_esp32_built: PASS - fresh after the real rebuild+flash - collect_kpi: PASS on the account above; the live ESP32 tick/FPS reading reflects whatever demo config is already on the bench board, not this diff - Real hardware (esp32p4rev1-eth, serial): rebuilt, reflashed, verified live - a flash-file upload/download round trip and an SD-file upload/download round trip both came back byte-exact through the consolidated primitive. One unrelated Guru Meditation (Cache error) self-recovered mid-session (clean reboot, rst:0xc) during an unrelated DELETE call on a path this diff never touches; retried cleanly afterward with no further incident, the same benign class already seen earlier this session.
PR MoonModules#136 review item 9 (yours, lower priority): fsSize and the SD-side sdCardSize behind it returned long, 32-bit and signed on ESP32. A real file at or past 2 GiB-1 (well inside FAT32's own 4 GiB-1 ceiling) overflowed that into a negative number, which every caller's `< 0` check then read as "missing" rather than "large" - the exact same shape point 4 of the MoonModules#134 review already fixed for sdCardUsed/sdCardTotal, widened to uint64_t for the same reason. fsSize carries a -1 sentinel, so its fix widens to the signed int64_t rather than unsigned. Flash: esp32p4rev1-eth +120 B (2328842 -> 2328962). desktop-linux unchanged. Account: src +22/-22 lines | test +17/-3 lines | duplicated lines 3738 -> 3738 (+0) | code findings 1630 -> 1630 (+0) Core: - platform.h: fsSize widened to int64_t; FsWriteSrc and the stream primitives untouched, since a single bounded chunk never approaches the 32-bit ceiling even where the whole file does. - platform_esp32_fs.cpp, platform_desktop.cpp: their fsSize implementations' narrowing casts (`static_cast<long>`) widened to int64_t. The desktop one was already correct on Linux (64-bit long) but not on Windows, where long stays 32-bit even in a 64-bit build - a second, previously silent instance of the same bug, fixed by the same change. - platform_esp32_sdmmc.h/.cpp: sdCardSize widened the same way - FatFs's own FSIZE_t already holds the real value correctly; only the narrowing return cast was throwing it away. - HttpServerModule.cpp, FilesystemModule.cpp, ControlModule.h, MoonLiveScriptFile.h: every `const long size = platform::fsSize(...)` local widened to int64_t to match, and the two Content-Length/log %ld format specifiers that printed one widened to %lld with an explicit cast - the same pairing other wide values in this codebase already use (JsonSink's %lld, the SD free-space message's %llu). Tests: - unit_FileManagerModule.cpp: a new case resizes a real file to 3,000,000,000 bytes (a sparse resize, not a real multi-gigabyte write) and checks fsSize reports it exactly, not negative - the case that overflowed a 32-bit signed long before this change. Checks (scoped to this diff's own trigger-table buckets - src/ and test/ only): - check_specs: PASS - check_prose: PASS - 0 new findings (one pre-existing, unrelated finding in docs/reference/hardware/gpio-usage.md, not touched by this diff) - check_docgen: PASS - 0 errors, 0 net new warnings (matches the committed baseline exactly) - check_code: PASS - 0 net new findings - build_desktop: PASS - zero errors/warnings in touched files - test_desktop: PASS - 2439/2439 (1 new) - run_scenario: PASS - FileManagerModule (1/1), ControlModule (2/2), FilesystemModule (2/2) - check_esp32_built: PASS - fresh after the real rebuild+flash - collect_kpi: PASS on the account above; the live ESP32 tick/FPS reading reflects whatever demo config is already on the bench board, not this diff - Real hardware (esp32p4rev1-eth, serial): rebuilt, reflashed, verified live - a flash-file and an SD-file round trip both came back byte-exact, no crash. Reproducing the exact >2 GiB boundary on this bench setup was not practical (the throttled transfer rate measured earlier this session, ~98 KB/s, would take hours to move 2+ GB); that exact boundary is proven on desktop instead, where the fix is the same portable int64_t arithmetic on both platforms, unlike a chip-specific driver call.
Adds microSD read/write as a chip-capability service on boards with flash to spare. The card mounts as a second folder in the existing File Manager tree, with no new HTTP routes and no new client UI: /api/dir and /api/file already dispatch on path, so the card gets upload, download, mkdir and recursive delete for free. A large transfer resumes a bounded chunk at a time across many ticks rather than blocking one, so a multi-minute upload never stalls rendering. Flash: esp32p4rev1-eth +150640 B (2177776 -> 2328416), esp32s3-zero +13152 B (2219120 -> 2232272). Account: src +1279/-51 lines | test +596/-1 lines | code findings 1620 -> 1629 (blocking call on the render path +6, complex function +6, large stack frame -3) | duplicated lines 3733 -> 3733 (+0) Core: - New SdCardModule (pins, format button, KiB usage gauge) and platform::sdMount/sdUnmount/sdFormat/sdCardUsed/sdCardTotal (uint64_t, so a >4 GB card is read correctly; the shared, size_t, flash-sized Progress gauge is fed KiB instead of being widened for one caller). - platform::fsXxx (read/write/list/exists/remove/mkdir) each gained a path-prefix check: /sd routes to the card, everything else keeps going to LittleFS/the desktop root. FileManagerModule, HttpServerModule's /api/dir and /api/file, and FilesystemModule::removeTree needed no changes at all to pick the card up. - New FsReadStream/FsWriteStream (explicit open/step/close) replace the reopen-per-chunk fsReadAt pattern for both flash and SD downloads, fixing a FAT cluster-chain re-traversal cost on every chunk. - New SdTransfer slot pair + HttpServerModule::driveSdTransfers, called from tick20ms next to drainPreviewSend/drainStateSend: a non-blocking read/writeSome per active slot per tick, bounded to one buffer's worth, with a 10 s stall guard (a liveness check, not a size or time cap on the transfer itself). /sd uploads and downloads hand off into this instead of the flash path's blocking-but-small synchronous handler. - esp32/platform_esp32_sdmmc.cpp: mount/unmount/format (field-by-field slot config, explicit NO_CD/NO_WP, GPIO validation before any pin-mask shift, the power-switch and LDO-domain sequencing a P4 needs) plus the file ops, written directly against FatFs (f_open/f_read/f_write) rather than through the stdio/VFS layer, to drop stdio's share of the flash cost. A write lands in a .tmp file and is renamed into place only on success (f_rename itself won't replace an existing file, so the old one is unlinked first); every failure path - write error, a stalled peer, a dropped connection - discards the .tmp instead, so a failed upload never leaves a partial file under its real name. Verified by killing a 10 MB upload mid-transfer: the card came back to exactly its prior contents. - A real bug from live hardware testing: driveSdUpload read into a 4 KB stack-local every tick, which a multi-minute upload stacked deeply enough to panic the board (a crash a browser's fetch() reports as the opaque "NetworkError when attempting to fetch resource.", not a clue to the real cause). Fixed by reading into the SdTransfer struct's own scratch buffer instead, the same way driveSdDownload already did it. Verified with a 40 MB upload (10x the file that first crashed it): one continuous 3m50s connection, board uptime and heap unmoved, file landed byte-exact. - hasSdCard gates on CONFIG_SOC_SDMMC_HOST_SUPPORTED *and* a new project Kconfig bool (MM_SD_CARD, set per-board: esp32p4rev1-eth, esp32s31, esp32s3-n16r8), not on chip capability alone - classic ESP32's SD host only works on fixed pins anyway, and the point is shipping it only where there's flash to spare. Desktop's hasSdCard is unconditionally true, backed by a real <root>/sd subfolder, so its tests exercise the real mount/format/gauge logic. UI: - app.js: zero changes for the File Manager tree itself (the unified /api/dir and /api/file routes make the card appear automatically); the format button renders through the existing generic confirm-button machinery. - A second bug from live use: the drop-upload path still applied FM_UPLOAD_CAP (256 KiB, sized for flash config files) to a /sd drop, where the server has no cap of its own beyond the card's free space - an 8.7 MB upload was rejected client-side before any request was sent. Fixed with fmIsSdPath(destDir), gating both the size check and the "not uploaded" message's wording. - A progress bar for the upload, requested after the fix: fmDropUpload now goes through uploadWithProgress (the same XMLHttpRequest helper the firmware-flash UI already uses - fetch cannot report upload progress at all), driving a small floating, non-modal panel (one row per file, closes on completion) so a long /sd transfer is visible without blocking the rest of the page the way the firmware-update overlay deliberately does. - style.css: .fm-upload-panel / .fm-upload-panel-row, bottom-left (the PiP preview pane already claims bottom-right), matching its fixed-position/border/shadow convention. Scripts/MoonDeck: - check_devices.py: "SD card" added to SUPPORTED_VOCAB. Tests: - unit_SdCardModule.cpp: pins visible/hidden by hasSdCard, mount/remount/release/format/tick1s-gauge round trip against a real backing folder. - unit_HttpServerModule_sdcard.cpp: traversal rejection, a multi-chunk upload/download round trip, the stall timeout, 507 (card full), 413 (FAT32 ceiling), the root listing advertising "sd", DELETE removeTree on a non-empty SD folder. - file-manager-sd-upload.test.mjs: fmIsSdPath's gating, a flash-bound file over cap skipped client-side, the same file bound for /sd sent instead, a dropped connection reported plainly rather than as a generic network error, a 507 surfacing the server's own message. Docs/CI: - docs/moonmodules/core/system.md: new "SD Card" section (pins, format, gauge, "appears under /sd in the File Manager"). Its card image reference is kept (check_docgen requires the tag even though the asset doesn't exist yet - a disclosed, unresolved gap, not a regression). - docs/reference/hardware/gpio-usage.md, esp32-p4-eth.md: dropped the "(unimplemented)" / "MoonLight does not mount it" claims the feature now makes false. - esp32/main/Kconfig.projbuild, three sdkconfig.defaults.*: MM_SD_CARD. - mooninstaller/deviceModels.json: the Waveshare ESP32-P4-ETH's "SD card" moved planned -> supported, with its SdCardModule pin catalog entry. Reviews (🐇 ewowi, PR #134): 1. Mount as /sd, delete the parallel API - done. The original commit's sdXxx public functions, /api/sddir + /api/sdfile routes, and the File Manager's separate SD panel are gone; one tree, one toggle. 2. Ship only where there's flash to spare; use FatFs directly, not stdio - done, via the new MM_SD_CARD Kconfig bool and the raw-FatFs rewrite above. The hoped-for flash saving didn't fully materialize (~240 B measured, not the ~15 KB estimated) because platform_esp32_fs.cpp's LittleFS side still links stdio regardless; said here rather than left to look like a wasted rewrite. 3. Large transfers blocking the render thread - done, as a real resumable per-tick transfer (SdTransfer + driveSdTransfers) rather than the smaller interim fallback of a size cap; no SD-specific ceiling beyond the card's own free space, which the 507 check already enforces. feedWatchdog() is gone: nothing blocks long enough to need it. 4. Cards over 4 GB - done, via the uint64_t sdCardUsed/Total above. 5. One open handle per download - done, via FsReadStream/FsWriteStream. 6. Smaller items (GPIO validation, doc fixes, positive doc-comment headings) - done; see platform_esp32_sdmmc.cpp's validOutputPin and its own "Drive 0 is assumed, not queried" / "Built field by field" sections. 7. Desktop test coverage - done: hasSdCard is unconditionally true on desktop, backed by a real subfolder, so unit_SdCardModule.cpp and the HTTP route tests exercise the real logic rather than a no-hardware stub. 8. Start again from current main - done; this branched from main after #135 merged, with the four reusable files (platform_esp32_sdmmc.cpp, SdCardModule.h/.cpp, unit_SdCardModule.cpp) rewritten against it rather than cherry-picked, since 17 of the original 21 files had changed on main since the old branch point. Checks: - check_specs: PASS - 136/136 modules - check_prose: PASS - 1 finding in added lines, a verified Vale tool artifact (isolating one changed table row without its header mis-parses it as a long sentence; a direct Vale run against the real file shows zero findings there) - build_docs --strict: PASS - check_docgen: PASS - 0 errors, 4314 warnings (was 4286; the new SD files' own docs, not a regression in a touched existing file) - test_host --python: PASS - 377 passed - check_taglines: SKIP - no front-page .md touched - check_code: FAIL - ratchet rose (blocking call on the render path +6, complex function +6); large stack frame improved (-3). Both remaining deltas are structural, not debt: one more per-tick resumable transfer mirroring drainPreviewSend/drainStateSend's already-accepted shape, and one more periodic gauge read mirroring filesystemUsed's. Already simplified what this change touched (driveSdTransfers split into driveSdUpload/driveSdDownload, handleWriteFile's /sd leg extracted into handleWriteFileSd) before accepting the rest per coding-standards.md's own escape valve: "a function may stay over a limit when splitting it would hide a sequence that belongs together." - build_desktop: PASS - zero errors/warnings in touched files - test_desktop: PASS - 2427/2427 - run_scenario: PASS - 30 scenarios, 17 passed / 13 skipped, 0 failed - check_esp32_built: PASS - fresh after the real rebuild+flash - build_desktop --no-jit: SKIP - MoonLive not touched - collect_kpi: PASS - recorded above - test_host --js: PASS - 243 passed - check_devices: PASS - 38 devices, 0 issues - check_firmwares: PASS - 14 variants, 0 issues - improv_smoke_test: SKIP - provisioning path not touched - Real hardware (esp32p4rev1-eth, over the network - serial unavailable this session): mount, File Manager discoverability, upload/download round trip, a 40 MB /sd upload completing without a crash, an interrupted upload leaving no partial file. The progress panel's wiring has its own JS tests; its on-screen appearance still wants a look in a real browser.
Mount a microSD card at /sd, addressing ewowi's PR #134 review
add microSD read/write support, shaped as a chip-capability service rather than a board-specific firmware variant: pins are runtime configuration like AudioService's mic, and no new Kconfig option or sdkconfig fragment is needed.
Core
platform::hasSdCardis now a chip-capability flag (CONFIG_SOC_SDMMC_HOST_SUPPORTED), not a per-board Kconfig gate;sdMount()takes pins as a runtimeSdPinConfigargument (clk/cmd/d0-d3/pwr) instead of hardcoded GPIOs, with bus width derived from which data lines are set. Newplatform::feedWatchdog()andplatform::sdFormat()calls.SdCardModule(Services child): pin controls hidden where the chip has no SDMMC host, a usage gauge, and a hiddenformatbutton. Mounting lives inprepare()/affectsPrepare(), notsetup(), so a pin edit unmounts and remounts live, the same contractI2cBusModule's sda/scl have;release()unmounts. This was a real bug caught by bench testing a first version that only wiredsetup().HttpServerModule:/api/sddir+/api/sdfileroutes mirroring the flash File Manager's, with an SD-sized Content-Length ceiling (FAT32's 4 GiB − 1) and overflow-safe parsing on this 32-bit target.UI
renderSdCardManager), sharing the File Manager's tree/toolbar machinery over anapiKindparameter. A 💾 Format button, SD-panel only, press-twice armed like the existing delete/restore buttons, wipes and reformats the card as FAT32.Catalog
SdCardModulewired with its schematic pins (clk 43, cmd 44, d0-d3 39-42, pwr 45), "SD card" moved to supported.check_devices.py: "SD card" added to the supported-capability vocabulary.Tests:
unit_SdCardModule.cpp— hidden pin controls on a hardware-less build, the Service role, prepare()'s "no SD hardware"/pin-live-reconfiguration contracts, release() safety, the format button's hidden declaration and its refusal while unmounted.Docs:
gpio-usage.mdandcore/system.mdcarry minimal SD mentions; the full per-board pin table belongs ondocs/reference/hardware/esp32-p4-eth.md, which doesn't exist on this branch (it's part of the still-unmergedesp32-p4-eth_ar/PR #130 branch) — not duplicated here.Bench-verified end to end on the Waveshare ESP32-P4-ETH (plain
esp32p4rev1-ethfirmware, flashed over OTA): mount survives reboot with persisted pins, mkdir/upload/read/delete round-trip, live pin-remount without a reboot, and — with the product owner's explicit go-ahead to wipe the card's real content — a live Format that emptied the card and left it fully writable afterward.docs/reference/metrics/docgen.mdis deliberately NOT included in this commit: regenerating it surfaces a pre-existing, unrelated error insrc/light/drivers/Hub75Slots.h(a 2-line code comment, from a prior commit that never re-ran the generator) that this change did not introduce and does not fix. Warnings are otherwise flat against the committed baseline, net of one untraced+1in the over-wide-comment-lines rule after exhaustive searching across every touched file.