fix(uci): bounded uci_wait_idle + connect short-read detection + Labels.from_file parser convergence

CLAUDE.md

@@ -139,6 +139,29 @@ is **$C9**. See `src/net/uci/uci_regs.inc` for the full equate list
 `uci_read_data`, etc. No zero-page usage — all absolute addressing
 and self-modifying code.
 
+`uci_wait_idle` is wall-clock-bounded (5 s budget via CIA1 TOD) per
+the design note below. On timeout it returns C=1 with `net_last_error
+= UCI_ERR_WAIT_TIMEOUT`. The four callers (`net_dhcp_acquire`,
+`net_tcp_connect`, `net_tcp_send`, `net_tcp_close`) all `bcs` out to
+surface the failure rather than letting the C64 hang indefinitely on
+a wedged FPGA. `uci_push_wait` and `uci_end_cmd` are still unbounded
+and should be converted to the same TOD pattern if a wedge there is
+ever observed.
+
+### UCI error codes
+
+`src/net/uci/uci_errors.inc` enumerates the values surfaced via
+`net_last_error` and `net_tcp_state`. The most load-bearing:
+
+ - `UCI_ERR_NO_SOCKET` — `net_tcp_connect` got a short-read on the
+ OPEN_TCP response (no socket-id byte), so the firmware never
+ actually opened the TCP connection. `net_tcp_state` is set to
+ `UCI_TCP_CONNECT_FAIL` and C=1 is returned. Without this check
+ the C64 would commit to a phantom socket and push TLS bytes
+ into nowhere (see issue #36).
+ - `UCI_ERR_WAIT_TIMEOUT` — `uci_wait_idle` exhausted its 5 s budget
+ (see issue #37).
+
 ### DNS
 
 UCI firmware resolves hostnames internally during `TCP_CONNECT`. There
@@ -360,16 +383,24 @@ touching TLS call sites.
 
 ### Design note — bounded timeouts must use wall-clock time
 
-Any future robustness work on the UCI adapter's spin-wait helpers
-(`uci_wait_idle`, `uci_push_wait`, etc.) MUST use a wall-clock time
-source — CIA timer on stock C64, TOD clock on U64E — rather than a
-cycle-counted iteration budget. The fences around every UCI register
-access make per-iteration cost scale with CPU speed: a budget that is
-ample at 1 MHz collapses to far too short at 48 MHz because turbo
-scales CPU cycles but not the FPGA's wire-level operation durations.
-A prior attempt on branch `feat/net-drain-abi` split waits into
-fast/long tiers with cycle-count budgets and broke DHCP at turbo for
-exactly this reason; the branch was abandoned.
+Robustness work on the UCI adapter's spin-wait helpers (`uci_wait_idle`,
+`uci_push_wait`, etc.) MUST use a wall-clock time source — CIA timer
+on stock C64, TOD clock on U64E — rather than a cycle-counted iteration
+budget. The fences around every UCI register access make per-iteration
+cost scale with CPU speed: a budget that is ample at 1 MHz collapses
+to far too short at 48 MHz because turbo scales CPU cycles but not the
+FPGA's wire-level operation durations. A prior attempt on branch
+`feat/net-drain-abi` split waits into fast/long tiers with cycle-count
+budgets and broke DHCP at turbo for exactly this reason; the branch
+was abandoned.
+
+`uci_wait_idle` is the first helper to follow this pattern (issue #37).
+At entry it samples CIA1 TOD ($DC08-$DC0B) — read order is HOUR
+(latch) → MIN → SEC → TENTHS (unlatch) — and on each spin pass re-reads
+TENTHS, bailing with C=1 + `net_last_error = UCI_ERR_WAIT_TIMEOUT`
+after 50 transitions (~5 s wall-clock, independent of CPU turbo). State
+lives in two SMC bytes inside the routine to match the file's no-ZP
+convention. Use this as the template for any future bounded helper.
 
 ## Memory layout
 

src/net/uci/net.s

@@ -313,6 +313,7 @@ net_poll:
 ; =============================================================================
 net_dhcp_acquire:
  jsr uci_wait_idle
+ bcs @dhcp_wait_to ; FPGA wedged — bail with C=1
 
  lda #UCI_TARGET_NETWORK
  jsr uci_begin_cmd
@@ -332,6 +333,7 @@ net_dhcp_acquire:
 
  lda #UCI_ERR_CMD_FAILED
  sta net_last_error
+@dhcp_wait_to:
  sec
  rts
 
@@ -399,6 +401,14 @@ net_tcp_connect:
  stx uci_connect_port_hi
 
  jsr uci_wait_idle
+ bcc :+
+ ; FPGA wedged before we even queued anything — surface the timeout
+ ; (net_last_error already set) with the connect-fail tcp_state.
+ lda #UCI_TCP_CONNECT_FAIL
+ sta net_tcp_state
+ sec
+ rts
+:
 
  lda #UCI_TARGET_NETWORK
  jsr uci_begin_cmd
@@ -446,6 +456,10 @@ net_tcp_connect:
 
 @tc_no_err:
  ; Read 1-byte socket_id response.
+ ; Pre-zero uci_socket_id so a short-read leaves a known sentinel
+ ; (uci_read_resp_bytes only writes the bytes it actually receives).
+ lda #$00
+ sta uci_socket_id
  lda #<uci_socket_id
  sta uci_resp_dst
  lda #>uci_socket_id
@@ -458,11 +472,29 @@ net_tcp_connect:
  jsr uci_drain_status
  jsr uci_ack
 
+ ; Validate the response: firmware must have returned at least 1
+ ; byte (uci_resp_count) AND a non-zero socket_id. Issue #36 — at
+ ; least one observed U64E firmware path returns no payload while
+ ; clearing the error bit, leaving uci_socket_id = 0 and us writing
+ ; into a phantom socket. Convert that into a clean failure.
+ lda uci_resp_count
+ beq @tc_no_socket
+ lda uci_socket_id
+ beq @tc_no_socket
+
  lda #UCI_TCP_CONNECTED
  sta net_tcp_state
  clc
  rts
 
+@tc_no_socket:
+ lda #UCI_ERR_NO_SOCKET
+ sta net_last_error
+ lda #UCI_TCP_CONNECT_FAIL
+ sta net_tcp_state
+ sec
+ rts
+
 ; =============================================================================
 ; net_tcp_send — push up to net_send_len bytes from (AX) through SOCKET_WRITE.
 ;
@@ -519,6 +551,12 @@ net_tcp_send:
 
 @begin_chunk:
  jsr uci_wait_idle
+ bcc :+
+ ; FPGA wedged mid-send — surface as send-fail (net_last_error already
+ ; set to UCI_ERR_WAIT_TIMEOUT inside uci_wait_idle).
+ sec
+ rts
+:
 
  lda #UCI_TARGET_NETWORK
  jsr uci_begin_cmd
@@ -646,6 +684,13 @@ net_tcp_send:
 ; =============================================================================
 net_tcp_close:
  jsr uci_wait_idle
+ bcc :+
+ ; FPGA wedged on close — force CLOSED state and bail. Best-effort
+ ; semantics already match the existing close path (no return code).
+ lda #UCI_TCP_CLOSED
+ sta net_tcp_state
+ rts
+:
 
  lda #UCI_TARGET_NETWORK
  jsr uci_begin_cmd

src/net/uci/uci_cmd.s

@@ -26,6 +26,10 @@
 ; one interface-index parameter). Later phases will extend as needed.
 
 .include "uci_regs.inc"
+.include "uci_errors.inc"
+
+; net_last_error lives in net.s's BSS — we set it on wait timeout (#37).
+.import net_last_error
 
 .export uci_abort
 .export uci_wait_idle
@@ -61,19 +65,69 @@ uci_abort:
  rts
 
 ; =============================================================================
-; uci_wait_idle — spin until STATE==0 AND CMD_BUSY==0
+; uci_wait_idle — spin until STATE==0 AND CMD_BUSY==0, with wall-clock cap
 ; UCI_STAT_STATE ($30) covers the state field; CMD_BUSY ($01) is bit 0.
 ; ORing them (MASK $31) and looping while nonzero gives "fully idle".
+;
+; Issue #37 — the historical unbounded spin converts an FPGA wedge into a
+; 600 s test sentinel timeout. The cap below uses CIA1 TOD (CIA_TOD_TENTHS,
+; ticks at 10 Hz) — the only clock that runs at the same wall-clock rate
+; regardless of CPU turbo speed. Cycle-counted budgets do NOT work here:
+; the per-iteration cost scales with turbo (each fence is ~38 us of FPGA
+; wall time but only a few CPU cycles at 48 MHz), so a budget tuned at
+; 1 MHz collapses at 48 MHz (and vice versa). A prior attempt on
+; feat/net-drain-abi shipped cycle-counted budgets and broke turbo DHCP
+; for exactly this reason.
+;
+; CIA TOD read protocol: reading the HOUR register latches the four
+; registers atomically; reading the TENTHS register unlatches them.
+; We only need TENTHS for our 5-second budget, but we still latch+unlatch
+; properly so we don't disturb other code that might be reading TOD.
+;
+; Budget: UCI_WAIT_IDLE_BUDGET_TENTHS (50 = 5 s). On expiry: set
+; net_last_error = UCI_ERR_WAIT_TIMEOUT, return C=1.
+;
+; Output: C=0 on idle, C=1 on timeout.
 ; Clobbers: A
 ; =============================================================================
+CIA_TOD_TENTHS = $DC08
+CIA_TOD_HOUR = $DC0B
+UCI_WAIT_IDLE_BUDGET_TENTHS = 50 ; 5 seconds at 10 Hz
+
 uci_wait_idle:
+ ; Sample initial TENTHS for delta-tracking. Latch via HOUR,
+ ; release via TENTHS. We don't care about the HOUR value itself.
+ lda CIA_TOD_HOUR
+ lda CIA_TOD_TENTHS
+ sta @wi_last_tenths
+ lda #$00
+ sta @wi_elapsed
+@wi_loop:
  lda UCI_STATUS
  uci_fence ; settle read before testing bits
  and #(UCI_STAT_STATE | UCI_STAT_CMD_BUSY) ; $31
  beq @idle_done
- jmp uci_wait_idle ; long branch: fence too wide for BNE
+
+ ; Check TOD for elapsed tenths. Latch (HOUR) then read TENTHS.
+ lda CIA_TOD_HOUR
+ lda CIA_TOD_TENTHS
+ cmp @wi_last_tenths
+ beq @wi_loop ; no change — keep spinning
+ sta @wi_last_tenths
+ inc @wi_elapsed
+ lda @wi_elapsed
+ cmp #UCI_WAIT_IDLE_BUDGET_TENTHS
+ bcc @wi_loop ; under budget — continue
+ ; Timeout
+ lda #UCI_ERR_WAIT_TIMEOUT
+ sta net_last_error
+ sec
+ rts
 @idle_done:
+ clc
  rts
+@wi_last_tenths: .byte 0
+@wi_elapsed: .byte 0
 
 ; =============================================================================
 ; uci_wait_not_busy — spin until CMD_BUSY==0 (ignore STATE)

src/net/uci/uci_errors.inc

@@ -12,11 +12,14 @@ UCI_ERR_CONNECT_FAIL = $84 ; TCP_CONNECT returned an error bit
 UCI_ERR_SEND_FAIL = $85 ; SOCKET_WRITE returned an error bit
 UCI_ERR_READ_FAIL = $86 ; SOCKET_READ returned an error bit
 UCI_ERR_SHORT_WRITE = $87 ; SOCKET_WRITE wrote fewer bytes than requested
+UCI_ERR_NO_SOCKET = $88 ; TCP_CONNECT response yielded no socket_id
+UCI_ERR_WAIT_TIMEOUT = $89 ; uci_wait_idle exceeded its wall-clock budget
 
 ; TCP state values stored in net_tcp_state
 UCI_TCP_CLOSED = $00 ; no active socket
 UCI_TCP_CONNECTED = $01 ; connected, reads/writes valid
 UCI_TCP_ERROR = $02 ; saw an error on a read — stop polling
+UCI_TCP_CONNECT_FAIL = $03 ; TCP_CONNECT did not yield a usable socket
 
 ; UCI firmware data queue max per SOCKET_WRITE push (see uci_network.py)
 UCI_DATA_QUEUE_MAX = 800

tools/uci/_analyze_ecdsa_trace.py

@@ -21,6 +21,8 @@
 from collections import Counter
 from pathlib import Path
 
+from c64_test_harness import Labels
+
 
 REPO_ROOT = Path(__file__).resolve().parents[2]
 DEFAULT_BASE = Path("/tmp/ecdsa_debug")
@@ -32,14 +34,9 @@ def _load_labels() -> dict[int, str]:
  if not labels_file.is_file():
  return {}
  out: dict[int, str] = {}
- for line in labels_file.read_text().splitlines():
- parts = line.split()
- if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."):
- name = parts[2][1:]
- _, hex_addr = parts[1].split(":", 1)
- a = int(hex_addr, 16)
- # Prefer the first (public) label if multiple map to same addr
- out.setdefault(a, name)
+ for name, a in Labels.from_file(labels_file).items():
+ # Prefer the first (public) label if multiple map to same addr
+ out.setdefault(a, name)
  return out
 
 

tools/uci/phase2_check.py

@@ -31,6 +31,7 @@
 import time
 from pathlib import Path
 
+from c64_test_harness import Labels
 from c64_test_harness.backends.device_lock import DeviceLock
 from c64_test_harness.backends.ultimate64 import Ultimate64Transport
 from c64_test_harness.backends.ultimate64_client import Ultimate64Client
@@ -74,14 +75,10 @@ def load_label(name: str) -> int:
  Entries look like: `al C:BC4B .net_local_ip` — the `.name` token
  is unambiguous across backend cfgs.
  """
- token = f".{name}"
- for line in LABELS_PATH.read_text().splitlines():
- parts = line.split()
- if len(parts) >= 3 and parts[0] == "al" and parts[2] == token:
- addr_tok = parts[1] # "C:BC4B"
- _, hex_addr = addr_tok.split(":", 1)
- return int(hex_addr, 16)
- raise KeyError(f"label {name!r} not found in {LABELS_PATH}")
+ labels = Labels.from_file(LABELS_PATH)
+ if name not in labels:
+ raise KeyError(f"label {name!r} not found in {LABELS_PATH}")
+ return labels[name]
 
 
 def is_plausible_private(ip: tuple[int, int, int, int]) -> bool:

tools/uci/phase3_tcp_echo.py

@@ -35,6 +35,7 @@
 import time
 from pathlib import Path
 
+from c64_test_harness import Labels
 from c64_test_harness.backends.device_lock import DeviceLock
 from c64_test_harness.backends.ultimate64 import Ultimate64Transport
 from c64_test_harness.backends.ultimate64_client import Ultimate64Client
@@ -107,14 +108,7 @@ def _run_echo_server(bind_ip: str, port: int, result: dict) -> None:
 
 
 def _load_labels() -> dict[str, int]:
- labels: dict[str, int] = {}
- for line in LABELS_PATH.read_text().splitlines():
- parts = line.split()
- if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."):
- name = parts[2][1:]
- _, hex_addr = parts[1].split(":", 1)
- labels[name] = int(hex_addr, 16)
- return labels
+ return dict(Labels.from_file(LABELS_PATH))
 
 
 def _build_test_routine(labels: dict[str, int], host_ip: str, port: int) -> bytes:

tools/uci/test_http_live.py

@@ -18,6 +18,7 @@
 import time
 from pathlib import Path
 
+from c64_test_harness import Labels
 from c64_test_harness.backends.device_lock import DeviceLock
 from c64_test_harness.backends.ultimate64 import Ultimate64Transport
 from c64_test_harness.backends.ultimate64_client import Ultimate64Client
@@ -44,14 +45,7 @@
 
 
 def _load_labels() -> dict[str, int]:
- labels: dict[str, int] = {}
- for line in LABELS_PATH.read_text().splitlines():
- parts = line.split()
- if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."):
- name = parts[2][1:]
- _, hex_addr = parts[1].split(":", 1)
- labels[name] = int(hex_addr, 16)
- return labels
+ return dict(Labels.from_file(LABELS_PATH))
 
 
 def _build_http_routine(labels: dict[str, int], hostname_len: int, port: int) -> bytes:

tools/uci/test_http_local.py

@@ -26,6 +26,7 @@
 import time
 from pathlib import Path
 
+from c64_test_harness import Labels
 from c64_test_harness.backends.device_lock import DeviceLock
 from c64_test_harness.backends.ultimate64 import Ultimate64Transport
 from c64_test_harness.backends.ultimate64_client import Ultimate64Client
@@ -97,14 +98,7 @@ def _run_http_server(bind_ip: str, port: int, result: dict) -> None:
 
 
 def _load_labels() -> dict[str, int]:
- labels: dict[str, int] = {}
- for line in LABELS_PATH.read_text().splitlines():
- parts = line.split()
- if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."):
- name = parts[2][1:]
- _, hex_addr = parts[1].split(":", 1)
- labels[name] = int(hex_addr, 16)
- return labels
+ return dict(Labels.from_file(LABELS_PATH))
 
 
 def _build_http_routine(labels: dict[str, int], port: int) -> bytes: