Write the web front end's shared half once - #69
Merged
Conversation
The front end was written for the ESP32 and then copied to the RP2350, and the copy showed: handleCommands was byte-identical in both backends down to its comments, and so were toUtf8, handleDisplay's body, the UTF-8 widening in broadcastLine, and broadcastFrame's rate limit, gather and compare. Even ChunkWriter - the thing that already separated what is written from where it goes - was defined twice. That mattered beyond tidiness. Both pages parse the catalog JSON, and each backend's web_test asserts on the bytes its own route wrote, never on what a page makes of them - so a divergence between two hand-maintained generators would have surfaced in a browser and nowhere else. WebFrontend now writes both documents, widens the lines, and owns the frame rate limit and the comparison. Each backend provides WebTransport.h, the way it already provides Pixels.h or Storage.h; that header names no framework type by value, because the firmware includes it and cannot promise the include order a backend's Arduino.h needs - and because httpd_handle_t is void* in the IDF and struct httpd_t* in the test stub, so a handle in it would not have compiled against both. Both classes are defined inside each WebInterface.cpp, next to the server handle and the client table they reach, which is why no new source file was needed. What stays per backend is what genuinely differs: starting a server, registering routes, checking a password - the ESP32 decodes the header itself because its core cannot decode base64, the RP2350 lets its library compare - and the update, which writes a spare partition on one and a file plus a loader command page on the other. Counted honestly this is close to line-neutral: 832 code lines across the two backends become 639 there plus 195 in the firmware and 114 of seam. The win is one generator instead of two, and the seam that lets the simulator serve these pages later. Measured: all four targets build warning-free. The AVR-Dx image is byte-identical to master at 48078/1700 - WebFrontend.cpp is compiled there and guarded to nothing, which is what proves the switch does not leak. Both host suites pass. The panel was driven in a browser against the rebuilt firmware: 14 of 111 cells lit, "ES IST HALB ZWOELF", brightness read back over the socket. Two small pre-existing finds fixed on the way through: WEB_INTERFACE_MAX_CLIENTS on the RP2350 sized a constant nothing read, since AsyncWebSocket owns the list - removed rather than left documenting a limit nothing enforces; and the two macros that moved are named WEB_FRONTEND_* now, after the module that owns them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Stage A+B of lifting the web front end into
firmware/. Routes, asset serving and dispatch (C) and the simulator's transport (D) are deliberately not here.What was wrong
The front end was written for the ESP32 and copied to the RP2350.
handleCommandswas byte-identical in both, comments included; so weretoUtf8,handleDisplay's body,broadcastLine's UTF-8 widening, andbroadcastFrame's rate limit / gather / compare.ChunkWriter— the thing that already separated what is written from where it goes — was defined twice.The risk that made this worth doing is not tidiness. Both pages parse the catalog JSON, and each
web_test.cppasserts on the bytes its own route wrote, never on what a page makes of them. A divergence between two hand-maintained generators would have surfaced in a browser and nowhere else.The seam
No new abstraction style: the repo's existing contract is a header the core reaches by name, provided per platform, with concrete types —
Pixels.h,Storage.h,System.h.WebTransport.hjoins that table inplatform/avr-dx/README.md.Two implementation findings changed the shape from the approved plan, both for the better:
WebTransport.hnames no framework type by value. It cannot: the firmware includes it and cannot promise the include order each backend'sArduino.hneeds. It forward-declaresstruct httpd_req/AsyncWebServerRequestinstead. This also dodged a trap —httpd_handle_tisvoid*in the IDF butstruct httpd_t*in the test stub, so a handle in that header would not have compiled against both.WebTransport.cppwas needed. Both classes are defined inside eachWebInterface.cpp, beside the server handle and client table they reach. Two fewer files than planned.What stays per backend
Starting a server, registering routes, the password (the ESP32 decodes the header itself because its core cannot decode base64; the RP2350 lets its library compare), and the update — a spare partition on one, a file plus a PicoOTA command page on the other.
Line count, honestly
Roughly neutral, not a win: 832 code lines across the two backends become 639 there, plus 195 in the firmware and 114 of seam. I estimated "~250 duplicated lines gone" when proposing this; that was true of the duplication but not of the total, because the seam costs about what the duplication saved. The win is one generator instead of two, and the seam stages C/D need. The line deletion comes in D, when
serve.js(226 lines),webhost.cpp(145) and the fakedhttpd_*layer go.Verified
WebFrontend.cppis compiled there (the.objexists) and guarded to nothing — that identity is what provesWEB_FRONTEND_SUPPORTdoes not leak.-Wall -Wextra -Werror.tools/documented-sizes.py --elf … --rp2350-elf …passes, all four checks.One structural consequence worth knowing:
WebFrontend.cppis excluded from theCOREglob in bothrun.shand picked up with the backend's own web source, becauseserial_testandds3231_testdeliberately do not linkWebInterface.cpp— the same split those files already made.Two pre-existing finds fixed in passing
WEB_INTERFACE_MAX_CLIENTSon the RP2350 sized a constant nothing read —AsyncWebSocketowns the list. Removed rather than left documenting a limit nothing enforces.WEB_FRONTEND_*now, after the module that owns them.One find NOT fixed here
platform/esp32/README.mdcontradicts itself and is stale: its "Verification status" block saysFlash: 17.5% of 3.2 MB(≈585 KB) while the same file's "Which ESP32 this needs" says "the image is about 1.05 MB" — and the build reports 1 100 421 bytes, i.e. 32.9%. RAM likewise reads 10.8% against an actual 15.9%. This predates this branch by a wide margin (nothing here adds 500 KB) and the ESP32's figures are the onesdocumented-sizes.pydoes not check. Left alone deliberately — it is a separate change, and it suggests the script should grow an ESP32 check rather than the line being hand-corrected again.🤖 Generated with Claude Code