diff --git a/packages/hal/src/validators/apex/__tests__/apex-rs232.test.ts b/packages/hal/src/validators/apex/__tests__/apex-rs232.test.ts index 95c1607..bafb50d 100644 --- a/packages/hal/src/validators/apex/__tests__/apex-rs232.test.ts +++ b/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] - const resolve = (ch: number) => usd[ch - 1] ?? null + // Resolve through the real module, not a hand-copied array. The previous + // 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', () => { - // channel 7 not present in the 6-entry USD table + it('maps the top channel to the largest note', () => { + // 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() }) }) }) diff --git a/packages/hal/src/validators/apex/apex-rs232.ts b/packages/hal/src/validators/apex/apex-rs232.ts index d8bcb7e..e8fc12f 100644 --- a/packages/hal/src/validators/apex/apex-rs232.ts +++ b/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 - * mirrored by their published integrator samples. No third-party (or - * lamassu-machine) source is copied; the byte layout below is a functional + * Interface Specification", document RS_232, Rev G 12/03/14. No third-party + * (or 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) - * [3] ENA denomination enable bitmask (0x7F = all, 0x00 = none) - * [4] CMD 0x00 base; | 0x20 stacks the escrowed note - * [5] RSVD 0x00 + * [2] CTRL msg type 1 (master) in bits 4-6, ack in bit 0 (toggles every message) + * [3] ENA BYTE 0 — per-note enable bits: bit 0 = note 1 … bit 6 = note 7 + * [4] CMD BYTE 1 — bit 4 escrow enable, bit 5 stack, bit 6 return + * [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 - * this driver consumes: + * Frame (acceptor → host), 11 bytes — STX, LEN, CTRL, six data bytes, ETX, + * 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]) -const CMD_STACK = 0x20 +// Host command byte (frame[4]) — spec Rev G, "Data Fields for Messages Sent +// 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, - // which makes the acceptor hand the note back (Apex has no distinct return - // opcode). 'stack' asserts CMD_STACK. Both are re-asserted until the device - // leaves escrow. NOTE: verify the return-by-disable behaviour on the 7600 - // during bench bring-up; some firmware returns only on escrow timeout. - let enableByte = this.enabledMask - let cmdByte = 0x00 - if (this.pendingAction === 'stack') cmdByte = CMD_STACK - else if (this.pendingAction === 'return') enableByte = 0x00 + // Escrow is asserted on EVERY poll. It is an enable bit, not a one-shot: + // with it clear the acceptor never stops at escrow, so the host is never + // offered the stack/return decision and notes are banked before anything + // has validated them. This driver's whole FSM is built around that + // decision point. + // + // Stack and return are the spec's own bits. An earlier version expressed + // return by zeroing the enable mask, on the assumption that the Apex had + // 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 - // is still parsed. Pyramid's published host samples don't verify the reply - // checksum, and the exact XOR range for the *reply* isn't confirmable from - // the (scanned) spec — so we don't want a wrong assumption to blackhole - // every otherwise-valid frame. Tighten to a hard drop once verified on hw. - if (frame[len - 1] !== computeChecksum(frame)) { - console.warn('[APEX] reply checksum mismatch (parsing anyway pending hw verification)') + // The XOR range is now confirmed from the spec, so a mismatch is a hard + // drop rather than the previous parse-anyway. A corrupted frame carries a + // denomination field, and crediting a note from a frame we know is damaged + // is the one outcome worth refusing outright. The raw bytes are logged so + // a systematic framing error is still diagnosable rather than silent. + const want = computeChecksum(frame, len) + 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)