tools/bench: host-side scripts for driving a TeensyROM, and CI that runs them - #35
Merged
Merged
Conversation
…erial Reflash without the program button (SD push + remote launch + the C64's Y/N answered by DMA), push files, launch, peek the C64's memory, read the screen, queue a keypress, and launch an extension and report how it ended. They share one small serial library and are covered by tests against a fake board on a pty. These replace a set of one-off scripts that each carried its own copy of the port code and a hardcoded device name.
They arrived from a branch that also carried the extension loader, so their references point at files main does not have: tools/flash-firmware.mjs, and vm/abi/README.md for the failure record. Point at docs/General_Usage.md and tools/Debug/ instead, so the next reader finds the other flashing routes and knows why there are two serial stacks in this tree. The extension image these docs describe is PR SensoriumEmbedded#31's, and so are the build/extensions/ paths in the hello recipe; say so rather than leaving a reader hunting for them. Importing trlink leaves a __pycache__ behind, so ignore it.
…mware The scripts carried byte strings inline -- b'\x64\xDE' for a directory listing, b'\xcc\x64' for an Ack -- and the fake board in the tests carried its own copy of each. Two copies of a wire value is how they drift, and neither copy says which firmware constant it came from. protocol.py now holds every value that goes on the wire and c64.py every machine address, each named as the firmware names it. test_protocol.py reads those names back out of Source/Teensy/ and fails if any pair has drifted, and a second test fails if protocol.py grows a value that mapping does not cover. The fake board keeps its own byte-order encoding, so it and the code under test still cannot agree on a wrong answer. The commands and their bytes are unchanged, with one exception: colors.py reads the 17 VIC registers it prints rather than 32.
Three commands the firmware has always answered and nothing here used. FWCheckToken is the designed way to tell the main image from the minimal one, and both answer it whatever else they are doing; probe.py now asks it instead of sending 'b' and reading the reply, which could not tell the two apart and depended on a debug command minimal does not have. It reads past anything the board says unprompted -- IOHandlers.ino:51 prints a line whenever a handler loads, which is exactly what a reset or a launch causes -- and still treats silence as the extension image or a hung board, exiting non-zero. ls.py closes the loop after push.py: until now nothing here could see what was on the card. A listing that stops before its end marker is refused rather than returned short, since a truncated listing would report a file that is there as missing. The firmware pages at the take value it was given, so ls.py says when it got a full page. reset.py is the way back to the menu, and out of the minimal image, without reaching for the C64's keyboard. Two fixes to the sites these share rather than to the new code alone: raw() and stream() both spun until their deadline on a closed port, because os.read returns b'' at EOF while select keeps reporting the port readable -- measured at 2.00s wall and 0.84s CPU against a 0.3s idle, now 0.31s and 0.00s -- and stream() reported that hangup as no drop, the opposite of what it promises. drive_number() replaces the bare int() that launch.py and ls.py both used, so a mistyped drive is a message rather than a traceback.
Answering the C64's Y/N prompt is the step that silently does nothing when the screen is not what fwupdate expects. The board then keeps running the old firmware, the updater's serial output never appears, and nothing said so: the symptom is a board that looks flashed and is not. hexfile.py decodes the Intel HEX records, takes the region the main image links into, and pulls GCC's __DATE__ and __TIME__ out of it. That pair is the second line of the version banner, so fwupdate can read the banner back after the reboot and compare it with the stamp in the hex it pushed. The stamp alone would not be enough. This build pins SOURCE_DATE_EPOCH, so both images in a hex it produces carry the same stamp, and the minimal image prints it in the same form; a board that fell back to minimal would match the stamp it was asked for. So the firmware check has to answer 'main' as well, and it answers without reading the hex. A file this reader cannot get data records out of is refused before anything is pushed; one it can, but cannot get a stamp from, is flashed and still checked for the main image, with only the stamp comparison skipped. Reconnecting after the reboot cannot assume the old device node: reflashing a TR+ moved it from cu.usbmodem192885901 to cu.usbmodem2101. reconnect prefers the port it was given while that node exists, and otherwise takes one that appeared beside it. On that board the port was away 1.34s and answered the firmware check cleanly 5.37s after returning, printing boot chatter in between, so await_image gives the chatter and the answer one deadline. The flash bounds come from the FLASH lines of tools/BootLinkerFiles/ imxrt1062_t41.ld.orig and .ld.upper; test_hexfile.py reads those and fails if either origin moves.
The README says macOS or Linux and the scripts use only termios, but port discovery globbed /dev/cu.usbmodem*, which is a BSD name. On Linux a Teensy comes up through the CDC-ACM driver as /dev/ttyACM*, so find_port found nothing and said so by naming a pattern that does not name a Linux node. Three of those sentences also said the port name tells the three images apart. The macOS node carries the USB serial number, which is what makes that work; the sentences now say so rather than claiming it of every platform. The tests did not catch it because they set TR_PORT to a pty they made, which skips discovery entirely. The new test drives ports() over a directory holding both node names plus tty.usbmodem2101, ttyS0 and cu.Bluetooth, and fails against the old single glob.
build-c64.test.mjs reassembles the C64 projects and byte-compares the result against the committed ROM headers, which is the only thing that catches a .asm change landing without its regenerated .prg.h. It skipped in CI, because it skips when ACME is not on PATH and no job installed one. It also named three projects of the twelve. The new tools job installs ACME and runs both suites: npm test, which now asserts rather than skips when CI is set, and the Python bench tests, which nothing ran before. The project list comes from loadProjects, less the one project KickAssembler builds, so a project added to the manifest is covered without touching the test. That takes the header comparison from SettingsMenu, TODCheck and BASIC to the whole manifest bar the KickAssembler one, and the test now also fails if the build writes a different set of headers than the manifest declares: corrupting a byte of ASIDPlayer.prg.h now fails the test, where before it passed. The job carries no `needs:`, so a compile failure still reports test results and a test failure still reports whether the firmware builds. npm test moves out of the build matrix, where it ran once per target without an assembler. That move would otherwise have loosened a release gate: a red tool test used to fail build and so skip collect, which owns the publish step, and collect now waits on both jobs to keep that true. ACME is deliberately unpinned here: tools/check-pins.mjs tracks the ACME pin sites and reads this file for other pins, so a version named in the workflow would drift out of that check silently. Ubuntu's package assembles the committed headers byte for byte, which is the guarantee that matters and the one a version string cannot give. Three sentences said this checking did not exist. Source/C64/README.md and docs/Architecture/Build-System.md described the gap this commit closes. CONTRIBUTING.md's "There's no hosted CI" was wrong when it was written: build.yml arrived in 31a920b and that sentence in 4bd8f52.
wr() retried os.write on BlockingIOError with no deadline, while rd, drain, stream and reconnect each carry one. Against a pty nobody reads, a 4 MB write was still spinning at 20 s; a no-progress bound turns a board that has stopped draining its endpoint into "board stopped reading after 1024 of 4194304 bytes" at 30 s. The bound is a `wr` argument so a test can use a short one. post() opened the local file bare, so push.py on a missing path printed a FileNotFoundError traceback where every other user error in these scripts is a one-line SystemExit. petscii_row's docstring claimed letters survive. Screen codes 65-90, which are A-Z in the lower/uppercase charset, decode to '.'. The docstring now names the charset it reads, a test pins the limit and the README's Limits section carries what it costs fwupdate.py; the decoder is unchanged.
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.
Host-side scripts for driving a real TeensyROM over USB serial: reflash it,
push files, launch things, list the card, read and poke the C64's memory.
Python 3, standard library only.
They were written to verify firmware with nobody at the machine, and they
replace a set of one-off scripts that each carried their own copy of the port
code and a hardcoded device name.
tools/Debug/keeps the scope andbus-timing scripts, which are a separate stack on pyserial.
What is here
on
trlink.py(the link),protocol.py(every wire value) andc64.py(memory locations and screen codes).
No magic numbers
protocol.pyholds every value that goes on the wire, named as the firmwarenames it, and
test_protocol.pyreads those names back out ofSource/Teensy/and fails if any pair has drifted. So a moved token fails a test rather than a
board, and nothing else in the package restates a byte string.
Tests, without hardware
python3 -m unittest discover -s tools/benchruns a fake board on a pty. Itkeeps its own byte-order encoding, so it and the code under test cannot agree
on a wrong answer.
CI, and one thing it was not checking
Nothing ran these tests, so the last commit adds a
toolsjob for the suitesthat need no Teensy toolchain. While adding it, a gap turned up that is worth
more than the bench scripts are:
tools/build-c64.test.mjsreassembles theC64 projects and byte-compares against the committed
.prg.hheaders, whichis the only thing standing between a
.asmchange and firmware built from astale header -- and it was skipping in CI, because it skips when ACME is not
on PATH and no job installed one. It also named three projects of twelve.
The job installs ACME, the test now derives its list from the project manifest
and asserts rather than skips under CI, and it fails if the build writes a
different set of headers than the manifest declares. Corrupting a byte of
ASIDPlayer.prg.hnow fails; before, it passed.TRCustomBasicCommandsstaysuncovered: KickAssembler needs a JRE.
collectnow waits on both jobs. It owns the release publish, and movingnpm testout of the build matrix would otherwise have let aRelease_v*tagship over a red header comparison.
ACME is deliberately unpinned in the workflow:
tools/check-pins.mjstracksfive ACME pin sites and reads
build.ymlfor other pins, so a version namedthere would be a sixth that its cross-file check never sees. The byte
comparison is the guarantee, which is what a version string could not give.
Reviewed, and what that found
Every commit here was reviewed on its own before it was pushed, including the
first, which is an unmodified cherry-pick of work that arrived from elsewhere
and had never been examined. Reviewing it turned out to be worth it -- see the
last bullet. Worth naming the findings, because each was a silent failure in
the direction that matters:
listdir()returned a truncated listing as a complete one when the boardwent quiet part way through, so a tool whose job is "did my push land?"
would have reported a file that is there as missing.
fwcheck()died on theLoading IO handler:line the firmware printsunprompted -- which a reset or a launch causes, so
probe.pyfailed in theone situation it exists for.
assert.ok(built.length > 0)in the CI commit itself passed with fifteenof sixteen headers written, because the comparison loop only iterates what
the build produced. It now compares against the set the manifest declares.
wr()retriedos.writeonBlockingIOErrorwith no deadline, while everyneighbouring method carries one. A 4 MB write to a reader that had gone away
was still spinning when killed at 20s; it now gives up after 30s of no
progress and says how far it got. Found in the cherry-picked commit.
Not covered
The fake board proves framing and byte order, not what a real board does.
exttest.pydescribes the extension image, which arrives with PR #31; theother two images are here today.
The bench tests need macOS or Linux --
termiosandpty.This touches
.github/workflows/build.yml, which PR #31 also touches; thatbranch needs a one-line rebase to move its
verify:extensionsstep into thenew job once this lands.