Skip to content

fix(tr): surface firmware NAK text on launch/upload failures#44

Merged
kfox merged 1 commit into
mainfrom
fix/tr-launch-error-surfacing
Jun 25, 2026
Merged

fix(tr): surface firmware NAK text on launch/upload failures#44
kfox merged 1 commit into
mainfrom
fix/tr-launch-error-surfacing

Conversation

@kfox

@kfox kfox commented Jun 25, 2026

Copy link
Copy Markdown
Owner

What

When the TeensyROM NAKs a command, teensyrom_dma._expect_ack drained the firmware's trailing error text only to resync the stream, then discarded it — so a failed PostFile/LaunchFile surfaced just a bare FailToken (0x9B7F). This captures that text and puts it in the raised error:

  • "Busy!" (a program is running / the menu handler isn't active) → new TRBusyError (subclass of TRError), so callers can distinguish it from a hard failure.
  • Other reasons ("Not enough room", "File already exists.", …) → appended to the FailToken/RetryToken/unexpected-reply message.

The drain (resync) behavior is unchanged — we just stop throwing the reason away. Covered by ErrorSurfacingTest against the loopback transport. No launch-path behavior change.

Why (context)

This is the safe, salvaged part of a TR-launcher investigation. The original goal (reset-free relaunch for looping launchers) turned out not verifiable and was backed out. The investigation also uncovered a separate, pre-existing intermittent bug: the keyboard poller's ReadC64Mem racing the launcher's reset()+PostFile on the shared TR link can desync the stream so the upload drops a byte — the .prg then loads one byte short and the BASIC autostart stub ?SYNTAX ERRORs. It's a race (reliable single-threaded, and on the Ultimate which has no shared-link poll), so verifying a fix needs a soak harness; it's tracked as a known issue in docs/caveats.md, not fixed here. This error surfacing is what makes that failure diagnosable.

Test

make check (ruff + mypy --strict + pyright + 1588 tests) green.

…ilures

When the TeensyROM NAKs a command, _expect_ack drained the trailing error
text purely to resync the stream and then discarded it — so a failed
PostFile/LaunchFile surfaced only a bare "FailToken (0x9B7F)". Capture that
text and put it in the raised error:

  * "Busy!" (program running / menu handler inactive) -> new TRBusyError
    (subclass of TRError) so callers can tell it apart from a hard failure;
  * other reasons ("Not enough room", "File already exists.", ...) appended
    to the FailToken/RetryToken/unexpected-reply message.

The drain (resync) behavior is unchanged; we just stop throwing the reason
away. Unit-tested via the loopback transport (ErrorSurfacingTest).

Context: this is the salvaged, safe part of a TR-launcher investigation. The
investigation uncovered a separate, pre-existing INTERMITTENT bug — the
keyboard poller's ReadC64Mem racing the launcher's reset+PostFile on the
shared TR link can desync the stream and make the upload drop a byte (the
.prg then loads one byte short -> ?SYNTAX ERROR). It's a race (reliable
single-threaded and on the Ultimate), needs a soak harness to verify a fix,
and is tracked separately as a known issue (docs/caveats.md). This error
surfacing is what makes that failure diagnosable. No launch-path behavior
change here.
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.69%. Comparing base (1ad2588) to head (b8801a9).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
c64cast/teensyrom_dma.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #44   +/-   ##
=======================================
  Coverage   79.68%   79.69%           
=======================================
  Files          68       68           
  Lines       12943    12947    +4     
  Branches     1910     1911    +1     
=======================================
+ Hits        10314    10318    +4     
  Misses       2190     2190           
  Partials      439      439           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@kfox kfox merged commit 4e1762a into main Jun 25, 2026
8 checks passed
@kfox kfox deleted the fix/tr-launch-error-surfacing branch June 25, 2026 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant