Skip to content

Commit 735a9cb

Browse files
authored
feat(cli): add --id option to discover command for slave ID filtering (#321)
Add optional --id parameter to the discover command that allows filtering device searches to specific slave IDs instead of searching the full 1-247 range. Core Implementation: - Add parseIdRange() utility to parse ID specifications like "1,2,3-5" - Extend GeneratorOptions with optional slaveIds filter - Update parameter generators to respect slave ID filtering - Add --id CLI option with support for multiple specifications User Experience: - Display filtered IDs in console output (e.g., "Slave IDs: 1, 2, 3 (3 IDs)") - Merge and deduplicate multiple --id flags - Maintain priority ordering (defaultAddress first, then 1, 2, then rest) Performance Benefits: - Searching only 5 IDs instead of 247 reduces combinations by ~98% - Example: Quick scan of IDs 1-5 completes in ~1 min vs ~50 min for all IDs - With driver: IDs 1-5 in ~30 sec vs ~25 min for all IDs Critical Fixes (Post-Review): 1. Progress tracker bug (e2d1514) - countParameterCombinations now respects slaveIds filter 2. Empty string validation (4ed1517) - Rejects empty --id specifications with clear error 3. Decimal validation (d744871) - Rejects decimal numbers explicitly Testing: - 28 unit tests for ID range parser (including edge cases) - 10 unit tests for parameter generator filtering - 10 E2E tests for discover command integration - All 449 tests passing Related issues: #326, #328, #329, #330, #331
1 parent a3b4be0 commit 735a9cb

11 files changed

Lines changed: 602 additions & 25 deletions

packages/cli/docs/discover-command.md

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,22 @@ This will:
5454
- Lower values scan faster but may cause errors
5555
- Recommended: 100ms minimum
5656

57+
- `--id <spec>` - Limit search to specific slave IDs (can be specified multiple times)
58+
- Format: Single IDs or ranges separated by commas (e.g., `1,2,3-5`)
59+
- Multiple `--id` flags are merged and deduplicated
60+
- Dramatically reduces scan time when you know which IDs to search
61+
- Examples:
62+
- `--id 1` - Search only slave ID 1
63+
- `--id 1,2,3` - Search IDs 1, 2, and 3
64+
- `--id 1-5` - Search IDs 1 through 5
65+
- `--id 1,3-5,10` - Search IDs 1, 3, 4, 5, and 10
66+
- `--id 1-3 --id 5-7` - Search IDs 1, 2, 3, 5, 6, and 7
67+
68+
- `--max-devices <count>` - Maximum devices to find (default: `1`, use `0` for unlimited)
69+
- Stops scanning after finding specified number of devices
70+
- Use `0` to find all devices on the bus
71+
- Speeds up discovery when you only need to find one device
72+
5773
### Output Options
5874

5975
- `--format <type>` - Output format (default: `table`)
@@ -188,12 +204,14 @@ Discovery time depends on several factors:
188204

189205
**Examples:**
190206

191-
| Strategy | Driver | Combinations | Timeout | Est. Time |
192-
| -------- | ------ | ------------ | ------- | ---------- |
193-
| Quick | No | ~2,964 | 1000ms | ~50 min |
194-
| Quick | Yes | ~1,482 | 1000ms | ~25 min |
195-
| Thorough | No | ~23,712 | 1000ms | ~6.5 hours |
196-
| Thorough | Yes | ~1,482 | 1000ms | ~25 min |
207+
| Strategy | Driver | ID Filter | Combinations | Timeout | Est. Time |
208+
| -------- | ------ | --------- | ------------ | ------- | ---------- |
209+
| Quick | No | None | ~2,964 | 1000ms | ~50 min |
210+
| Quick | No | 1-5 | ~60 | 1000ms | ~1 min |
211+
| Quick | Yes | None | ~1,482 | 1000ms | ~25 min |
212+
| Quick | Yes | 1-5 | ~30 | 1000ms | ~30 sec |
213+
| Thorough | No | None | ~23,712 | 1000ms | ~6.5 hours |
214+
| Thorough | Yes | None | ~1,482 | 1000ms | ~25 min |
197215

198216
**Note:** Actual time is usually much faster because:
199217

@@ -202,10 +220,11 @@ Discovery time depends on several factors:
202220

203221
### Optimization Tips
204222

205-
1. **Use a driver** - Reduces combinations by 50-90%
206-
2. **Lower timeout** - Use 250-500ms if devices respond quickly
207-
3. **Start with quick strategy** - Finds 90% of devices
208-
4. **Know your device** - If you know slave ID, modify the scan manually
223+
1. **Filter slave IDs** - Use `--id` if you know which IDs to search (reduces combinations by up to 98%)
224+
2. **Use a driver** - Reduces combinations by 50-90%
225+
3. **Lower timeout** - Use 250-500ms if devices respond quickly
226+
4. **Start with quick strategy** - Finds 90% of devices
227+
5. **Stop after first device** - Default `--max-devices 1` stops after finding one device
209228

210229
### Progress Monitoring
211230

@@ -391,6 +410,26 @@ Discover on Windows COM port:
391410
ya-modbus discover --port COM3
392411
```
393412

413+
### Example 8: Search Specific Slave IDs
414+
415+
Search only specific slave IDs to dramatically reduce scan time:
416+
417+
```bash
418+
# Search only ID 1
419+
ya-modbus discover --port /dev/ttyUSB0 --id 1
420+
421+
# Search IDs 1-5
422+
ya-modbus discover --port /dev/ttyUSB0 --id 1-5
423+
424+
# Search specific IDs: 1, 2, 3, 10, 11, 12
425+
ya-modbus discover --port /dev/ttyUSB0 --id 1-3,10-12
426+
427+
# Search IDs from multiple specifications (merged and deduplicated)
428+
ya-modbus discover --port /dev/ttyUSB0 --id 1,2 --id 3-5
429+
```
430+
431+
**Performance benefit:** Searching only 5 IDs instead of all 247 reduces combinations by ~98%, completing in seconds instead of minutes.
432+
394433
## Advanced Usage
395434

396435
### Combining with Other Commands

packages/cli/src/commands/discover.test.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,4 +516,128 @@ describe('Discover Command', () => {
516516
expect(stdoutWriteSpy).not.toHaveBeenCalled()
517517
})
518518
})
519+
520+
describe('--id option (slave ID filtering)', () => {
521+
test('should show filtered IDs when --id is specified', async () => {
522+
jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
523+
524+
await discoverCommand({
525+
port: '/dev/ttyUSB0',
526+
id: '1,2,3',
527+
format: 'table',
528+
})
529+
530+
expect(consoleLogSpy).toHaveBeenCalledWith('Slave IDs: 1, 2, 3 (3 IDs)')
531+
})
532+
533+
test('should show "all" when --id is not specified', async () => {
534+
jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
535+
536+
await discoverCommand({
537+
port: '/dev/ttyUSB0',
538+
format: 'table',
539+
})
540+
541+
expect(consoleLogSpy).toHaveBeenCalledWith('Slave IDs: 1-247 (all)')
542+
})
543+
544+
test('should parse ID ranges correctly', async () => {
545+
jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
546+
547+
await discoverCommand({
548+
port: '/dev/ttyUSB0',
549+
id: '1-5,10',
550+
format: 'table',
551+
})
552+
553+
expect(consoleLogSpy).toHaveBeenCalledWith('Slave IDs: 1, 2, 3, 4, 5, 10 (6 IDs)')
554+
})
555+
556+
test('should merge multiple --id specifications', async () => {
557+
jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
558+
559+
await discoverCommand({
560+
port: '/dev/ttyUSB0',
561+
id: ['1,2', '3-5'],
562+
format: 'table',
563+
})
564+
565+
expect(consoleLogSpy).toHaveBeenCalledWith('Slave IDs: 1, 2, 3, 4, 5 (5 IDs)')
566+
})
567+
568+
test('should deduplicate merged ID specifications', async () => {
569+
jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
570+
571+
await discoverCommand({
572+
port: '/dev/ttyUSB0',
573+
id: ['1-3', '2-4'],
574+
format: 'table',
575+
})
576+
577+
expect(consoleLogSpy).toHaveBeenCalledWith('Slave IDs: 1, 2, 3, 4 (4 IDs)')
578+
})
579+
580+
test('should pass filtered IDs to scanner', async () => {
581+
const scanSpy = jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
582+
583+
await discoverCommand({
584+
port: '/dev/ttyUSB0',
585+
id: '1,5,10',
586+
format: 'table',
587+
})
588+
589+
expect(scanSpy).toHaveBeenCalledWith(
590+
expect.objectContaining({
591+
slaveIds: [1, 5, 10],
592+
}),
593+
expect.anything()
594+
)
595+
})
596+
597+
test('should not pass slaveIds to scanner when --id not specified', async () => {
598+
const scanSpy = jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
599+
600+
await discoverCommand({
601+
port: '/dev/ttyUSB0',
602+
format: 'table',
603+
})
604+
605+
// Should not have slaveIds property in generator options
606+
const generatorOptions = scanSpy.mock.calls[0][0]
607+
expect(generatorOptions).not.toHaveProperty('slaveIds')
608+
})
609+
610+
test('should handle invalid ID specification with error', async () => {
611+
await expect(
612+
discoverCommand({
613+
port: '/dev/ttyUSB0',
614+
id: 'invalid',
615+
format: 'table',
616+
})
617+
).rejects.toThrow(/invalid/i)
618+
})
619+
620+
test('should handle out-of-range IDs with error', async () => {
621+
await expect(
622+
discoverCommand({
623+
port: '/dev/ttyUSB0',
624+
id: '0,1,2',
625+
format: 'table',
626+
})
627+
).rejects.toThrow(/valid Modbus slave address/i)
628+
})
629+
630+
test('should not show filtered IDs in silent mode', async () => {
631+
jest.spyOn(scanner, 'scanForDevices').mockResolvedValue(mockDevices)
632+
633+
await discoverCommand({
634+
port: '/dev/ttyUSB0',
635+
id: '1,2,3',
636+
format: 'table',
637+
silent: true,
638+
})
639+
640+
expect(consoleLogSpy).not.toHaveBeenCalledWith(expect.stringContaining('Slave IDs'))
641+
})
642+
})
519643
})

packages/cli/src/commands/discover.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { DiscoveryStrategy } from '../discovery/parameter-generator.js'
44
import { ProgressTracker } from '../discovery/progress.js'
55
import { scanForDevices } from '../discovery/scanner.js'
66
import { formatDiscoveryJSON, formatDiscoveryTable } from '../formatters/discovery-results.js'
7+
import { parseIdRange } from '../utils/parse-id-range.js'
78

89
/**
910
* Discover command options
@@ -18,6 +19,7 @@ export interface DiscoverOptions {
1819
timeout?: number
1920
delay?: number
2021
maxDevices?: number
22+
id?: string | string[]
2123
verbose?: boolean
2224
silent?: boolean
2325

@@ -42,10 +44,29 @@ export async function discoverCommand(options: DiscoverOptions): Promise<void> {
4244
const verbose = options.verbose ?? false
4345
const silent = options.silent ?? false
4446

47+
// Parse and merge ID specifications
48+
let slaveIds: number[] | undefined
49+
if (options.id) {
50+
const idSpecs = Array.isArray(options.id) ? options.id : [options.id]
51+
const allIds = new Set<number>()
52+
53+
for (const spec of idSpecs) {
54+
const ids = parseIdRange(spec)
55+
ids.forEach((id) => allIds.add(id))
56+
}
57+
58+
slaveIds = Array.from(allIds).sort((a, b) => a - b)
59+
}
60+
4561
if (!silent) {
4662
console.log(`Starting Modbus device discovery on ${options.port}...`)
4763
console.log(`Strategy: ${strategy}`)
4864
console.log(`Timeout: ${timeout}ms, Delay: ${delay}ms`)
65+
if (slaveIds) {
66+
console.log(`Slave IDs: ${slaveIds.join(', ')} (${slaveIds.length} IDs)`)
67+
} else {
68+
console.log('Slave IDs: 1-247 (all)')
69+
}
4970
if (maxDevices === 0) {
5071
console.log('Mode: Find all devices')
5172
} else {
@@ -103,6 +124,10 @@ export async function discoverCommand(options: DiscoverOptions): Promise<void> {
103124
generatorOptions.supportedConfig = driverMetadata.supportedConfig
104125
}
105126

127+
if (slaveIds) {
128+
generatorOptions.slaveIds = slaveIds
129+
}
130+
106131
const totalCombinations = countParameterCombinations(generatorOptions)
107132

108133
const progress = new ProgressTracker(totalCombinations)

packages/cli/src/discovery/parameter-generator-utils.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,45 @@ describe('countParameterCombinations', () => {
113113
const count = countParameterCombinations(options)
114114
expect(count).toBe(1)
115115
})
116+
117+
test('respects slaveIds filter', () => {
118+
const options: GeneratorOptions = {
119+
strategy: 'quick',
120+
slaveIds: [1, 2, 3],
121+
}
122+
123+
// 2 baud × 3 parity × 1 data × 1 stop × 3 filtered IDs
124+
// = 2 × 3 × 1 × 1 × 3 = 18
125+
const count = countParameterCombinations(options)
126+
expect(count).toBe(18)
127+
})
128+
129+
test('slaveIds filter overrides addressRange', () => {
130+
const options: GeneratorOptions = {
131+
strategy: 'quick',
132+
slaveIds: [1, 5, 10],
133+
supportedConfig: {
134+
validAddressRange: [1, 247], // This should be ignored
135+
},
136+
}
137+
138+
// 2 baud × 3 parity × 2 data × 2 stop × 3 filtered IDs
139+
// Falls back to STANDARD_DATA_BITS and STANDARD_STOP_BITS when supportedConfig exists
140+
// = 2 × 3 × 2 × 2 × 3 = 72
141+
const count = countParameterCombinations(options)
142+
expect(count).toBe(72)
143+
})
144+
145+
test('handles empty slaveIds array', () => {
146+
const options: GeneratorOptions = {
147+
strategy: 'quick',
148+
slaveIds: [],
149+
}
150+
151+
// 2 baud × 3 parity × 1 data × 1 stop × 0 IDs = 0
152+
const count = countParameterCombinations(options)
153+
expect(count).toBe(0)
154+
})
116155
})
117156

118157
describe('default config prioritization', () => {

packages/cli/src/discovery/parameter-generator-utils.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,21 @@ export function getParameterArrays(
8080
* ```
8181
*/
8282
export function countParameterCombinations(options: GeneratorOptions): number {
83-
const { strategy, supportedConfig } = options
83+
const { strategy, supportedConfig, slaveIds } = options
8484

8585
const { baudRates, parities, dataBits, stopBits, addressRange } = getParameterArrays(
8686
strategy,
8787
supportedConfig
8888
)
8989

9090
// Calculate address count
91-
const [minId, maxId] = addressRange
92-
const addressCount = maxId - minId + 1
91+
let addressCount: number
92+
if (slaveIds !== undefined) {
93+
addressCount = slaveIds.length
94+
} else {
95+
const [minId, maxId] = addressRange
96+
addressCount = maxId - minId + 1
97+
}
9398

9499
// Total combinations = addresses × baud rates × parities × data bits × stop bits
95100
return addressCount * baudRates.length * parities.length * dataBits.length * stopBits.length

0 commit comments

Comments
 (0)