Skip to content

Commit 3e30034

Browse files
groupskygroupsky
authored
fix(emulator): convert service vector callbacks to callback-style for modbus-serial compatibility (#303) (#304)
Fixes #303 by converting Promise-based service vector callbacks to callback-style to work around a modbus-serial bug where Promise-based callbacks are not properly awaited. This resolves "Modbus exception 4: Slave device failure" errors. Root Cause: modbus-serial@8.0.23 ServerTCP/ServerSerial does not properly await Promise-based callbacks for read operations. The library's type definitions claim to support Promise-based callbacks, but the implementation doesn't actually await them, causing timeouts and exception 4 errors. See upstream issue: yaacov/node-modbus-serial#548 Changes: 1. Test that reproduces the bug - Added tcp.async-compat.test.ts and rtu.async-compat.test.ts with simulateModbusSerialCall helper demonstrating exact bug behavior 2. Fix implementation - Converted all read operations (getHoldingRegister, getInputRegister, getMultipleHoldingRegisters, getMultipleInputRegisters, getCoil, getDiscreteInput) to callback-style in both TCP and RTU transports 3. Refactor test duplication - Extracted callServiceVector helper into shared test-helpers.ts, removing 80 lines of duplication 4. Fix TypeScript compilation - Corrected callback signatures to match FCallbackVal<T> type (non-optional value parameter, proper error handling) 5. Increase test coverage - Added 18 tests for error handling and edge cases, bringing coverage to ~95% statements, ~73% branches, 100% functions Write operations (setRegister, setRegisterArray, setCoil) remain Promise-based as they work correctly with modbus-serial. Testing: - All 309 tests pass - New async-compat tests verify fix works with simulated modbus-serial behavior - No breaking changes to public API Follow-up issues created: #305, #306, #307, #308 Fixes #303 Related to yaacov/node-modbus-serial#548
1 parent bd591a8 commit 3e30034

9 files changed

Lines changed: 1018 additions & 92 deletions

File tree

‎packages/emulator/src/emulator.rtu.integration.test.ts‎

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import { describe, it, expect, beforeEach, afterEach, jest } from '@jest/globals'
1313

1414
import { ModbusEmulator } from './emulator.js'
15+
import { callServiceVector } from './transports/test-helpers.js'
1516

1617
// Store captured service vector for testing
1718
let capturedServiceVector: any = null
@@ -78,25 +79,43 @@ describe('ModbusEmulator RTU Integration', () => {
7879
})
7980

8081
it('should read single holding register via RTU service vector', async () => {
81-
const result = await capturedServiceVector.getHoldingRegister(0, 1)
82+
const result = await callServiceVector<number>(
83+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
84+
0,
85+
1
86+
)
8287

83-
expect(result).toEqual([230])
88+
expect(result).toEqual(230)
8489
})
8590

8691
it('should read multiple holding registers via RTU service vector', async () => {
87-
const result = await capturedServiceVector.getMultipleHoldingRegisters(0, 3, 1)
92+
const result = await callServiceVector<number[]>(
93+
capturedServiceVector.getMultipleHoldingRegisters.bind(capturedServiceVector),
94+
0,
95+
3,
96+
1
97+
)
8898

8999
expect(result).toEqual([230, 52, 1196])
90100
})
91101

92102
it('should read single input register via RTU service vector', async () => {
93-
const result = await capturedServiceVector.getInputRegister(0, 1)
103+
const result = await callServiceVector<number>(
104+
capturedServiceVector.getInputRegister.bind(capturedServiceVector),
105+
0,
106+
1
107+
)
94108

95-
expect(result).toEqual([500])
109+
expect(result).toEqual(500)
96110
})
97111

98112
it('should read multiple input registers via RTU service vector', async () => {
99-
const result = await capturedServiceVector.getMultipleInputRegisters(0, 2, 1)
113+
const result = await callServiceVector<number[]>(
114+
capturedServiceVector.getMultipleInputRegisters.bind(capturedServiceVector),
115+
0,
116+
2,
117+
1
118+
)
100119

101120
expect(result).toEqual([500, 60])
102121
})
@@ -118,15 +137,24 @@ describe('ModbusEmulator RTU Integration', () => {
118137
await capturedServiceVector.setRegister(0, 300, 1)
119138

120139
// Verify by reading back
121-
const result = await capturedServiceVector.getHoldingRegister(0, 1)
122-
expect(result).toEqual([300])
140+
const result = await callServiceVector<number>(
141+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
142+
0,
143+
1
144+
)
145+
expect(result).toEqual(300)
123146
})
124147

125148
it('should write multiple registers via RTU service vector', async () => {
126149
await capturedServiceVector.setRegisterArray(0, [500, 100, 200], 1)
127150

128151
// Verify by reading back
129-
const result = await capturedServiceVector.getMultipleHoldingRegisters(0, 3, 1)
152+
const result = await callServiceVector<number[]>(
153+
capturedServiceVector.getMultipleHoldingRegisters.bind(capturedServiceVector),
154+
0,
155+
3,
156+
1
157+
)
130158
expect(result).toEqual([500, 100, 200])
131159
})
132160
})
@@ -157,12 +185,20 @@ describe('ModbusEmulator RTU Integration', () => {
157185

158186
it('should route requests to correct device via unit ID', async () => {
159187
// Read from device 1
160-
const result1 = await capturedServiceVector.getHoldingRegister(0, 1)
161-
expect(result1).toEqual([100])
188+
const result1 = await callServiceVector<number>(
189+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
190+
0,
191+
1
192+
)
193+
expect(result1).toEqual(100)
162194

163195
// Read from device 2
164-
const result2 = await capturedServiceVector.getHoldingRegister(0, 2)
165-
expect(result2).toEqual([200])
196+
const result2 = await callServiceVector<number>(
197+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
198+
0,
199+
2
200+
)
201+
expect(result2).toEqual(200)
166202
})
167203
})
168204

@@ -178,7 +214,13 @@ describe('ModbusEmulator RTU Integration', () => {
178214
await emulator.start()
179215

180216
// Requesting from non-existent device 99 should throw
181-
await expect(capturedServiceVector.getHoldingRegister(0, 99)).rejects.toThrow()
217+
await expect(
218+
callServiceVector(
219+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
220+
0,
221+
99
222+
)
223+
).rejects.toThrow()
182224
})
183225
})
184226

‎packages/emulator/src/emulator.tcp.integration.test.ts‎

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { describe, it, expect, beforeEach, afterEach, jest } from '@jest/globals'
66

77
import { ModbusEmulator } from './emulator.js'
8+
import { callServiceVector } from './transports/test-helpers.js'
89

910
// Store captured service vector for testing
1011
let capturedServiceVector: any = null
@@ -72,13 +73,21 @@ describe('ModbusEmulator with TCP transport', () => {
7273
})
7374

7475
it('should read single holding register via TCP service vector', async () => {
75-
const result = await capturedServiceVector.getHoldingRegister(0, 1)
76-
expect(result).toEqual([230])
76+
const result = await callServiceVector<number>(
77+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
78+
0,
79+
1
80+
)
81+
expect(result).toEqual(230)
7782
})
7883

7984
it('should read single input register via TCP service vector', async () => {
80-
const result = await capturedServiceVector.getInputRegister(0, 1)
81-
expect(result).toEqual([52])
85+
const result = await callServiceVector<number>(
86+
capturedServiceVector.getInputRegister.bind(capturedServiceVector),
87+
0,
88+
1
89+
)
90+
expect(result).toEqual(52)
8291
})
8392

8493
it('should read multiple holding registers via TCP service vector', async () => {
@@ -89,7 +98,12 @@ describe('ModbusEmulator with TCP transport', () => {
8998
},
9099
})
91100

92-
const result = await capturedServiceVector.getMultipleHoldingRegisters(0, 2, 2)
101+
const result = await callServiceVector<number[]>(
102+
capturedServiceVector.getMultipleHoldingRegisters.bind(capturedServiceVector),
103+
0,
104+
2,
105+
2
106+
)
93107
expect(result).toEqual([100, 200])
94108
})
95109

@@ -101,7 +115,12 @@ describe('ModbusEmulator with TCP transport', () => {
101115
},
102116
})
103117

104-
const result = await capturedServiceVector.getMultipleInputRegisters(0, 2, 2)
118+
const result = await callServiceVector<number[]>(
119+
capturedServiceVector.getMultipleInputRegisters.bind(capturedServiceVector),
120+
0,
121+
2,
122+
2
123+
)
105124
expect(result).toEqual([150, 250])
106125
})
107126
})
@@ -126,8 +145,12 @@ describe('ModbusEmulator with TCP transport', () => {
126145
await capturedServiceVector.setRegister(0, 300, 1)
127146

128147
// Verify by reading back
129-
const result = await capturedServiceVector.getHoldingRegister(0, 1)
130-
expect(result).toEqual([300])
148+
const result = await callServiceVector<number>(
149+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
150+
0,
151+
1
152+
)
153+
expect(result).toEqual(300)
131154
})
132155

133156
it('should write multiple registers via TCP service vector', async () => {
@@ -141,7 +164,12 @@ describe('ModbusEmulator with TCP transport', () => {
141164
await capturedServiceVector.setRegisterArray(0, [400, 500], 2)
142165

143166
// Verify by reading back
144-
const result = await capturedServiceVector.getMultipleHoldingRegisters(0, 2, 2)
167+
const result = await callServiceVector<number[]>(
168+
capturedServiceVector.getMultipleHoldingRegisters.bind(capturedServiceVector),
169+
0,
170+
2,
171+
2
172+
)
145173
expect(result).toEqual([400, 500])
146174
})
147175
})
@@ -169,11 +197,19 @@ describe('ModbusEmulator with TCP transport', () => {
169197
})
170198

171199
it('should route requests to correct device via unit ID', async () => {
172-
const result1 = await capturedServiceVector.getHoldingRegister(0, 1)
173-
expect(result1).toEqual([100])
200+
const result1 = await callServiceVector<number>(
201+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
202+
0,
203+
1
204+
)
205+
expect(result1).toEqual(100)
174206

175-
const result2 = await capturedServiceVector.getHoldingRegister(0, 2)
176-
expect(result2).toEqual([200])
207+
const result2 = await callServiceVector<number>(
208+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
209+
0,
210+
2
211+
)
212+
expect(result2).toEqual(200)
177213
})
178214
})
179215

@@ -190,7 +226,13 @@ describe('ModbusEmulator with TCP transport', () => {
190226
await emulator.start()
191227

192228
// Requesting from non-existent device 99 should throw
193-
await expect(capturedServiceVector.getHoldingRegister(0, 99)).rejects.toThrow()
229+
await expect(
230+
callServiceVector(
231+
capturedServiceVector.getHoldingRegister.bind(capturedServiceVector),
232+
0,
233+
99
234+
)
235+
).rejects.toThrow()
194236
})
195237
})
196238

0 commit comments

Comments
 (0)