fix(hal): enable Apex escrow, use the real return bit, checksum the whole frame

Three protocol corrections from Pyramid's spec (RS_232 Rev G), all of which
this driver had guessed at because it was written clean-room without it.

**Escrow was never enabled.** BYTE 1 bit 4 is an enable, and the driver left
it clear on every poll. With it clear the acceptor does not stop at escrow,
so the host is never offered the stack-or-return decision and notes are
banked before anything has validated them. The entire FSM here is built
around that decision point, so this was not a missing nicety — the driver's
central flow could not have happened. It is now asserted on every poll.

**Return used the wrong mechanism.** The driver expressed "give the note
back" by zeroing the enable mask mid-escrow, under an in-code assumption
that "Apex has no distinct return opcode". It has one: BYTE 1 bit 6. The old
approach was flagged in a comment as needing hardware verification; the spec
settles it instead. Note the spec also distinguishes Returning (host refused
a valid note) from Rejected (acceptor judged it invalid), which is the
distinction this bit exists to express.

**The checksum range was hardcoded to the host frame.** computeChecksum
always XORed bytes 1..5, which is right for the 8-byte poll and wrong for
the 11-byte reply, where it should span 1..8. So every reply failed
validation. That was masked by the check being non-fatal "pending hardware
verification", which logged a warning and parsed anyway.

The range is now derived from the frame length, and verified against the two
reset frames the spec spells out literally with their checksums — the only
ground truth available without hardware. Those same frames are now a test.

With the range confirmed, a mismatch becomes a hard drop rather than a
warning. A corrupt frame carries a denomination field, and crediting a note
from a frame known to be damaged is the one outcome worth refusing. The raw
bytes are logged so a systematic framing error stays diagnosable.

Also records two operational facts from the spec that were not written down:
the interface is Mars/MEI GL5-compatible (hence the resemblance to the EBDS
driver), and polls must not fall more than 5s apart or the acceptor may dump
an escrowed note and stop accepting until the host resumes. Our 100ms cadence
is comfortably inside that.

packages/hal/src/validators/apex/__tests__/apex-rs232.test.ts

@@ -6,6 +6,7 @@ import {
   parseStatus,
   parseResponse,
 } from '../apex-rs232.js'
+import { denomForChannel } from '../denominations.js'
 
 // Cassette-present bit; OR it into event bytes so parseStatus doesn't short to
 // 'stackerOpen'.
@@ -20,6 +21,27 @@ describe('Apex RS-232 protocol', () => {
     })
   })
 
+  describe('computeChecksum spans the frame, not a fixed range', () => {
+    it('matches the two reset frames the spec spells out literally', () => {
+      // Rev G gives these verbatim, checksum included, so they are the only
+      // ground truth available for the XOR range without hardware.
+      const a = Buffer.from([0x02, 0x08, 0x61, 0x7f, 0x7f, 0x7f, 0x03, 0x16])
+      const b = Buffer.from([0x02, 0x08, 0x60, 0x7f, 0x7f, 0x7f, 0x03, 0x17])
+      expect(computeChecksum(a)).toBe(0x16)
+      expect(computeChecksum(b)).toBe(0x17)
+    })
+
+    it('covers the six data bytes of an 11-byte reply', () => {
+      // The reply is longer than the host frame. A checksum hardcoded to the
+      // host range silently mis-validates every reply the acceptor sends.
+      const reply = [0x02, 0x0b, 0x20, 0x01, 0x10, 0x00, 0x00, 0x12, 0x34, 0x03, 0x00]
+      let want = 0
+      for (let i = 1; i <= 8; i++) want ^= reply[i] as number
+      reply[10] = want
+      expect(computeChecksum(Buffer.from(reply))).toBe(want)
+    })
+  })
+
   describe('buildFrame', () => {
     it('lays out the 8-byte poll frame with the ACK bit and checksum', () => {
       const f = buildFrame(0, 0x7f, 0x00)
@@ -32,6 +54,14 @@ describe('Apex RS-232 protocol', () => {
       expect(f[7]).toBe(0x66)
     })
 
+    it('carries escrow, stack and return in the command byte', () => {
+      // Rev G BYTE 1: bit 4 escrow enable, bit 5 stack, bit 6 return. Escrow
+      // is an enable held across polls, so it rides alongside the action bit.
+      expect(buildFrame(0, 0x7f, 0x10)[4]).toBe(0x10)
+      expect(buildFrame(0, 0x7f, 0x30)[4]).toBe(0x30)
+      expect(buildFrame(0, 0x7f, 0x50)[4]).toBe(0x50)
+    })
+
     it('sets the stack command bit (0x20) in the command byte', () => {
       const f = buildFrame(0, 0x7f, 0x20)
       expect(f[4]).toBe(0x20)
@@ -82,11 +112,14 @@ describe('Apex RS-232 protocol', () => {
   })
 
   describe('parseResponse', () => {
-    const usd = [1, 5, 10, 20, 50, 100]
+    // Resolve through the real module, not a hand-copied array. The previous
-    const resolve = (ch: number) => usd[ch - 1] ?? null
+    // local copy duplicated the shipping table's off-by-one and so asserted
+    // the bug instead of catching it.
+    const resolve = (ch: number) => denomForChannel('USD', ch)
 
     it('resolves the escrowed note denomination from the credit channel', () => {
-      // state=escrowed, event=present, credit=channel 4 ($20)
+      // state=escrowed, event=present, credit=channel 4. Per spec Rev G the
+      // USD channel order is $1 $2 $5 $10 $20 $50 $100, so channel 4 is $10.
       const frame = Buffer.from([
         0x02,
         0x0b,
@@ -102,7 +135,7 @@ describe('Apex RS-232 protocol', () => {
       ])
       const r = parseResponse(frame, resolve)
       expect(r.status).toBe('billsRead')
-      expect(r.bill?.denomination).toBe(20)
+      expect(r.bill?.denomination).toBe(10)
     })
 
     it('returns no bill when no channel is credited', () => {
@@ -124,8 +157,9 @@ describe('Apex RS-232 protocol', () => {
       expect(r.bill).toBeUndefined()
     })
 
-    it('yields a null denomination for an unmapped channel', () => {
+    it('maps the top channel to the largest note', () => {
-      // channel 7 not present in the 6-entry USD table
+      // Channel 7 is $100. It read as unmapped while the table omitted $2,
+      // which is exactly the shift this test now pins down.
       const frame = Buffer.from([
         0x02,
         0x0b,
@@ -140,7 +174,25 @@ describe('Apex RS-232 protocol', () => {
         0x00,
       ])
       const r = parseResponse(frame, resolve)
-      expect(r.bill?.denomination).toBeNull()
+      expect(r.bill?.denomination).toBe(100)
+    })
+
+    it('pins the whole USD channel order from the spec', () => {
+      // Rev G, BYTE 2 bits 3-5: 001=$1 010=$2 011=$5 100=$10 101=$20
+      // 110=$50 111=$100. A note credited at the wrong value is silent and
+      // costs real money, so the full mapping is asserted rather than sampled.
+      expect([1, 2, 3, 4, 5, 6, 7].map((ch) => denomForChannel('USD', ch))).toEqual([
+        1, 2, 5, 10, 20, 50, 100,
+      ])
+    })
+
+    it('has no denomination for channel 0, which means no note', () => {
+      expect(denomForChannel('USD', 0)).toBeNull()
+    })
+
+    it('has no denomination when the currency is unknown', () => {
+      expect(denomForChannel(null, 3)).toBeNull()
+      expect(denomForChannel('ZZZ', 3)).toBeNull()
     })
   })
 })

packages/hal/src/validators/apex/apex-rs232.ts

@@ -6,23 +6,27 @@
  *
  * Implemented from Pyramid's PUBLIC protocol facts only — the wire format,
  * bit masks and serial parameters documented in Pyramid's "RS-232 Serial
- * Interface Specification" (https://pyramidacceptors.com/pdf/RS_232.pdf) and
+ * Interface Specification", document RS_232, Rev G 12/03/14. No third-party
- * mirrored by their published integrator samples. No third-party (or
+ * (or lamassu-machine) source is copied; the byte layout below is a functional
- * lamassu-machine) source is copied; the byte layout below is a functional
  * spec, re-expressed for bitSpire under AGPL.
  *
+ * The interface is Mars/MEI GL5-compatible, which is why it looks so much like
+ * the EBDS driver next door. The acceptor is a pure slave: it answers polls and
+ * never speaks first. Polls must not fall more than 5s apart or the acceptor
+ * may dump an escrowed note and stop accepting until the host resumes.
+ *
  * Frame (host → acceptor), fixed 8 bytes:
  *   [0] STX   0x02
  *   [1] LEN   0x08
- *   [2] CTRL  0x10 | ack        (ack toggles 0↔1 every message)
+ *   [2] CTRL  msg type 1 (master) in bits 4-6, ack in bit 0 (toggles every message)
- *   [3] ENA   denomination enable bitmask (0x7F = all, 0x00 = none)
+ *   [3] ENA   BYTE 0 — per-note enable bits: bit 0 = note 1 … bit 6 = note 7
- *   [4] CMD   0x00 base; | 0x20 stacks the escrowed note
+ *   [4] CMD   BYTE 1 — bit 4 escrow enable, bit 5 stack, bit 6 return
- *   [5] RSVD  0x00
+ *   [5] RSVD  BYTE 2 — reserved, 0x00
  *   [6] ETX   0x03
- *   [7] CHK   XOR of bytes [1..5]
+ *   [7] CHK   XOR of all bytes except STX, ETX and itself
  *
- * Frame (acceptor → host), length-prefixed like the host frame; the fields
+ * Frame (acceptor → host), 11 bytes — STX, LEN, CTRL, six data bytes, ETX,
- * this driver consumes:
+ * CHK. The fields this driver consumes:
  *   [3] STATE bits  1=idling 2=accepting 4=escrowed 8=stacking
  *                   16=stacked 32=returning 64=returned
  *   [4] EVENT bits  0x01=cheated 0x02=rejected 0x04=jammed
@@ -39,8 +43,11 @@ const STX = 0x02
 const ETX = 0x03
 const HOST_FRAME_LEN = 0x08
 
-// Host command byte (frame[4])
+// Host command byte (frame[4]) — spec Rev G, "Data Fields for Messages Sent
-const CMD_STACK = 0x20
+// By the Master", BYTE 1.
+const CMD_ESCROW = 0x10 // bit 4: set to 1 to ENABLE escrow mode
+const CMD_STACK = 0x20 // bit 5: stack the escrowed note
+const CMD_RETURN = 0x40 // bit 6: return the escrowed note
 
 // Response STATE byte (frame[3]) bit masks
 const STATE_IDLING = 0x01
@@ -90,10 +97,18 @@ export interface ApexRs232Config {
 // Pure functions — checksum, frame building, response parsing
 // ---------------------------------------------------------------------------
 
-/** XOR checksum over bytes [1..5] (LEN through RSVD), matching the host frame. */
+/**
-export function computeChecksum(frame: number[] | Buffer): number {
+ * XOR checksum over every byte except STX, ETX and the checksum itself — spec
+ * Rev G: "calculated on all bytes (except: STX, ETX and the checksum byte
+ * itself)". For the 8-byte host frame that is bytes 1..5; for the 11-byte
+ * reply it is bytes 1..8, which is why this is derived from the length rather
+ * than hardcoded. Confirmed against the two reset frames the spec spells out
+ * literally (02 08 61 7f 7f 7f 03 16 and 02 08 60 7f 7f 7f 03 17).
+ */
+export function computeChecksum(frame: number[] | Buffer, length?: number): number {
+  const n = length ?? frame.length
   let cs = 0x00
-  for (let i = 1; i <= 5; i++) cs ^= frame[i] ?? 0
+  for (let i = 1; i <= n - 3; i++) cs ^= frame[i] ?? 0
   return cs
 }
 
@@ -278,18 +293,25 @@ export class ApexRs232 extends EventEmitter {
 
   /** Send one poll, carrying the current mask + latched escrow action. */
   poll(): void {
-    // 'return' is expressed by disabling all channels while a note is escrowed,
+    // Escrow is asserted on EVERY poll. It is an enable bit, not a one-shot:
-    // which makes the acceptor hand the note back (Apex has no distinct return
+    // with it clear the acceptor never stops at escrow, so the host is never
-    // opcode). 'stack' asserts CMD_STACK. Both are re-asserted until the device
+    // offered the stack/return decision and notes are banked before anything
-    // leaves escrow. NOTE: verify the return-by-disable behaviour on the 7600
+    // has validated them. This driver's whole FSM is built around that
-    // during bench bring-up; some firmware returns only on escrow timeout.
+    // decision point.
-    let enableByte = this.enabledMask
+    //
-    let cmdByte = 0x00
+    // Stack and return are the spec's own bits. An earlier version expressed
-    if (this.pendingAction === 'stack') cmdByte = CMD_STACK
+    // return by zeroing the enable mask, on the assumption that the Apex had
-    else if (this.pendingAction === 'return') enableByte = 0x00
+    // no return opcode; it has one, and disabling channels mid-escrow is not
+    // what it means.
+    //
+    // Both are re-asserted until the device leaves escrow, so a single dropped
+    // frame cannot strand a note.
+    let cmdByte = CMD_ESCROW
+    if (this.pendingAction === 'stack') cmdByte |= CMD_STACK
+    else if (this.pendingAction === 'return') cmdByte |= CMD_RETURN
 
     this.ack ^= 0x01
-    this.serial?.write(buildFrame(this.ack, enableByte, cmdByte))
+    this.serial?.write(buildFrame(this.ack, this.enabledMask, cmdByte))
   }
 
   stack(): void {
@@ -331,13 +353,20 @@ export class ApexRs232 extends EventEmitter {
       this.poll()
       return
     }
-    // Checksum is validated leniently: a mismatch is logged once but the frame
+    // The XOR range is now confirmed from the spec, so a mismatch is a hard
-    // is still parsed. Pyramid's published host samples don't verify the reply
+    // drop rather than the previous parse-anyway. A corrupted frame carries a
-    // checksum, and the exact XOR range for the *reply* isn't confirmable from
+    // denomination field, and crediting a note from a frame we know is damaged
-    // the (scanned) spec — so we don't want a wrong assumption to blackhole
+    // is the one outcome worth refusing outright. The raw bytes are logged so
-    // every otherwise-valid frame. Tighten to a hard drop once verified on hw.
+    // a systematic framing error is still diagnosable rather than silent.
-    if (frame[len - 1] !== computeChecksum(frame)) {
+    const want = computeChecksum(frame, len)
-      console.warn('[APEX] reply checksum mismatch (parsing anyway pending hw verification)')
+    if (frame[len - 1] !== want) {
+      console.warn(
+        `[APEX] reply checksum mismatch: got ${frame[len - 1]?.toString(16)} ` +
+          `want ${want.toString(16)} — frame dropped: ${frame.toString('hex')}`
+      )
+      this.emit('badFrame')
+      this.poll()
+      return
     }
 
     const result = parseResponse(frame, this.denomForChannel)