fix discovery monitor behavior and transport option semantics

Author: GhostTypes

docs/clients.md

@@ -148,7 +148,7 @@ Discovers printers on the local network using UDP multicast and broadcast.
 - **`idleTimeout`** (number): Time to wait after last response (default: 1500ms)
 - **`maxRetries`** (number): Maximum retry attempts (default: 3)
 - **`useMulticast`** (boolean): Use multicast discovery (default: true)
-- **`useBroadcast`** (boolean): Use broadcast discovery (default: true)
+- **`useBroadcast`** (boolean): Use subnet broadcast discovery (default: true)
 - **`ports`** (number[]): Specific ports to scan (default: [8899, 19000, 48899])
 
 **Returns:** An array of `DiscoveredPrinter` objects with comprehensive printer information.

docs/specs/printer-discovery.md

@@ -1178,53 +1178,14 @@ printers.forEach(printer => {
 | N/A | `eventPort` | New field |
 | N/A | `statusCode` | New field |
 
-### Backward Compatibility Layer
+### Backward Compatibility
 
-**File:** `src/api/PrinterDiscovery.legacy.ts` (NEW)
+No compatibility shim is provided in this implementation.
 
-```typescript
-/**
- * @deprecated Use PrinterDiscovery instead
- */
-export class FlashForgePrinterDiscovery extends PrinterDiscovery {
-    /**
-     * @deprecated Use discover() instead
-     */
-    public async discoverPrintersAsync(
-        timeoutMs = 10000,
-        idleTimeoutMs = 1500,
-        maxRetries = 3
-    ): Promise<FlashForgePrinter[]> {
-        const printers = await this.discover({
-            timeout: timeoutMs,
-            idleTimeout: idleTimeoutMs,
-            maxRetries
-        });
-
-        // Convert to old format
-        return printers.map(toLegacyPrinter);
-    }
-}
-
-/**
- * @deprecated Use DiscoveredPrinter instead
- */
-export class FlashForgePrinter {
-    public name: string = '';
-    public serialNumber: string = '';
-    public ipAddress: string = '';
-    public isAD5X?: boolean;
-}
-
-function toLegacyPrinter(printer: DiscoveredPrinter): FlashForgePrinter {
-    const legacy = new FlashForgePrinter();
-    legacy.name = printer.name;
-    legacy.serialNumber = printer.serialNumber || '';
-    legacy.ipAddress = printer.ipAddress;
-    legacy.isAD5X = printer.model === PrinterModel.AD5X;
-    return legacy;
-}
-```
+This is an intentional breaking change. Consumers must migrate from:
+- `FlashForgePrinterDiscovery` -> `PrinterDiscovery`
+- `FlashForgePrinter` -> `DiscoveredPrinter`
+- `discoverPrintersAsync(timeout, idleTimeout, maxRetries)` -> `discover({ timeout, idleTimeout, maxRetries })`
 
 ---
 

src/api/PrinterDiscovery.test.ts

@@ -3,10 +3,10 @@
  *
  * Tests protocol parsers (modern 276-byte, legacy 140-byte), model detection,
  * status mapping, multi-port discovery, timeout handling, deduplication,
- * and backward compatibility with legacy API.
+ * and monitor/event behavior.
  */
 import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
-import * as dgram from 'node:dgram';
+import type * as dgram from 'node:dgram';
 import { EventEmitter } from 'node:events';
 import { PrinterDiscovery } from './PrinterDiscovery';
 import {
@@ -16,7 +16,7 @@ import {
     type DiscoveredPrinter,
     type DiscoveryOptions,
 } from '../models/PrinterDiscovery';
-import { InvalidResponseError, SocketCreationError } from './network/DiscoveryErrors';
+import { InvalidResponseError } from './network/DiscoveryErrors';
 
 // Suppress logs during tests
 const originalConsole = { ...console };
@@ -444,7 +444,7 @@ describe('PrinterDiscovery', () => {
 
                 protected async createDiscoverySocket(): Promise<dgram.Socket> {
                     const mockSocket = new EventEmitter() as dgram.Socket;
-                    mockSocket.bind = vi.fn((port, callback) => {
+                    mockSocket.bind = vi.fn((_port, callback) => {
                         if (callback) callback();
                     });
                     mockSocket.setBroadcast = vi.fn();
@@ -489,7 +489,7 @@ describe('PrinterDiscovery', () => {
 
                 protected async createDiscoverySocket(): Promise<dgram.Socket> {
                     const mockSocket = new EventEmitter() as dgram.Socket;
-                    mockSocket.bind = vi.fn((port, callback) => {
+                    mockSocket.bind = vi.fn((_port, callback) => {
                         if (callback) callback();
                     });
                     mockSocket.setBroadcast = vi.fn();
@@ -540,7 +540,7 @@ describe('PrinterDiscovery', () => {
             class TestPrinterDiscovery extends PrinterDiscovery {
                 protected async createDiscoverySocket(): Promise<dgram.Socket> {
                     const mockSocket = new EventEmitter() as dgram.Socket;
-                    mockSocket.bind = vi.fn((port, callback) => {
+                    mockSocket.bind = vi.fn((_port, callback) => {
                         if (callback) callback();
                     });
                     mockSocket.setBroadcast = vi.fn();
@@ -567,7 +567,7 @@ describe('PrinterDiscovery', () => {
 
                 protected async createDiscoverySocket(): Promise<dgram.Socket> {
                     const mockSocket = new EventEmitter() as dgram.Socket;
-                    mockSocket.bind = vi.fn((port, callback) => {
+                    mockSocket.bind = vi.fn((_port, callback) => {
                         if (callback) callback();
                     });
                     mockSocket.setBroadcast = vi.fn();
@@ -612,6 +612,96 @@ describe('PrinterDiscovery', () => {
 
             await endPromise;
         });
+
+        it('should emit end when stopped manually', async () => {
+            class TestPrinterDiscovery extends PrinterDiscovery {
+                protected async createDiscoverySocket(): Promise<dgram.Socket> {
+                    const mockSocket = new EventEmitter() as dgram.Socket;
+                    mockSocket.bind = vi.fn((_port, callback) => {
+                        if (callback) callback();
+                    });
+                    mockSocket.setBroadcast = vi.fn();
+                    mockSocket.send = vi.fn();
+                    mockSocket.close = vi.fn();
+                    mockSocket.addMembership = vi.fn();
+                    return mockSocket;
+                }
+            }
+
+            const discovery = new TestPrinterDiscovery();
+            const monitor = discovery.monitor({ timeout: 1000, maxRetries: 10 });
+
+            const endPromise = new Promise<void>((resolve, reject) => {
+                const timeoutHandle = setTimeout(() => {
+                    reject(new Error('Timed out waiting for end event'));
+                }, 500);
+
+                monitor.on('error', reject);
+                monitor.on('end', () => {
+                    clearTimeout(timeoutHandle);
+                    resolve();
+                });
+            });
+
+            setTimeout(() => {
+                monitor.stop();
+            }, 50);
+
+            await endPromise;
+        });
+
+        it('should honor idleTimeout after first discovery response', async () => {
+            class TestPrinterDiscovery extends PrinterDiscovery {
+                public mockSocket: dgram.Socket | null = null;
+
+                protected async createDiscoverySocket(): Promise<dgram.Socket> {
+                    const mockSocket = new EventEmitter() as dgram.Socket;
+                    mockSocket.bind = vi.fn((_port, callback) => {
+                        if (callback) callback();
+                    });
+                    mockSocket.setBroadcast = vi.fn();
+                    mockSocket.send = vi.fn();
+                    mockSocket.close = vi.fn();
+                    mockSocket.addMembership = vi.fn();
+                    this.mockSocket = mockSocket;
+                    return mockSocket;
+                }
+            }
+
+            const discovery = new TestPrinterDiscovery();
+            const start = Date.now();
+            const monitor = discovery.monitor({ timeout: 1000, idleTimeout: 100, maxRetries: 10 });
+
+            const endPromise = new Promise<void>((resolve, reject) => {
+                const timeoutHandle = setTimeout(() => {
+                    reject(new Error('Timed out waiting for idleTimeout end event'));
+                }, 1000);
+
+                monitor.on('error', reject);
+                monitor.on('end', () => {
+                    clearTimeout(timeoutHandle);
+                    resolve();
+                });
+            });
+
+            setTimeout(() => {
+                const mockSocket = discovery.mockSocket;
+                if (mockSocket) {
+                    const buffer = createModernBuffer({ name: 'Idle Timeout Test' });
+                    mockSocket.emit('message', buffer, {
+                        address: '192.168.1.160',
+                        port: 8899,
+                        family: 'IPv4',
+                        size: 276,
+                    });
+                }
+            }, 50);
+
+            await endPromise;
+            const elapsedMs = Date.now() - start;
+
+            expect(elapsedMs).toBeLessThan(500);
+        });
     });
 
     describe('Broadcast Address Calculation', () => {
@@ -637,15 +727,18 @@ describe('PrinterDiscovery', () => {
 describe('DiscoveryOptions', () => {
     // Test discovery class with mocked socket
     class TestPrinterDiscovery extends PrinterDiscovery {
+        public mockSocket: dgram.Socket | null = null;
+
         protected async createDiscoverySocket(): Promise<dgram.Socket> {
             const mockSocket = new EventEmitter() as dgram.Socket;
-            mockSocket.bind = vi.fn((port, callback) => {
+            mockSocket.bind = vi.fn((_port, callback) => {
                 if (callback) callback();
             });
             mockSocket.setBroadcast = vi.fn();
             mockSocket.send = vi.fn();
             mockSocket.close = vi.fn();
             mockSocket.addMembership = vi.fn();
+            this.mockSocket = mockSocket;
             return mockSocket;
         }
     }
@@ -692,4 +785,22 @@ describe('DiscoveryOptions', () => {
         const printers = await discovery.discover(options);
         expect(printers).toHaveLength(0);
     });
+
+    it('should not send packets when multicast and broadcast are both disabled', async () => {
+        const discovery = new TestPrinterDiscovery();
+
+        const options: DiscoveryOptions = {
+            timeout: 50,
+            idleTimeout: 25,
+            maxRetries: 1,
+            useMulticast: false,
+            useBroadcast: false,
+            ports: [8899, 19000, 48899],
+        };
+
+        const printers = await discovery.discover(options);
+        expect(printers).toHaveLength(0);
+        expect(discovery.mockSocket).not.toBeNull();
+        expect((discovery.mockSocket?.send as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled();
+    });
 });