Skip to content

Bound a point's DCC address to the accessory space (#152) - #177

Merged
bazauto merged 1 commit into
mainfrom
fix/152-point-dcc-address-range
Aug 24, 2026
Merged

bazauto merged 1 commit into
mainfrom
fix/152-point-dcc-address-range

Conversation

@bazauto

@bazauto bazauto commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Closes #152.

points.dcc_address is an accessory address. The old check was
z.number().int().positive(), which happily accepts 9999 — a perfectly good loco address,
and an accessory address the command station will not transmit. Two numeric spaces that are
not the same one, and nothing said so.

The DCC accessory space ends at 2044, and PicoDCC enforces that at its parser's single
validation choke point: an address outside [1, 2044] means the packet is never sent. What
that costs is specific. setPoint resolves on the serial write either way, the route holds
the point lock believing the point moved, and with positionFeedback: 'none' on every live
point there is no confirmation channel to say otherwise — so a train is routed over a point
still lying the other way.

Why now

The issue was written when a bad address produced nothing at all. Two things have changed
since. bazauto/PicoDCC#47 made the station answer <X>, and #148 made the orchestrator act
on one: an <X> following an accessory command is now a point-command-rejected fault
against the route that issued it (docs/dcc-link.md D5). That half of #152 is already
landed
— it arrived with #148 rather than here, and this PR does not touch it.

What is left is the half that stops the <X> being generated at all. The route fault catches
this late and at the worst possible moment: a fault raised while a train is running is an
expensive way to learn that somebody typed 20450 into a config form last Tuesday. Both halves
stand. This one refuses the value where it was entered.

What lands

services/validation.ts gains dccAccessoryAddressSchema — z.number().int().min(1).max(2044)
— on pointCreateSchema, pointUpdateSchema and pointRowSchema. A bad value on
POST/PUT .../points is an ordinary 400, which is the existing convention for a malformed
operator UI request and not a Safe-Stop.

The Configure → Points form mirrors the bound: min/max on the create input, and a named
DCC address must be 1–2044 on both the create form and the inline EditableCell edit,
rather than a generic 400 coming back. Affordance, not authorisation — the backend stays the
authority, exactly as with role.

No CHECK constraint, and no migration

The issue asked for one. It is deliberately not here, following DD9's call on
sensors.in_service and B9's on blocks.length_mm: a CHECK on an existing SQLite table
forces drizzle-kit to emit a table rebuild — a DROP TABLE points — against a database
deployed to a live layout that cannot be reset.

I checked what the constraint would be guarding before deciding. The live layout holds six
points on addresses 11–16 (read with its -wal, on the bench box), and the dev database
holds the same. Nothing has ever written an out-of-range value, because the only writer is
the route that now rejects one.

The half worth having is covered instead by putting the bound on pointRowSchema. That is
the read path, so a row written before this bound existed, or edited outside the app, throws
PointRowInvalidError on the way out of the database rather than reaching setPoint.
It is parsePointRow's stated posture already — a row in the database is either valid or it
is corruption, and corruption must throw.

Also refused: validating in formatSetPoint or SerialDccAdapter.setPoint.
domain/dccWireFormat.ts formats and never validates (#147), and by the time an address
reaches it the configuration error is already several layers deep.

The sub-address is untouched. SerialDccAdapter.setPoint hardcodes 0, which is inside the
valid 0–7 range, so there is nothing to guard yet.

Tested

npm test from the repo root:

 Test Files  84 passed (84)
      Tests  1497 passed (1497)      # backend

 Test Files  27 passed (27)
      Tests  327 passed (327)        # frontend

npm run lint clean, and both workspace builds pass (tsc, and vite build for the
frontend).

New coverage: tests/unit/services/validation.test.ts takes parsePointRow,
pointCreateSchema and pointUpdateSchema over the boundaries — 1 and 2044 accepted, 0, -1,
2045, 20450 and 1.5 refused, and 9999 named explicitly, since that is the loco address
that motivated the whole thing. tests/integration/routes.test.ts asserts the 400 on both
POST and PUT, that the repository is never called, and that the boundary values still
create.

Docs

  • docs/dcc-link.md — D16, the decision: the bound, why the boundary rather than the
    reply, and the three refused alternatives above
  • docs/point-feedback.md — D10 gains the cross-reference, since that is where this table's
    "no CHECK constraint" position is recorded
  • docs/current-state.md — the DCC link section
  • CLAUDE.md — the index line, and one Traps entry, because a bound with no migration
    behind it is exactly the kind of thing that reads as an oversight
  • README.md — Next Milestones now names Per-loco speed step mode (128 / 28) #151 as what is left of the command-station work

docs/mqtt-contract.md is untouched and needed no amendment: nothing here changes a topic,
payload, QoS or retention setting.

points.dcc_address is an accessory address, valid on [1, 2044]. The old
z.number().int().positive() accepted 9999 -- a valid loco address, and an
accessory address PicoDCC will not transmit. An out-of-range command is
dropped by the station, but setPoint resolves on the serial write regardless
and the route holds the point lock believing the point moved.

dccAccessoryAddressSchema carries the bound on pointCreateSchema,
pointUpdateSchema and pointRowSchema. The read path is deliberate: there is
no CHECK constraint and no migration (DD9, B9 -- a rebuild of a live table to
guard a writer that does not exist), so the row schema is what catches a row
predating the bound, throwing PointRowInvalidError on the way out of the
database rather than reaching setPoint.

The Configure -> Points form mirrors the bound as affordance; the backend
stays the authority.

The other half of #152 -- acting on the <X> a bad address now draws -- landed
with #148 as the point-command-rejected route fault, and is untouched here.

Decision recorded as docs/dcc-link.md D16.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bazauto
bazauto merged commit a1d09ce into main Aug 24, 2026
1 check passed
@bazauto
bazauto deleted the fix/152-point-dcc-address-range branch August 24, 2026 21:12
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.

points.dcc_address is unvalidated: an out-of-range point command is silently dropped

1 participant