diff --git a/apps/machine/electron/hal-service.ts b/apps/machine/electron/hal-service.ts index 2ec863b..1259c77 100644 --- a/apps/machine/electron/hal-service.ts +++ b/apps/machine/electron/hal-service.ts @@ -376,12 +376,10 @@ export async function initializeHal(config: HalConfig): Promise { setCassettes: async (cassettes: CassetteConfig[]): Promise => { console.log( '[HAL] Hot-reloading cassette layout:', - cassettes - .map((c) => `bay${c.position}:${c.denomination}×${c.count ?? 0}`) - .join(', ') + cassettes.map((c) => `bay${c.position}:${c.denomination}×${c.count ?? 0}`).join(', ') ) - // Rebuild bays first so subsequent dispense calls see the new layout - // even if the dispenser re-init is slow / fails. + const previousBays = bays + const previousInitData = dispenserInitData bays = cassettes .slice() .sort((a, b) => a.position - b.position) @@ -391,31 +389,40 @@ export async function initializeHal(config: HalConfig): Promise { count: c.count ?? 0, })) dispenserInitData = { fiatCode: valConfig.fiatCode, cassettes } - // Close + re-init the dispenser so its internal per-bay state matches - // the new layout. Errors here surface to the caller (operator-config - // consumer) — the renderer can decide whether to retry. + // Close + re-init the dispenser so its internal per-bay state matches the + // new layout. close() now resolves only once the port is really closed, + // so the re-open below cannot race it (aiolabs/bitspire#118). + // + // On failure, roll the in-memory layout back. This reverses an earlier + // deliberate choice to keep the new bays "even if the dispenser re-init + // is slow / fails": with the re-init failing every time on douro, the app + // kept a layout the device had never taken and the operator-config + // consumer went on to publish a cassettes-state event advertising it. A + // subsequent dispense would then pick bays by a layout the hardware does + // not share. Better to surface the failure and stay truthful about what + // the device is actually running. try { - dispenser.close() + await dispenser.close() + await dispenser.init(dispenserInitData) } catch (err) { - console.warn('[HAL] Dispenser close during setCassettes raised:', err) + bays = previousBays + dispenserInitData = previousInitData + console.error('[HAL] Dispenser re-init failed; keeping the previous cassette layout:', err) + throw err } - await dispenser.init(dispenserInitData) console.log('[HAL] Dispenser re-initialized with new cassettes') }, cleanup: async () => { - return new Promise((resolve) => { - validator?.disable() - validator?.lightOff() - dispenser.close() - if (validator) { - validator.close((err?: Error) => { - if (err) console.error('[HAL] Validator close error:', err) - resolve() - }) - } else { + validator?.disable() + validator?.lightOff() + await dispenser.close() + if (!validator) return + await new Promise((resolve) => { + validator?.close((err?: Error) => { + if (err) console.error('[HAL] Validator close error:', err) resolve() - } + }) }) }, } diff --git a/packages/hal/src/dispensers/f56/f56-rs232.ts b/packages/hal/src/dispensers/f56/f56-rs232.ts index ae233c0..24e1d60 100644 --- a/packages/hal/src/dispensers/f56/f56-rs232.ts +++ b/packages/hal/src/dispensers/f56/f56-rs232.ts @@ -240,9 +240,25 @@ fsm.on('send', (data: Buffer) => { serial?.write(data) }) -export function close(): void { - serial?.close() +/** + * Close the serial port, resolving once the OS handle is really gone. + * + * serialport's close() is asynchronous, so returning before its callback fires + * let a close-then-reopen caller (setCassettes -> init) race the old handle and + * fail to take the exclusive lock. Narrower window than puloon's, which + * deferred the close behind a timer, but the same bug. See aiolabs/bitspire#118. + */ +export async function close(): Promise { + const port = serial + if (!port) return + // Claim the handle up front so a concurrent close() can't double-close it. serial = null + await new Promise((resolve) => { + port.close((err?: Error | null) => { + if (err) console.warn('F56 | serial close raised:', err.message) + resolve() + }) + }) } export default { diff --git a/packages/hal/src/dispensers/f56/index.ts b/packages/hal/src/dispensers/f56/index.ts index 182febf..c64882d 100644 --- a/packages/hal/src/dispensers/f56/index.ts +++ b/packages/hal/src/dispensers/f56/index.ts @@ -56,14 +56,14 @@ export class F56Dispenser implements BillDispenser { const { bills, error } = await f56.billCount(notes) if (error) { - this.close() + await this.close() ;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError' ;(error as Error & { statusCode: number }).statusCode = 570 } return { value: bills, error } } catch (err) { - this.close() + await this.close() const error = err as Error ;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError' ;(error as Error & { statusCode: number }).statusCode = 570 @@ -71,8 +71,8 @@ export class F56Dispenser implements BillDispenser { } } - close(): void { - f56.close() + async close(): Promise { + await f56.close() this.initialized = false } diff --git a/packages/hal/src/dispensers/puloon/index.ts b/packages/hal/src/dispensers/puloon/index.ts index 63186e9..f065fd4 100644 --- a/packages/hal/src/dispensers/puloon/index.ts +++ b/packages/hal/src/dispensers/puloon/index.ts @@ -50,7 +50,7 @@ export class PuloonDispenser implements BillDispenser { const { bills, error } = await this.device.dispense(notes) if (error) { - this.close() + await this.close() error.name = 'PuloonDispenseError' console.log('PULOON | dispense error', error) } @@ -58,8 +58,8 @@ export class PuloonDispenser implements BillDispenser { return { value: bills, error } } - close(): void { - this.device.close() + async close(): Promise { + await this.device.close() this.initialized = false } diff --git a/packages/hal/src/dispensers/puloon/puloon-rs232.ts b/packages/hal/src/dispensers/puloon/puloon-rs232.ts index e057b73..c579f14 100644 --- a/packages/hal/src/dispensers/puloon/puloon-rs232.ts +++ b/packages/hal/src/dispensers/puloon/puloon-rs232.ts @@ -13,6 +13,9 @@ */ import { SerialPort } from 'serialport' + +/** Grace period for an in-flight write to land before the port is closed. */ +const SERIAL_DRAIN_MS = 100 import { billLengths, encodeBillLength } from './bills.js' import type { CassetteConfig, DispenseResult } from '../../types.js' @@ -238,13 +241,32 @@ export class PuloonRs232 { } /** Close the serial port */ - close(): void { - if (this.serial) { - setTimeout(() => { - this.serial?.close() - this.serial = null - }, 100) - } + /** + * Close the serial port, resolving only once the OS handle is really gone. + * + * This used to defer `serial.close()` behind a 100 ms setTimeout and return + * immediately, with no way for a caller to know when the port was free. A + * caller that closed and reopened — setCassettes -> init — therefore raced a + * handle that was still open and got EAGAIN "Cannot lock port" every time, + * since the overlap was the full 100 ms. And if the reopen ever won the + * race, the pending timer fired afterwards and closed the *new* handle, + * leaving a silently dead dispenser. See aiolabs/bitspire#118. + * + * The drain delay is kept — it lets an in-flight write land before the port + * goes away — but it is now awaited rather than fired and forgotten. + */ + async close(): Promise { + const serial = this.serial + if (!serial) return + // Claim the handle up front so a concurrent close() can't double-close it. + this.serial = null + await new Promise((resolve) => setTimeout(resolve, SERIAL_DRAIN_MS)) + await new Promise((resolve) => { + serial.close((err?: Error | null) => { + if (err) console.warn('PULOON | serial close raised:', err.message) + resolve() + }) + }) } /** Send a command and wait for a single response */ diff --git a/packages/hal/src/types.ts b/packages/hal/src/types.ts index 0936fea..bf0615f 100644 --- a/packages/hal/src/types.ts +++ b/packages/hal/src/types.ts @@ -179,8 +179,13 @@ export interface BillDispenser { error?: Error }> - /** Close the connection */ - close(): void + /** + * Close the connection. + * + * Resolves only once the underlying port is actually closed, so a caller may + * safely reopen it straight after awaiting this (aiolabs/bitspire#118). + */ + close(): Promise /** * Check if bills are present at the dispense outlet