Isp progress segments#28
Merged
Merged
Conversation
Review fixes on the LAB-44 visual indicator (stacked on #26): - Retire the boot-indicator flag AFTER arming the CIPO confirmation: the one-shot LittleFS erase (tens of ms, core 1 parked) no longer delays the triggering command's reply. It can still stall the NEXT transaction; after an ISP flash, send a throwaway display command before starting a stream. - Exclude ERROR_DISPLAY (0xC2) from retiring the flag; only content commands count (retires_boot_indicator). - kNominalPages 512 -> 576: real images (up to ~140 KB / ~550 pages) now fill the bar near-linearly without wrapping. - Failed ISP_COMMIT (status 8) shows a sad smiley instead of leaving the misleading full progress bar. - WRITE_PAGE: bounds-check the 24-bit page index itself so a huge index cannot wrap the 32-bit offset math past the staging limit. Testability: - Extract the pure logic (staging constants, bar math, retire filter, glyphs) into Arduino-free src/isp_logic.h, shared by isp.cpp, messenger.cpp, and the tests. - protocol.h no longer includes Arduino.h (stdint/stddef suffice) so the tests can compile it host-side. - New Unity tests (panel/test/test_isp_logic, 14 cases) on the native platform via `pixi run test`, using the system C++ compiler (conda's cxx-compiler breaks on the space in this repo's path). - New ci.yml runs the tests + the release catalog build on every PR and push to main. All 10 firmware envs rebuild clean; 14/14 native tests pass.
The LAB-44 bar now spans every phase the panel can physically show, weighted roughly by wall-clock: - WRITE_PAGE upload fills columns 0..16 (progress_cols rescaled to a 16-column segment; oversize images still wrap within it) - VERIFY_STAGED entry lights column 17 - COMMIT's LittleFS staging animates 17..20, pushed between 4 KB flash chunks (core 1 scans between ops); full bar = staged, reboot imminent. The old pre-staging full-bar push is gone. - The post-reboot OTA-copy window stays dark (the OTA stub runs before the framework); the boot smiley remains the success signal and the sad smiley the failure signal. All pushes go through a shared push_progress_if_changed throttle; segment constants and math live in isp_logic.h with the other pure logic. VERIFY_STAGED now CRCs the staged image through the same 4 KB PSRAM->SRAM burst buffer the COMMIT path uses (hoisted to a shared burst_buf_ with a crc32_psram helper), replacing the per-byte XIP scan and its stale "memcpy bursts mis-read this PSRAM" warning; the bursts are hardware-proven by post-flash CRC verification. Tests: existing progress_cols cases retuned for the 16-column upload segment; new cases cover segment layout and the commit-fill math (endpoints, monotonic 4 KB sweeps over indivisible totals, degenerate inputs). 18 cases pass via `pixi run test`. Fleet-validated on the live 20-panel arena: three full batches were flashed with this firmware on the receiving side (bar sweep, commit animation, smiley/sad-smiley behavior), every panel CRC MATCH, verify phase ~52 ms end to end.
Two sources of spurious PE02/PE03 error glyphs: - The controller's CS lines each gate two panels (one per SPI bus), and single-panel ops like ISP clock only the target's bus, so the bus-paired panel (180 degrees opposite in the ring) sees ~1100 clockless CS pulses per flash. Production builds treated the resulting 0-byte reads as messages and raised PE02/PE03. - At arena power-on the controller's pins are high-Z until its SPI init runs, so floating CS/SCK can drift across logic thresholds and clock junk transactions into the panels. Both are fixed panel-side: - Messenger::update() returns on a 0-byte read in ALL builds (previously an SPI_DIAG-only path for the CS-idle timeout): a clockless envelope carries no data and is not a message. Real data loss is still caught controller-side (page CRC + CIPO confirmation). - The CS and SCK inputs get internal pull-ups so an undriven bus parks at its idle state (CS high: the PL022 ignores everything; SCK high: MODE3 idle level). A companion controller fix (Arena-Firmware) preloads the CS output registers HIGH before pinMode(OUTPUT), removing the boot-time CS-low pulse. Hardware-confirmed for the flash-time case: a full 20-panel batch with this build on the fleet showed no opposite-panel glyphs. The power-on case awaits confirmation across a few arena power cycles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #27 (which stacks on #26); this PR adds the two commits on top of
isp-indicator-review.Whole-process ISP progress bar + burst-CRC verify (74b6bc8)
push_progress_if_changedthrottle; segment constants and math live inisp_logic.hwith the other pure logic.burst_buf_+crc32_psram), replacing the per-byte XIP scan and its stale "memcpy bursts mis-read this PSRAM" warning; the bursts are hardware-proven by post-flash CRC verification.Ignore clockless CS envelopes; park the SPI bus at idle (a3a447f)
Two sources of spurious PE02/PE03 glyphs, both fixed panel-side:
Messenger::update()now returns on a 0-byte read in ALL builds (previously an SPI_DIAG-only path): a clockless envelope carries no data and is not a message. Real data loss is still caught controller-side (page CRC + CIPO confirmation).Test plan
pixi run test: 18 nativeisp_logiccases pass (existing progress cases retuned for the 16-column upload segment; new cases cover segment layout and commit-fill math: endpoints, monotonic 4 KB sweeps over indivisible totals, degenerate inputs). CI from ISP indicator hardening, host-side unit tests, PR/push CI #27 runs these on every PR.Companion changes
Companion changes
isp-poll-and-timing(reiserlab/LED-Display_G6_Firmware_Arena): the controller-side half of the boot glitch fix (preload CS output registers HIGH before pinMode(OUTPUT), removing the boot-time CS-low pulse), plus poll-based ISP receipts/liveness and split clocks/pacing that produce the ~3.2 s/panel reflash this bar is tuned against. - webDisplayTools ISP batch retries + total-first summary; poll-based ISP handoff refresh webDisplayTools#163: Arena Studio batch retries + total-first summary; its handoff doc describes this bar's segments and the sad-smiley failure signal.Closes #6