Merge pull request 'fix(hal): await the serial close, and stop reporting a cassette layout the dispenser refused' (#119) from fix/dispenser-close-race into dev

Reviewed-on: #119

apps/machine/electron/hal-service.ts

@@ -376,12 +376,10 @@ export async function initializeHal(config: HalConfig): Promise<HalInstance> {
     setCassettes: async (cassettes: CassetteConfig[]): Promise<void> => {
       console.log(
         '[HAL] Hot-reloading cassette layout:',
-        cassettes
+        cassettes.map((c) => `bay${c.position}:${c.denomination}×${c.count ?? 0}`).join(', ')
-          .map((c) => `bay${c.position}:${c.denomination}×${c.count ?? 0}`)
-          .join(', ')
       )
-      // Rebuild bays first so subsequent dispense calls see the new layout
+      const previousBays = bays
-      // even if the dispenser re-init is slow / fails.
+      const previousInitData = dispenserInitData
       bays = cassettes
         .slice()
         .sort((a, b) => a.position - b.position)
@@ -391,31 +389,40 @@ export async function initializeHal(config: HalConfig): Promise<HalInstance> {
           count: c.count ?? 0,
         }))
       dispenserInitData = { fiatCode: valConfig.fiatCode, cassettes }
-      // Close + re-init the dispenser so its internal per-bay state matches
+      // Close + re-init the dispenser so its internal per-bay state matches the
-      // the new layout. Errors here surface to the caller (operator-config
+      // new layout. close() now resolves only once the port is really closed,
-      // consumer) — the renderer can decide whether to retry.
+      // 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<void>((resolve) => {
+      validator?.disable()
-        validator?.disable()
+      validator?.lightOff()
-        validator?.lightOff()
+      await dispenser.close()
-        dispenser.close()
+      if (!validator) return
-        if (validator) {
+      await new Promise<void>((resolve) => {
-          validator.close((err?: Error) => {
+        validator?.close((err?: Error) => {
-            if (err) console.error('[HAL] Validator close error:', err)
+          if (err) console.error('[HAL] Validator close error:', err)
-            resolve()
-          })
-        } else {
           resolve()
-        }
+        })
       })
     },
   }

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<void> {
+  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<void>((resolve) => {
+    port.close((err?: Error | null) => {
+      if (err) console.warn('F56 | serial close raised:', err.message)
+      resolve()
+    })
+  })
 }
 
 export default {

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 {
+  async close(): Promise<void> {
-    f56.close()
+    await f56.close()
     this.initialized = false
   }
 

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 {
+  async close(): Promise<void> {
-    this.device.close()
+    await this.device.close()
     this.initialized = false
   }
 

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) {
+   * Close the serial port, resolving only once the OS handle is really gone.
-      setTimeout(() => {
+   *
-        this.serial?.close()
+   * This used to defer `serial.close()` behind a 100 ms setTimeout and return
-        this.serial = null
+   * immediately, with no way for a caller to know when the port was free. A
-      }, 100)
+   * 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<void> {
+    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<void>((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 */

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<void>
 
   /**
    * Check if bills are present at the dispense outlet