From d70d9060dacfda1561d6a306853bfc97375dc88d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 16:35:32 +0000 Subject: [PATCH 01/10] Add scripted-transport harness and Modbus golden-frame tests Adds a ScriptedTransport fake lower layer that records sent buffers and delivers scripted responses through Layer.forward, enabling full-stack protocol layer tests against exact wire bytes without a socket. The Modbus tests assert byte sequences taken from the worked examples in the MODBUS Application Protocol Specification V1.1b3, so encode and decode are checked against the spec rather than against each other. These tests currently fail, documenting known bugs fixed in the next commit: - writeSingleCoil sends 0x00FF instead of 0xFF00 for ON - writeMultipleCoils emits the WriteSingle layout (no quantity/byte count/bit packing) and mutates the caller's array - register writes use writeInt16BE, rejecting unsigned values >= 0x8000 - readHoldingRegisters(address, callback) treats the callback as the register count - the defragger only forwards the last complete frame when one chunk contains several https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- test/harness/ScriptedTransport.js | 57 +++++++ test/unit/defragger.test.js | 70 +++++++++ test/unit/modbus.layer.test.js | 244 ++++++++++++++++++++++++++++++ test/unit/modbus.pdu.test.js | 88 +++++++++++ 4 files changed, 459 insertions(+) create mode 100644 test/harness/ScriptedTransport.js create mode 100644 test/unit/defragger.test.js create mode 100644 test/unit/modbus.layer.test.js create mode 100644 test/unit/modbus.pdu.test.js diff --git a/test/harness/ScriptedTransport.js b/test/harness/ScriptedTransport.js new file mode 100644 index 0000000..1ec6597 --- /dev/null +++ b/test/harness/ScriptedTransport.js @@ -0,0 +1,57 @@ +import Layer from '../../src/layers/Layer.js'; + +/** + * An in-memory stand-in for the TCP transport layer. + * + * Records every buffer upper layers send and replies with scripted + * responses, so protocol layers can be tested against exact wire bytes + * without a socket. Incoming bytes are delivered through `forward()`, + * the same path the real TCP layer uses, so upper-layer defraggers + * are exercised exactly as in production. + */ +export default class ScriptedTransport extends Layer { + constructor(name = 'tcp') { + super(name, null); + /** Every buffer sent by upper layers, in order */ + this.sent = []; + /** Queued per-request handlers */ + this.handlers = []; + } + + /** + * Queue a handler for the next request. + * handler(requestBuffer, transport) may call transport.deliver() + * synchronously, asynchronously, or never. + */ + onNextRequest(handler) { + this.handlers.push(handler); + return this; + } + + /** Queue raw buffer(s) to deliver upward when the next request arrives */ + reply(...buffers) { + return this.onNextRequest(() => { + buffers.forEach((buffer) => this.deliver(buffer)); + }); + } + + /** Queue a no-response for the next request */ + ignoreNextRequest() { + return this.onNextRequest(() => {}); + } + + /** Deliver raw bytes to the upper layer, as if received from the network */ + deliver(buffer) { + this.forward(buffer); + } + + sendNextMessage() { + for (;;) { + const request = this.getNextRequest(); + if (request == null) break; + this.sent.push(request.message); + const handler = this.handlers.shift(); + if (handler) handler(request.message, this); + } + } +} diff --git a/test/unit/defragger.test.js b/test/unit/defragger.test.js new file mode 100644 index 0000000..7cf7784 --- /dev/null +++ b/test/unit/defragger.test.js @@ -0,0 +1,70 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import Defragger from '../../src/defragger.js'; + +/** + * Test protocol: 1-byte length prefix followed by that many payload bytes. + */ +function createDefragger() { + return new Defragger( + (data, offsetRef, length) => length >= 1 && length >= 1 + data.readUInt8(0), + (data) => 1 + data.readUInt8(0), + ); +} + +function frame(...payload) { + return Buffer.from([payload.length, ...payload]); +} + +describe('Defragger', () => { + it('returns null while a frame is incomplete', () => { + const defragger = createDefragger(); + assert.equal(defragger.defrag(Buffer.from([3, 1])), null); + assert.equal(defragger.defrag(Buffer.from([2])), null); + }); + + it('returns a frame once it is complete across chunks', () => { + const defragger = createDefragger(); + assert.equal(defragger.defrag(Buffer.from([3, 1])), null); + assert.deepEqual(defragger.defrag(Buffer.from([2, 3])), frame(1, 2, 3)); + assert.equal(defragger.defrag(), null); + }); + + it('returns an exactly-aligned single frame', () => { + const defragger = createDefragger(); + assert.deepEqual(defragger.defrag(frame(9, 8)), frame(9, 8)); + assert.equal(defragger.defrag(), null); + }); + + it('returns every frame when one chunk contains multiple frames', () => { + const defragger = createDefragger(); + const chunk = Buffer.concat([frame(1), frame(2, 3), frame(4, 5, 6)]); + assert.deepEqual(defragger.defrag(chunk), frame(1)); + assert.deepEqual(defragger.defrag(), frame(2, 3)); + assert.deepEqual(defragger.defrag(), frame(4, 5, 6)); + assert.equal(defragger.defrag(), null); + }); + + it('keeps a trailing partial frame buffered after complete frames', () => { + const defragger = createDefragger(); + const chunk = Buffer.concat([frame(1, 2), Buffer.from([2, 9])]); + assert.deepEqual(defragger.defrag(chunk), frame(1, 2)); + assert.equal(defragger.defrag(), null); + assert.deepEqual(defragger.defrag(Buffer.from([10])), frame(9, 10)); + }); + + it('reassembles frames delivered byte-by-byte', () => { + const defragger = createDefragger(); + const stream = Buffer.concat([frame(7, 8), frame(9)]); + const frames = []; + for (let i = 0; i < stream.length; i++) { + let result = defragger.defrag(stream.subarray(i, i + 1)); + while (result != null) { + frames.push(result); + result = defragger.defrag(); + } + } + assert.deepEqual(frames, [frame(7, 8), frame(9)]); + }); +}); diff --git a/test/unit/modbus.layer.test.js b/test/unit/modbus.layer.test.js new file mode 100644 index 0000000..7949fad --- /dev/null +++ b/test/unit/modbus.layer.test.js @@ -0,0 +1,244 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import Modbus from '../../src/layers/modbus/index.js'; +import ScriptedTransport from '../harness/ScriptedTransport.js'; + +/** + * Golden-frame tests for the Modbus TCP layer. + * + * Expected byte sequences are taken from the worked examples in the + * MODBUS Application Protocol Specification V1.1b3 (sections 6.1-6.12), + * wrapped in an MBAP header (transaction, protocol 0, length, unit 0xFF). + * They are intentionally hard-coded so that encode and decode are tested + * against the spec rather than against each other. + */ + +function hex(s) { + return Buffer.from(s.replace(/\s+/g, ''), 'hex'); +} + +function createStack(options) { + const transport = new ScriptedTransport(); + const layer = new Modbus(transport, options); + return { transport, layer }; +} + +function withTimeout(promise, label, ms = 1000) { + return new Promise((resolve, reject) => { + const handle = setTimeout(() => { + reject(new Error(`${label} did not settle within ${ms}ms`)); + }, ms); + promise.then( + (value) => { clearTimeout(handle); resolve(value); }, + (err) => { clearTimeout(handle); reject(err); }, + ); + }); +} + +describe('Modbus TCP layer: spec example frames', () => { + it('readCoils 20-38 (spec 6.1)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 01 03 cd 6b 05')); + const value = await layer.readCoils(0x0013, 0x13); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 01 0013 0013')]); + assert.deepEqual(value, [0xCD, 0x6B, 0x05]); + }); + + it('readDiscreteInputs 197-218 (spec 6.2)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 02 03 ac db 35')); + const value = await layer.readDiscreteInputs(0x00C4, 0x16); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 02 00c4 0016')]); + assert.deepEqual(value, [0xAC, 0xDB, 0x35]); + }); + + it('readHoldingRegisters 108-110 (spec 6.3)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0009 ff 03 06 022b 0000 0064')); + const value = await layer.readHoldingRegisters(0x006B, 3); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 03 006b 0003')]); + assert.deepEqual(value, [555, 0, 100]); + }); + + it('readInputRegisters 9 (spec 6.4)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0005 ff 04 02 000a')); + const value = await layer.readInputRegisters(0x0008, 1); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 04 0008 0001')]); + assert.deepEqual(value, [10]); + }); + + it('writeSingleCoil ON encodes 0xFF00 (spec 6.5)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 05 00ac ff00')); + const value = await withTimeout(layer.writeSingleCoil(0x00AC, true), 'writeSingleCoil'); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 05 00ac ff00')]); + assert.deepEqual(value, { address: 0x00AC, value: 0xFF00 }); + }); + + it('writeSingleCoil OFF encodes 0x0000', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 05 0007 0000')); + const value = await withTimeout(layer.writeSingleCoil(0x0007, false), 'writeSingleCoil'); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 05 0007 0000')]); + assert.deepEqual(value, { address: 0x0007, value: 0x0000 }); + }); + + it('writeSingleHoldingRegister (spec 6.6)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 06 0001 0003')); + const value = await layer.writeSingleHoldingRegister(0x0001, [3]); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 06 0001 0003')]); + assert.deepEqual(value, { address: 1, value: 3 }); + }); + + it('writeSingleHoldingRegister accepts unsigned values >= 0x8000', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 06 0002 9c40')); + const value = await withTimeout( + layer.writeSingleHoldingRegister(0x0002, [40000]), + 'writeSingleHoldingRegister', + ); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 06 0002 9c40')]); + assert.deepEqual(value, { address: 2, value: 40000 }); + }); + + it('writeMultipleCoils packs bits with quantity and byte count (spec 6.11)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 0f 0013 000a')); + const coils = [ + true, false, true, true, false, false, true, true, + true, false, + ]; + const value = await withTimeout(layer.writeMultipleCoils(0x0013, coils), 'writeMultipleCoils'); + assert.deepEqual(transport.sent, [hex('0001 0000 0009 ff 0f 0013 000a 02 cd 01')]); + assert.deepEqual(value, { address: 0x0013, count: 10 }); + }); + + it('writeMultipleCoils does not mutate the caller\'s array', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0006 ff 0f 0000 0002')); + const coils = [true, false]; + await withTimeout(layer.writeMultipleCoils(0, coils), 'writeMultipleCoils'); + assert.deepEqual(coils, [true, false]); + }); +}); + +describe('Modbus TCP layer: argument handling', () => { + it('readHoldingRegisters count defaults to 1', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0005 ff 03 02 0007')); + const value = await withTimeout(layer.readHoldingRegisters(0x0008), 'readHoldingRegisters'); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 03 0008 0001')]); + assert.deepEqual(value, [7]); + }); + + it('readHoldingRegisters(address, callback) treats the function as the callback', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0005 ff 03 02 0007')); + const value = await withTimeout(new Promise((resolve, reject) => { + layer.readHoldingRegisters(0x0008, (err, val) => (err ? reject(err) : resolve(val))); + }), 'readHoldingRegisters callback'); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 ff 03 0008 0001')]); + assert.deepEqual(value, [7]); + }); + + it('uses the unitID from constructor options', async () => { + const { transport, layer } = createStack({ unitID: 0x11 }); + transport.reply(hex('0001 0000 0005 11 04 02 000a')); + const value = await layer.readInputRegisters(0x0008, 1); + assert.deepEqual(transport.sent, [hex('0001 0000 0006 11 04 0008 0001')]); + assert.deepEqual(value, [10]); + }); +}); + +describe('Modbus TCP layer: exception responses', () => { + it('rejects with the spec error description (spec 7)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0003 ff 83 02')); + await assert.rejects( + withTimeout(layer.readHoldingRegisters(0xFFFF, 1), 'readHoldingRegisters'), + /Illegal data address/, + ); + }); + + it('rejects with the spec error description for illegal data value', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0003 ff 85 03')); + await assert.rejects( + withTimeout(layer.writeSingleCoil(0, true), 'writeSingleCoil'), + /Illegal data value/, + ); + }); +}); + +describe('Modbus TCP layer: framing', () => { + const response = hex('0001 0000 0009 ff 03 06 022b 0000 0064'); + + it('reassembles a response split across two chunks', async () => { + const { transport, layer } = createStack(); + transport.onNextRequest((request, t) => { + t.deliver(response.subarray(0, 4)); + t.deliver(response.subarray(4)); + }); + const value = await withTimeout(layer.readHoldingRegisters(0x006B, 3), 'split response'); + assert.deepEqual(value, [555, 0, 100]); + }); + + it('reassembles a response delivered byte-by-byte', async () => { + const { transport, layer } = createStack(); + transport.onNextRequest((request, t) => { + for (let i = 0; i < response.length; i++) { + t.deliver(response.subarray(i, i + 1)); + } + }); + const value = await withTimeout(layer.readHoldingRegisters(0x006B, 3), 'byte-by-byte response'); + assert.deepEqual(value, [555, 0, 100]); + }); + + it('handles two responses coalesced into one chunk', async () => { + const { transport, layer } = createStack(); + const response1 = hex('0001 0000 0005 ff 03 02 0001'); + const response2 = hex('0002 0000 0005 ff 03 02 0002'); + transport.ignoreNextRequest(); + transport.onNextRequest((request, t) => { + t.deliver(Buffer.concat([response1, response2])); + }); + const first = layer.readHoldingRegisters(0, 1); + const second = layer.readHoldingRegisters(1, 1); + assert.deepEqual( + await withTimeout(Promise.all([first, second]), 'coalesced responses'), + [[1], [2]], + ); + }); +}); + +describe('Modbus TCP layer: transactions', () => { + it('increments the transaction ID per request', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('0001 0000 0005 ff 03 02 0001')); + transport.reply(hex('0002 0000 0005 ff 03 02 0002')); + await layer.readHoldingRegisters(0, 1); + await layer.readHoldingRegisters(1, 1); + assert.deepEqual(transport.sent, [ + hex('0001 0000 0006 ff 03 0000 0001'), + hex('0002 0000 0006 ff 03 0001 0001'), + ]); + }); + + it('matches out-of-order responses to the right requests', async () => { + const { transport, layer } = createStack(); + transport.ignoreNextRequest(); + transport.onNextRequest((request, t) => { + t.deliver(hex('0002 0000 0005 ff 03 02 0002')); + t.deliver(hex('0001 0000 0005 ff 03 02 0001')); + }); + const first = layer.readHoldingRegisters(0, 1); + const second = layer.readHoldingRegisters(1, 1); + assert.deepEqual( + await withTimeout(Promise.all([first, second]), 'out-of-order responses'), + [[1], [2]], + ); + }); +}); diff --git a/test/unit/modbus.pdu.test.js b/test/unit/modbus.pdu.test.js new file mode 100644 index 0000000..a9433e7 --- /dev/null +++ b/test/unit/modbus.pdu.test.js @@ -0,0 +1,88 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import PDU from '../../src/core/modbus/pdu.js'; +import TCPFrame from '../../src/core/modbus/frames/tcp.js'; +import { Functions } from '../../src/core/modbus/constants.js'; + +/** + * Golden byte sequences from the MODBUS Application Protocol + * Specification V1.1b3 worked examples. + */ + +function hex(s) { + return Buffer.from(s.replace(/\s+/g, ''), 'hex'); +} + +describe('Modbus PDU encoding', () => { + it('encodes a read request (spec 6.3)', () => { + assert.deepEqual( + PDU.EncodeReadRequest(Functions.ReadHoldingRegisters, 0x006B, 3), + hex('03 006b 0003'), + ); + }); + + it('encodes a write request with unsigned 16-bit values', () => { + assert.deepEqual( + PDU.EncodeWriteRequest(Functions.WriteSingleHoldingRegister, 0x0002, [40000]), + hex('06 0002 9c40'), + ); + }); + + it('encodes negative values as two\'s complement', () => { + assert.deepEqual( + PDU.EncodeWriteRequest(Functions.WriteSingleHoldingRegister, 0x0000, [-1]), + hex('06 0000 ffff'), + ); + }); + + it('encodes write request values given as 2-byte buffers', () => { + assert.deepEqual( + PDU.EncodeWriteRequest(Functions.WriteSingleHoldingRegister, 0x0001, [hex('abcd')]), + hex('06 0001 abcd'), + ); + }); + + it('encodes a write multiple coils request (spec 6.11)', () => { + const coils = [ + true, false, true, true, false, false, true, true, + true, false, + ]; + assert.deepEqual( + PDU.EncodeWriteMultipleCoilsRequest(0x0013, coils), + hex('0f 0013 000a 02 cd 01'), + ); + }); +}); + +describe('Modbus PDU decoding', () => { + it('decodes a read holding registers response (spec 6.3)', () => { + const pdu = PDU.Decode(hex('03 06 022b 0000 0064'), { current: 0 }, 8); + assert.equal(pdu.error, undefined); + assert.deepEqual(pdu.value, [555, 0, 100]); + assert.equal(pdu.fn.code, Functions.ReadHoldingRegisters); + }); + + it('decodes an exception response (spec 7)', () => { + const pdu = PDU.Decode(hex('83 02'), { current: 0 }, 2); + assert.equal(pdu.error.code, 2); + assert.equal(pdu.error.message, 'Illegal data address'); + }); +}); + +describe('Modbus TCP frame', () => { + it('encodes the MBAP header (big-endian, length covers unit + PDU)', () => { + assert.deepEqual( + TCPFrame.Encode(0x0001, 0, 0xFF, hex('03 006b 0003')), + hex('0001 0000 0006 ff 03 006b 0003'), + ); + }); + + it('decodes a full frame', () => { + const packet = TCPFrame.Decode(hex('1234 0000 0005 11 03 02 0007'), { current: 0 }); + assert.equal(packet.transactionID, 0x1234); + assert.equal(packet.protocolID, 0); + assert.equal(packet.unitID, 0x11); + assert.deepEqual(packet.pdu.value, [7]); + }); +}); From fd29d052203146f9318162c23011f3840b1f1bac Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 16:37:49 +0000 Subject: [PATCH 02/10] Fix Modbus encoding bugs and defragger frame loss caught by golden tests - writeSingleCoil now encodes the spec-mandated 0xFF00 for ON; a conforming server rejects any other nonzero value with Illegal Data Value, so coils could never be switched on - writeMultipleCoils now emits the proper function 0x0F layout (quantity of outputs, byte count, LSB-first bit-packed coils) via a dedicated PDU encoder, and no longer mutates the caller's array - register writes use writeUInt16BE with masking, so unsigned values >= 0x8000 and negative two's-complement values both encode instead of throwing - read requests shuffle (address, callback) arguments and default the count to 1, so the documented callback form no longer encodes the callback function as the register count - Defragger now returns one frame per call (callable without new data to drain), and Layer.forwardTo loops until the buffer is exhausted; previously every complete frame except the last in a chunk was silently dropped, hanging the requests awaiting them Verified against the unit suite (198 passing) and both integration tests run locally against real servers: pymodbus and OpENer. https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- src/core/modbus/pdu.js | 23 ++++++++++++++++++++++- src/defragger.js | 19 ++++++++++++------- src/layers/Layer.js | 10 ++++++++-- src/layers/modbus/index.js | 20 +++++++++++++------- 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/core/modbus/pdu.js b/src/core/modbus/pdu.js index 8dd3f24..bde134e 100644 --- a/src/core/modbus/pdu.js +++ b/src/core/modbus/pdu.js @@ -46,7 +46,9 @@ export default class PDU { if (Buffer.isBuffer(value) && value.length === 2) { value.copy(buffer, offset, 0, 2); } else if (Number.isFinite(value)) { - buffer.writeInt16BE(value, offset); + /** registers are unsigned 16-bit on the wire; mask so negative + * inputs encode as two's complement instead of throwing */ + buffer.writeUInt16BE(value & 0xFFFF, offset); } else { throw new Error('Modbus write request error: currently supports buffer, array of 2-byte buffers, or array of finite numbers'); } @@ -58,6 +60,25 @@ export default class PDU { return buffer; } + /** + * Function 0x0F layout: fn(1), address(2), quantity of outputs(2), + * byte count(1), coil values packed LSB-first + */ + static EncodeWriteMultipleCoilsRequest(address, values) { + const byteCount = Math.ceil(values.length / 8); + const buffer = Buffer.alloc(6 + byteCount); + buffer.writeUInt8(Functions.WriteMultipleCoils, 0); + buffer.writeUInt16BE(address, 1); + buffer.writeUInt16BE(values.length, 3); + buffer.writeUInt8(byteCount, 5); + for (let i = 0; i < values.length; i++) { + if (values[i]) { + buffer[6 + (i >> 3)] |= 1 << (i & 0b111); + } + } + return buffer; + } + static Decode(buffer, offsetRef, pduLength) { const fn = PDU.Fn(buffer, offsetRef); const data = PDU.Data(buffer, offsetRef, pduLength); diff --git a/src/defragger.js b/src/defragger.js index d8ccabc..bdb5f85 100644 --- a/src/defragger.js +++ b/src/defragger.js @@ -8,22 +8,27 @@ export default class Defragger { this._lengthHandler = lengthHandler; } + /** + * Appends data, if given, and returns the next complete frame or null. + * Call again without data to drain any remaining buffered frames. + */ defrag(data) { - let defraggedData = null; - - this._dataLength += data.length; - this._data = Buffer.concat([this._data, data], this._dataLength); + if (data != null && data.length > 0) { + this._dataLength += data.length; + this._data = Buffer.concat([this._data, data], this._dataLength); + } - while ( + if ( this._dataLength > 0 && this._completeHandler(this._data, { current: 0 }, this._dataLength) ) { const length = this._lengthHandler(this._data, { current: 0 }); - defraggedData = this._data.slice(0, length); + const frame = this._data.slice(0, length); this._dataLength -= length; this._data = this._data.slice(length); + return frame; } - return defraggedData; + return null; } } diff --git a/src/layers/Layer.js b/src/layers/Layer.js index ec241fd..3043acb 100644 --- a/src/layers/Layer.js +++ b/src/layers/Layer.js @@ -145,8 +145,14 @@ export default class Layer extends EventEmitter { static forwardTo(layer, data, info, context) { if (layer._defragger != null) { // eslint-disable-line no-underscore-dangle - data = layer._defragger.defrag(data); // eslint-disable-line no-underscore-dangle - if (data == null) return; + /** one chunk may complete several frames; forward each one */ + let frame = layer._defragger.defrag(data); // eslint-disable-line no-underscore-dangle + while (frame != null) { + layer.emit('data', frame, info, context); + layer.handleData(frame, info, context); + frame = layer._defragger.defrag(); // eslint-disable-line no-underscore-dangle + } + return; } layer.emit('data', data, info, context); layer.handleData(data, info, context); diff --git a/src/layers/modbus/index.js b/src/layers/modbus/index.js index d811e14..5f710da 100644 --- a/src/layers/modbus/index.js +++ b/src/layers/modbus/index.js @@ -10,7 +10,6 @@ const { ReadInputRegisters, ReadHoldingRegisters, WriteSingleCoil, - WriteMultipleCoils, WriteSingleHoldingRegister, // WriteMultipleHoldingRegisters } = MB.Functions; @@ -22,6 +21,13 @@ const DefaultOptions = { }; function readRequest(self, fn, address, count, callback) { + if (typeof count === 'function' && callback == null) { + callback = count; // eslint-disable-line no-param-reassign + count = undefined; // eslint-disable-line no-param-reassign + } + if (count == null) { + count = 1; // eslint-disable-line no-param-reassign + } return CallbackPromise(callback, (resolver) => { self._send(PDU.EncodeReadRequest(fn, address, count), {}, resolver); }); @@ -92,20 +98,20 @@ export default class Modbus extends Layer { return readRequest(this, ReadInputRegisters, inputAddressing, count, callback); } - readHoldingRegisters(inputAddressing, count = 1, callback) { + readHoldingRegisters(inputAddressing, count, callback) { return readRequest(this, ReadHoldingRegisters, inputAddressing, count, callback); } writeSingleCoil(inputAddressing, value, callback) { - const values = [value ? 0x00FF : 0x0000]; + /** 0xFF00 is the only valid ON value for function 0x05 */ + const values = [value ? 0xFF00 : 0x0000]; return writeRequest(this, WriteSingleCoil, inputAddressing, values, callback); } writeMultipleCoils(inputAddressing, values, callback) { - for (let i = 0; i < values.length; i++) { - values[i] = values[i] ? 0x00FF : 0x0000; - } - return writeRequest(this, WriteMultipleCoils, inputAddressing, values, callback); + return CallbackPromise(callback, (resolver) => { + this._send(PDU.EncodeWriteMultipleCoilsRequest(inputAddressing, values), {}, resolver); + }); } writeSingleHoldingRegister(inputAddressing, values, callback) { From 116a0c39e2c6587283d5c090831d5691eb2b418c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 21:27:23 +0000 Subject: [PATCH 03/10] Add PCCC golden-frame tests for the DF1 command set Drives the PCCC layer through the scripted transport and asserts the exact DF1 wire bytes (CMD/STS/TNS/FNC, logical ASCII addressing, FLAG byte data descriptors) for typed read/write, word range read, diagnostic status, and echo, plus STS/EXT-STS error mapping and transaction matching. Three tests fail against the current code, documenting known bugs fixed in the next commit: - typedWrite passes the offset ref object instead of the offset number when encoding extended data descriptors, so float writes always throw ERR_INVALID_ARG_TYPE - typedWrite for timer/counter/control/string/long files reaches the same broken path through an undefined type id instead of failing with a clear unsupported-type error https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- test/unit/pccc.layer.test.js | 208 +++++++++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+) create mode 100644 test/unit/pccc.layer.test.js diff --git a/test/unit/pccc.layer.test.js b/test/unit/pccc.layer.test.js new file mode 100644 index 0000000..09e4cc9 --- /dev/null +++ b/test/unit/pccc.layer.test.js @@ -0,0 +1,208 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import PCCCLayer from '../../src/layers/pccc/index.js'; +import ScriptedTransport from '../harness/ScriptedTransport.js'; + +/** + * Golden-frame tests for the PCCC layer. + * + * Expected byte sequences follow the DF1 protocol reference manual + * (Allen-Bradley publication 1770-6.5.16): commands are + * CMD(1) STS(1) TNS(2, little-endian) FNC(1) followed by + * function-specific data; replies set 0x40 in CMD. Logical ASCII + * addresses are encoded as 0x00 0x24 '
' 0x00. Typed data uses + * the FLAG-byte descriptor scheme (type id in the high nibble, size in + * the low nibble, extended forms when a field exceeds 3/4 bits). + */ + +function hex(s) { + return Buffer.from(s.replace(/\s+/g, ''), 'hex'); +} + +function createStack() { + const transport = new ScriptedTransport(); + const layer = new PCCCLayer(transport); + return { transport, layer }; +} + +function withTimeout(promise, label, ms = 1000) { + return new Promise((resolve, reject) => { + const handle = setTimeout(() => { + reject(new Error(`${label} did not settle within ${ms}ms`)); + }, ms); + promise.then( + (value) => { clearTimeout(handle); resolve(value); }, + (err) => { clearTimeout(handle); reject(err); }, + ); + }); +} + +describe('PCCC layer: typed read', () => { + it('reads an integer file element (CMD 0x0F, FNC 0x68)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4f00 0100 42 0500')); + const value = await withTimeout(layer.typedRead('N7:1'), 'typedRead'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 68 0000 0100 0024 4e373a31 00 0100'), + ]); + assert.equal(value, 5); + }); + + it('reads multiple integer elements as an array', async () => { + const { transport, layer } = createStack(); + /** array descriptor (0x97 0x09, 7 bytes) containing INT descriptor 0x42 */ + transport.reply(hex('4f00 0100 9709 42 0000 feff ff00')); + const value = await withTimeout(layer.typedRead('N7:0', 3), 'typedRead'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 68 0000 0300 0024 4e373a30 00 0300'), + ]); + assert.deepEqual(value, [0, -2, 255]); + }); + + it('reads a float file element (extended type id descriptor 0x94 0x08)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4f00 0100 9408 0000c03f')); + const value = await withTimeout(layer.typedRead('F8:0'), 'typedRead'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 68 0000 0100 0024 46383a30 00 0100'), + ]); + assert.equal(value, 1.5); + }); + + it('reads a timer element into a structured value', async () => { + const { transport, layer } = createStack(); + /** descriptor 0x56 (Timer, 6 bytes): EN set, PRE 1000, ACC 500 */ + transport.reply(hex('4f00 0100 56 0080 e803 f401')); + const value = await withTimeout(layer.typedRead('T4:0'), 'typedRead'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 68 0000 0100 0024 54343a30 00 0100'), + ]); + assert.deepEqual(value, { + EN: true, TT: false, DN: false, PRE: 1000, ACC: 500, + }); + }); +}); + +describe('PCCC layer: typed write', () => { + it('writes an integer file element (CMD 0x0F, FNC 0x67)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4f00 0100')); + const reply = await withTimeout(layer.typedWrite('N7:3', 5), 'typedWrite'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 67 0000 0100 0024 4e373a33 00 42 0500'), + ]); + assert.equal(reply.status.code, 0); + }); + + it('writes a float file element (extended type id descriptor 0x94 0x08)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4f00 0100')); + const reply = await withTimeout(layer.typedWrite('F8:0', 1.5), 'typedWrite'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 67 0000 0100 0024 46383a30 00 9408 0000c03f'), + ]); + assert.equal(reply.status.code, 0); + }); + + it('rejects writes to timer files with a clear error', async () => { + const { layer } = createStack(); + await assert.rejects( + withTimeout(layer.typedWrite('T4:0', 1), 'typedWrite'), + /not currently supported/, + ); + }); + + it('rejects writes to long files with a clear error', async () => { + const { layer } = createStack(); + await assert.rejects( + withTimeout(layer.typedWrite('L9:0', 1), 'typedWrite'), + /not currently supported/, + ); + }); + + it('rejects unknown address prefixes', async () => { + const { layer } = createStack(); + await assert.rejects( + withTimeout(layer.typedWrite('Q2:0', 1), 'typedWrite'), + /Unsupported address/, + ); + }); +}); + +describe('PCCC layer: other commands', () => { + it('word range read (CMD 0x0F, FNC 0x01) resolves the raw data', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4f00 0100 3412 7856')); + const value = await withTimeout(layer.wordRangeRead('N7:0', 2), 'wordRangeRead'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 01 0000 0200 0024 4e373a30 00 04'), + ]); + assert.deepEqual(value, hex('3412 7856')); + }); + + it('diagnostic status (CMD 0x06, FNC 0x03)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4600 0100 ee31c0')); + const value = await withTimeout(layer.diagnosticStatus(), 'diagnosticStatus'); + assert.deepEqual(transport.sent, [hex('0600 0100 03')]); + assert.deepEqual(value, hex('ee31c0')); + }); + + it('echo (CMD 0x06, FNC 0x00)', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4600 0100 dead')); + const value = await withTimeout(layer.echo(hex('dead')), 'echo'); + assert.deepEqual(transport.sent, [hex('0600 0100 00 dead')]); + assert.deepEqual(value, hex('dead')); + }); +}); + +describe('PCCC layer: error replies', () => { + it('rejects with the STS description', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4f10 0100')); + await assert.rejects( + withTimeout(layer.typedRead('N7:0'), 'typedRead'), + /Illegal command or format/, + ); + }); + + it('rejects with the EXT STS description when STS is 0xF0', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4ff0 0100 11')); + await assert.rejects( + withTimeout(layer.typedRead('N7:0'), 'typedRead'), + /Illegal data type/, + ); + }); +}); + +describe('PCCC layer: transactions', () => { + it('increments the transaction number per request', async () => { + const { transport, layer } = createStack(); + transport.reply(hex('4f00 0100 42 0100')); + transport.reply(hex('4f00 0200 42 0200')); + await withTimeout(layer.typedRead('N7:0'), 'first typedRead'); + await withTimeout(layer.typedRead('N7:1'), 'second typedRead'); + assert.deepEqual(transport.sent, [ + hex('0f00 0100 68 0000 0100 0024 4e373a30 00 0100'), + hex('0f00 0200 68 0000 0100 0024 4e373a31 00 0100'), + ]); + }); + + it('matches out-of-order replies by transaction number', async () => { + const { transport, layer } = createStack(); + transport.ignoreNextRequest(); + transport.onNextRequest((request, t) => { + t.deliver(hex('4f00 0200 42 0200')); + t.deliver(hex('4f00 0100 42 0100')); + }); + const first = layer.typedRead('N7:0'); + const second = layer.typedRead('N7:1'); + assert.deepEqual( + await withTimeout(Promise.all([first, second]), 'out-of-order replies'), + [1, 2], + ); + }); +}); From a493d9d6fcdee23a0c48e62408658f3549047719 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 21:27:53 +0000 Subject: [PATCH 04/10] Fix PCCC typed write encoding bugs caught by golden tests - EncodeDataDescriptor passed the offset ref object instead of the offset number in the extended-type-id branch, so every typed write whose type id needs an extension byte (notably floats, id 0x08) threw ERR_INVALID_ARG_TYPE instead of writing - TypedWriteRequest now rejects timer/counter/control/string/long addresses with a clear unsupported-type error; previously the undefined type id flowed into the descriptor encoder and produced the same cryptic TypeError - PCCC internal replies no longer fall through to forward() when the context callback was already consumed https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- src/layers/pccc/encoding.js | 2 +- src/layers/pccc/index.js | 4 +++- src/layers/pccc/packet.js | 4 ++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/layers/pccc/encoding.js b/src/layers/pccc/encoding.js index f564256..b70c580 100644 --- a/src/layers/pccc/encoding.js +++ b/src/layers/pccc/encoding.js @@ -65,7 +65,7 @@ export function EncodeDataDescriptor(data, offsetRef, id, size) { } if (idLength > 0 && sizeLength === 0) { - offsetRef.current = data.writeUInt8(((0b1000 | idLength) << 4) | size, offsetRef); + offsetRef.current = data.writeUInt8(((0b1000 | idLength) << 4) | size, offsetRef.current); offsetRef.current = encodeUnsignedInteger(data, offsetRef.current, id, idLength); return; } diff --git a/src/layers/pccc/index.js b/src/layers/pccc/index.js index 81dd823..7781ed8 100644 --- a/src/layers/pccc/index.js +++ b/src/layers/pccc/index.js @@ -248,8 +248,10 @@ export default class PCCCLayer extends Layer { const callback = this.callbackForContext(savedContext.context); if (callback != null) { callback(getError(packet.status), packet, info); - return; } + /** internal replies must never be forwarded to upper layers, + * even if the callback was already consumed */ + return; } this.forward(packet.data, info, savedContext.context); diff --git a/src/layers/pccc/packet.js b/src/layers/pccc/packet.js index 4181f3b..99ce008 100644 --- a/src/layers/pccc/packet.js +++ b/src/layers/pccc/packet.js @@ -229,6 +229,10 @@ export default class PCCCPacket { throw new Error(`Unsupported address: ${address}`); } + if (info.id == null) { + throw new Error(`Writing to ${info.datatype} files is not currently supported (address: ${address})`); + } + const valueCount = values.length; const dataValueLength = valueCount * info.size; const dataTypeLength = DataTypeEncodingLength(info.id, info.size); From f3de38c3b9fd4ae91bf2cb9e4f9f08362e3582d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 21:40:00 +0000 Subject: [PATCH 05/10] Add Logix5000 layer tests over an emulated CIP connection Extends the scripted transport with request objects, contextual delivery, and a fallback responder, and adds a LogixResponder harness that answers ForwardOpen/ForwardClose and routes connected messages to scripted Message Router responses, so the full Logix5000 -> CIPConnectionLayer stack runs against exact wire bytes. Covers the ForwardOpen/ForwardClose handshake, Read Tag (atomic DINT/ REAL, arrays, fragmented transfers, error statuses), Write Tag type lookup, Read Modify Write, request timeouts, and the connection keep-alive lifecycle. Most of these tests fail against the current code, documenting known bugs fixed in the next commit: - readTag throws for every non-empty response (stale EPath.Decode call) - fragmented reads conflate the request offset with the reply parse offset and fail beyond the first fragment - tag cache keys collapse to one key per scope, so writeTag encodes with the wrong tag's data type - object-mapped CIP statuses without extended status crash response decoding - request timeouts are discarded, so lost responses hang forever - the connected keep-alive interval leaks after destroy - mask range validation compares the masks array instead of its values https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- test/harness/LogixResponder.js | 97 +++++++++++++ test/harness/ScriptedTransport.js | 23 ++- test/unit/logix5000.layer.test.js | 224 ++++++++++++++++++++++++++++++ 3 files changed, 338 insertions(+), 6 deletions(-) create mode 100644 test/harness/LogixResponder.js create mode 100644 test/unit/logix5000.layer.test.js diff --git a/test/harness/LogixResponder.js b/test/harness/LogixResponder.js new file mode 100644 index 0000000..2a6f270 --- /dev/null +++ b/test/harness/LogixResponder.js @@ -0,0 +1,97 @@ +/** + * Emulates the CIP connection handshake of a Logix controller on top of + * a ScriptedTransport, so the Logix5000/CIPConnectionLayer stack can be + * tested against scripted Message Router responses. + * + * - ForwardOpen (0x54) requests are answered with a success response + * echoing the connection IDs, serial numbers, and packet rates from + * the request. + * - ForwardClose (0x4E) requests are answered with a success response. + * - Connected messages (2-byte sequence count + MR request) are matched + * against the queued connected handlers in order; the response payload + * is sent back with the same sequence count and connection IDs. + */ +export default class LogixResponder { + constructor(transport) { + this.transport = transport; + /** Message Router request buffers from connected messages (sequence stripped) */ + this.connectedRequests = []; + this.connectedHandlers = []; + this.forwardOpenRequests = []; + this.forwardCloseRequests = []; + this.sendInfo = null; + transport.respond((message, t, request) => this.handle(message, t, request)); + } + + /** Queue Message Router response payload(s) for the next connected request(s) */ + replyToConnected(...payloads) { + payloads.forEach((payload) => this.connectedHandlers.push(() => payload)); + return this; + } + + /** Queue a function (mrRequestBuffer) => responsePayload | null */ + onConnected(handler) { + this.connectedHandlers.push(handler); + return this; + } + + /** Queue a no-response for the next connected request */ + ignoreConnected() { + return this.onConnected(() => null); + } + + handle(message, transport, request) { + if (request.context != null) { + /** Unconnected Message Router request */ + const service = message.readUInt8(0); + if (service === 0x54) { + /** + * ForwardOpen request layout (after service, path size, 4-byte path): + * timing(2) @6, OtoT connection ID @8, TtoO connection ID @12, + * serial(2) vendor(2) originator serial(4) @16, + * O->T RPI @28, T->O RPI @34 + */ + this.forwardOpenRequests.push(message); + const data = Buffer.alloc(26); + message.copy(data, 0, 8, 16); /** echo OtoT + TtoO connection IDs */ + message.copy(data, 8, 16, 24); /** echo serial, vendor, originator serial */ + message.copy(data, 16, 28, 32); /** O->T RPI as actual packet rate */ + message.copy(data, 20, 34, 38); /** T->O RPI as actual packet rate */ + this.sendInfo = { + connectionID: message.readUInt32LE(8), + responseID: message.readUInt32LE(12), + }; + transport.deliver( + Buffer.concat([Buffer.from([0xD4, 0x00, 0x00, 0x00]), data]), + null, + request.context, + ); + } else if (service === 0x4E) { + /** ForwardClose: serial(2) vendor(2) originator serial(4) @8 */ + this.forwardCloseRequests.push(message); + const data = Buffer.alloc(10); + message.copy(data, 0, 8, 16); + transport.deliver( + Buffer.concat([Buffer.from([0xCE, 0x00, 0x00, 0x00]), data]), + null, + request.context, + ); + } else { + throw new Error(`LogixResponder: unexpected unconnected service 0x${service.toString(16)}`); + } + return; + } + + /** Connected message: 2-byte sequence count + MR request */ + const sequence = message.subarray(0, 2); + const mrRequest = message.subarray(2); + this.connectedRequests.push(mrRequest); + const handler = this.connectedHandlers.shift(); + if (handler) { + const payload = handler(mrRequest); + if (payload != null) { + this.transport.deliver(Buffer.concat([sequence, payload]), this.sendInfo); + } + } + } +} diff --git a/test/harness/ScriptedTransport.js b/test/harness/ScriptedTransport.js index 1ec6597..f598860 100644 --- a/test/harness/ScriptedTransport.js +++ b/test/harness/ScriptedTransport.js @@ -14,14 +14,18 @@ export default class ScriptedTransport extends Layer { super(name, null); /** Every buffer sent by upper layers, in order */ this.sent = []; + /** Every queued request object ({ layer, info, message, context }), in order */ + this.requests = []; /** Queued per-request handlers */ this.handlers = []; + /** Fallback handler used when no queued handler exists */ + this.responder = null; } /** * Queue a handler for the next request. - * handler(requestBuffer, transport) may call transport.deliver() - * synchronously, asynchronously, or never. + * handler(requestBuffer, transport, requestObject) may call + * transport.deliver() synchronously, asynchronously, or never. */ onNextRequest(handler) { this.handlers.push(handler); @@ -40,9 +44,15 @@ export default class ScriptedTransport extends Layer { return this.onNextRequest(() => {}); } + /** Set a fallback handler called for any request with no queued handler */ + respond(handler) { + this.responder = handler; + return this; + } + /** Deliver raw bytes to the upper layer, as if received from the network */ - deliver(buffer) { - this.forward(buffer); + deliver(buffer, info, context) { + this.forward(buffer, info, context); } sendNextMessage() { @@ -50,8 +60,9 @@ export default class ScriptedTransport extends Layer { const request = this.getNextRequest(); if (request == null) break; this.sent.push(request.message); - const handler = this.handlers.shift(); - if (handler) handler(request.message, this); + this.requests.push(request); + const handler = this.handlers.shift() || this.responder; + if (handler) handler(request.message, this, request); } } } diff --git a/test/unit/logix5000.layer.test.js b/test/unit/logix5000.layer.test.js new file mode 100644 index 0000000..ab6b3d3 --- /dev/null +++ b/test/unit/logix5000.layer.test.js @@ -0,0 +1,224 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import Logix5000 from '../../src/layers/cip/layers/Logix5000/index.js'; +import CIPRequest from '../../src/layers/cip/core/request.js'; +import EPath from '../../src/layers/cip/core/epath/index.js'; +import ScriptedTransport from '../harness/ScriptedTransport.js'; +import LogixResponder from '../harness/LogixResponder.js'; + +/** + * Tests for the Logix5000 layer over an emulated CIP connection. + * + * Wire bytes follow the Logix5000 Data Access reference (Rockwell + * publication 1756-PM020) and CIP Vol 1: symbol services 0x4C (Read Tag), + * 0x52 (Read Tag Fragmented), 0x4D (Write Tag), 0x4E (Read Modify Write), + * 0x55 (Get Instance Attribute List); ANSI extended symbolic paths + * (0x91, length, name); type codes 0xC4 (DINT) and 0xCA (REAL). + */ + +function hex(s) { + return Buffer.from(s.replace(/\s+/g, ''), 'hex'); +} + +function createStack() { + const transport = new ScriptedTransport(); + const responder = new LogixResponder(transport); + const layer = new Logix5000(transport); + return { transport, responder, layer }; +} + +function withTimeout(promise, label, ms = 2000) { + return new Promise((resolve, reject) => { + const handle = setTimeout(() => { + reject(new Error(`${label} did not settle within ${ms}ms`)); + }, ms); + promise.then( + (value) => { clearTimeout(handle); resolve(value); }, + (err) => { clearTimeout(handle); reject(err); }, + ); + }); +} + +describe('Logix5000: connection handshake', () => { + it('opens with ForwardOpen and closes with ForwardClose', async () => { + const { transport, responder, layer } = createStack(); + responder.replyToConnected(hex('cc 00 0000 c400 39300000')); + await withTimeout(layer.readTag('TagA', 1), 'readTag'); + + assert.equal(responder.forwardOpenRequests.length, 1); + const forwardOpen = responder.forwardOpenRequests[0]; + /** service, path size 2 words, Connection Manager class 0x06 instance 1 */ + assert.deepEqual(forwardOpen.subarray(0, 6), hex('54 02 2006 2401')); + + await withTimeout(transport.close(), 'close'); + assert.equal(responder.forwardCloseRequests.length, 1); + assert.deepEqual(responder.forwardCloseRequests[0].subarray(0, 6), hex('4e 02 2006 2401')); + }); +}); + +describe('Logix5000: readTag', () => { + it('reads an atomic DINT tag', async () => { + const { transport, responder, layer } = createStack(); + responder.replyToConnected(hex('cc 00 0000 c400 39300000')); + const value = await withTimeout(layer.readTag('TagA', 1), 'readTag'); + assert.deepEqual(responder.connectedRequests, [ + hex('4c 03 9104 54616741 0100'), + ]); + assert.equal(value, 12345); + await transport.close(); + }); + + it('reads an atomic REAL tag', async () => { + const { transport, responder, layer } = createStack(); + responder.replyToConnected(hex('cc 00 0000 ca00 0000c03f')); + const value = await withTimeout(layer.readTag('Speed', 1), 'readTag'); + assert.deepEqual(responder.connectedRequests, [ + hex('4c 04 9105 5370656564 00 0100'), + ]); + assert.equal(value, 1.5); + await transport.close(); + }); + + it('reads multiple elements as an array', async () => { + const { transport, responder, layer } = createStack(); + responder.replyToConnected(hex('cc 00 0000 c400 01000000 02000000 03000000')); + const value = await withTimeout(layer.readTag('TagA', 3), 'readTag'); + assert.deepEqual(responder.connectedRequests, [ + hex('4c 03 9104 54616741 0300'), + ]); + assert.deepEqual(value, [1, 2, 3]); + await transport.close(); + }); + + it('reads a large tag with fragmented transfers', async () => { + const { transport, responder, layer } = createStack(); + /** initial read replies with partial transfer (status 0x06) */ + responder.replyToConnected(hex('cc 00 0600 c400 01000000')); + /** first fragment: partial, elements 1 and 2 */ + responder.replyToConnected(hex('d2 00 0600 c400 01000000 02000000')); + /** second fragment: complete, element 3 */ + responder.replyToConnected(hex('d2 00 0000 c400 03000000')); + + const value = await withTimeout(layer.readTag('TagA', 3), 'readTag fragmented'); + + assert.deepEqual(responder.connectedRequests, [ + hex('4c 03 9104 54616741 0300'), + /** fragmented read from byte offset 0 */ + hex('52 03 9104 54616741 0300 00000000'), + /** fragmented read resumes at byte offset 8 (2 DINTs received) */ + hex('52 03 9104 54616741 0300 08000000'), + ]); + assert.deepEqual(value, [1, 2, 3]); + await transport.close(); + }); + + it('rejects with the CIP status description on error replies', async () => { + const { transport, responder, layer } = createStack(); + responder.replyToConnected(hex('cc 00 0500')); + await assert.rejects( + withTimeout(layer.readTag('TagA', 1), 'readTag'), + /Request Path destination unknown/, + ); + await transport.close(); + }); + + it('rejects cleanly when an object-mapped status has no extended status', async () => { + const { transport, responder, layer } = createStack(); + /** status 0xFF maps to extended descriptions; reply carries none */ + responder.replyToConnected(hex('cc 00 ff00')); + await assert.rejects( + withTimeout(layer.readTag('TagA', 1), 'readTag'), + /CIP Error/, + ); + await transport.close(); + }); +}); + +describe('Logix5000: writeTag', () => { + it('looks up each tag\'s own type before writing', async () => { + const { transport, responder, layer } = createStack(); + + /** instance attribute list (names) starting at instance 0: TagA(1), TagB(2) */ + responder.onConnected((mr) => { + assert.deepEqual(mr, hex('55 02 206b 2400 0100 0100')); + return hex('d5 00 0000 01000000 0400 54616741 02000000 0400 54616742'); + }); + /** instance attribute list (types) starting at instance 2: TagB is REAL */ + responder.onConnected((mr) => { + assert.deepEqual(mr, hex('55 02 206b 2402 0100 0200')); + return hex('d5 00 0000 02000000 ca00'); + }); + /** write TagB as REAL 1.5 */ + responder.onConnected((mr) => { + assert.deepEqual(mr, hex('4d 03 9104 54616742 ca00 0100 0000c03f')); + return hex('cd 00 0000'); + }); + await withTimeout(layer.writeTag('TagB', 1.5), 'writeTag TagB'); + + /** instance attribute list (types) starting at instance 1: TagA is DINT */ + responder.onConnected((mr) => { + assert.deepEqual(mr, hex('55 02 206b 2401 0100 0200')); + return hex('d5 00 0000 01000000 c400 02000000 ca00'); + }); + /** write TagA as DINT 99 — not with TagB's cached REAL type */ + responder.onConnected((mr) => { + assert.deepEqual(mr, hex('4d 03 9104 54616741 c400 0100 63000000')); + return hex('cd 00 0000'); + }); + await withTimeout(layer.writeTag('TagA', 99), 'writeTag TagA'); + + assert.equal(responder.connectedRequests.length, 5); + await transport.close(); + }); +}); + +describe('Logix5000: readModifyWriteTag', () => { + it('encodes the masks (1756-PM020 Read Modify Write)', async () => { + const { transport, responder, layer } = createStack(); + responder.replyToConnected(hex('ce 00 0000')); + await withTimeout(layer.readModifyWriteTag('TagA', [0x0F], [0xF0]), 'readModifyWriteTag'); + assert.deepEqual(responder.connectedRequests, [ + hex('4e 03 9104 54616741 0100 0f f0'), + ]); + await transport.close(); + }); + + it('rejects mask values above 255 with a clear error', async () => { + const { transport, layer } = createStack(); + await assert.rejects( + withTimeout(layer.readModifyWriteTag('TagA', [0x1FF, 0x00], [0xFF, 0xFF]), 'readModifyWriteTag'), + /Values in masks must be/, + ); + await transport.close(); + }); +}); + +describe('Logix5000: request timeouts', () => { + it('rejects when no response arrives within the timeout', async () => { + const { transport, responder, layer } = createStack(); + responder.ignoreConnected(); + const path = EPath.Encode(true, EPath.ConvertSymbolToSegments('TagA')); + const request = new CIPRequest(0x4C, path, hex('0100')); + await assert.rejects( + withTimeout(layer.sendRequest(true, request, null, 50), 'sendRequest'), + /Timeout/, + ); + await transport.close(); + }); +}); + +describe('CIP connection layer: resend keep-alive', () => { + it('stops the keep-alive interval when the layer is destroyed', async () => { + const { transport, responder, layer } = createStack(); + responder.replyToConnected(hex('cc 00 0000 c400 39300000')); + await withTimeout(layer.readTag('TagA', 1), 'readTag'); + + const connectionLayer = layer.lowerLayer; + assert.ok(connectionLayer.__resendInterval != null, 'keep-alive interval should be armed'); + + connectionLayer.destroy('test'); + assert.equal(connectionLayer.__resendInterval, null, 'keep-alive interval should be cleared on destroy'); + await transport.close(); + }); +}); From d55770689e28834f587b56fd6b4454eff7692dbe Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 21:41:11 +0000 Subject: [PATCH 06/10] Fix Logix5000 read/write path bugs caught by the new layer tests - parseReadTag called Logix5000DecodeDataType with a numeric offset and a callback, both stale from the offset-ref refactor of EPath.Decode; the data type was never decoded so every readTag with a non-empty response threw. It now decodes through an offset ref and uses the returned segment's value. - readTagFragmented reused one offset ref as both the fragmented-read request offset and the reply parse cursor, so any tag needing more than one fragment failed or returned corrupted data; the request offset is now tracked separately and accumulates per fragment. - scopedGenerator returned an arrow function spreading the outer function's arguments, collapsing every tag cache key in a scope to a single entry; symbol instance IDs and cached types could belong to a different tag, making writeTag encode with the wrong data type. It now builds keys from its own parameters. - statusHandler indexed object-mapped status descriptions with extended.readUInt16LE(0) whenever extended.length >= 0, crashing on empty extended status buffers; it now requires two bytes and falls back to the generic description. - parseReadTagMemberStructure returned an Error object as if it were decoded data; it now throws. - readModifyWriteTag validated the masks array instead of its elements, so out-of-range values produced a cryptic buffer RangeError. - Request timeouts are now plumbed through Logix5000 send -> CIPInternalLayer.sendRequest -> contextCallback instead of being discarded, so lost responses reject instead of hanging forever (this also makes the listTags retry/backoff logic reachable). - CIPConnectionLayer.handleDestroy now stops the connected keep-alive resend interval, which previously kept firing into the destroyed stack. https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- src/layers/cip/layers/Logix5000/index.js | 45 ++++++++++--------- .../cip/layers/internal/CIPConnectionLayer.js | 1 + .../cip/layers/internal/CIPInternalLayer.js | 4 +- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/layers/cip/layers/Logix5000/index.js b/src/layers/cip/layers/Logix5000/index.js index a8afd1b..fc14396 100644 --- a/src/layers/cip/layers/Logix5000/index.js +++ b/src/layers/cip/layers/Logix5000/index.js @@ -53,14 +53,14 @@ import { const DEFAULT_SCOPE = '__DEFAULT_GLOBAL_SCOPE__'; -function Logix5000DecodeDataType(buffer, offsetRef, cb) { +function Logix5000DecodeDataType(buffer, offsetRef) { const startingOffset = offsetRef.current; - const type = EPath.Decode(buffer, offsetRef, null, false, cb); + const segment = EPath.Decode(buffer, offsetRef, null, false); /** TODO: Why is this necessary? */ if (offsetRef.current - startingOffset < 2) { offsetRef.current += 1; } - return type; + return segment; } async function readTagFragmented(layer, path, elements) { @@ -69,20 +69,21 @@ async function readTagFragmented(layer, path, elements) { const reqData = Buffer.allocUnsafe(6); reqData.writeUInt16LE(elements, 0); - const offsetRef = { current: 0 }; + let requestOffset = 0; const chunks = []; while (true) { - reqData.writeUInt32LE(offsetRef.current, 2); + reqData.writeUInt32LE(requestOffset, 2); const reply = await sendPromise(layer, service, path, reqData, 5000); - /** remove the tag type bytes if already received */ + /** each reply starts with the tag type; keep it on the first chunk only */ + const offsetRef = { current: 0 }; Logix5000DecodeDataType(reply.data, offsetRef); const dataTypeOffset = offsetRef.current; chunks.push(chunks.length > 0 ? reply.data.slice(dataTypeOffset) : reply.data); if (reply.status.code === GeneralStatusCodes.PartialTransfer) { - offsetRef.current = reply.data.length - dataTypeOffset; + requestOffset += reply.data.length - dataTypeOffset; } else if (reply.status.code === 0) { break; } else { @@ -101,7 +102,7 @@ async function parseReadTagMemberStructure(layer, structureType, data, offset) { const template = await layer.readTemplate(structureType.template.id); if (!template || !Array.isArray(template.members)) { - return new Error(`Unable to read template: ${structureType.template.id}`); + throw new Error(`Unable to read template: ${structureType.template.id}`); } const { members } = template; @@ -153,15 +154,15 @@ async function parseReadTag(layer, scope, tag, elements, data) { return undefined; } - let typeInfo; - const offset = Logix5000DecodeDataType(data, 0, (val) => { typeInfo = val.value; }); + const offsetRef = { current: 0 }; + const typeSegment = Logix5000DecodeDataType(data, offsetRef); + const typeInfo = typeSegment ? typeSegment.value : undefined; if (!typeInfo) { throw new Error('Unable to decode data type from read tag response data'); } const values = []; - const offsetRef = { current: offset }; if (!typeInfo.constructed || typeInfo.abbreviated === false) { for (let i = 0; i < elements; i++) { @@ -225,8 +226,12 @@ async function parseReadTag(layer, scope, tag, elements, data) { function statusHandler(code, extended, cb) { let error = GenericServiceStatusDescriptions[code]; - if (typeof error === 'object' && Buffer.isBuffer(extended) && extended.length >= 0) { - error = error[extended.readUInt16LE(0)]; + if (typeof error === 'object') { + if (Buffer.isBuffer(extended) && extended.length >= 2) { + error = error[extended.readUInt16LE(0)]; + } else { + error = undefined; + } } if (error) { cb(null, error); @@ -234,14 +239,14 @@ function statusHandler(code, extended, cb) { } /** Use driver specific error handling if exists */ -async function send(self, service, path, data, callback /* , timeout */) { +async function send(self, service, path, data, callback, timeout) { try { const request = new CIPRequest(service, path, data, null, { serviceNames: SymbolServiceNames, statusHandler, }); - const response = await self.sendRequest(true, request); + const response = await self.sendRequest(true, request, null, timeout); // console.log(response); if (response.status.error) { callback(response.status.description, response); @@ -395,11 +400,11 @@ function parseTemplateNameInfo(data, offset, cb) { // return error; // } -function scopedGenerator() { +function scopedGenerator(...scopeArgs) { const separator = '::'; - const args = [...arguments].filter((arg) => !!arg); - const preface = args.length > 0 ? args.join(separator) + separator : ''; - return () => preface + [...arguments].join(separator); + const scopes = scopeArgs.filter((arg) => !!arg); + const preface = scopes.length > 0 ? scopes.join(separator) + separator : ''; + return (...parts) => preface + parts.join(separator); } async function getSymbolInstanceID(layer, scope, tag) { @@ -886,7 +891,7 @@ export default class Logix5000 extends CIPLayer { } for (let i = 0; i < sizeOfMasks; i++) { - if (ORmasks[i] < 0 || ORmasks > 0xFF || ANDmasks[i] < 0 || ANDmasks > 0xFF) { + if (ORmasks[i] < 0 || ORmasks[i] > 0xFF || ANDmasks[i] < 0 || ANDmasks[i] > 0xFF) { resolver.reject('Values in masks must be greater than or equal to zero and less than or equal to 255'); return; } diff --git a/src/layers/cip/layers/internal/CIPConnectionLayer.js b/src/layers/cip/layers/internal/CIPConnectionLayer.js index 73e1e73..628be1d 100644 --- a/src/layers/cip/layers/internal/CIPConnectionLayer.js +++ b/src/layers/cip/layers/internal/CIPConnectionLayer.js @@ -488,6 +488,7 @@ class CIPConnectionLayer extends Layer { } handleDestroy() { + stopResend(this); this._connectionState = 0; this._sequenceCount = 0; this.sendInfo = null; diff --git a/src/layers/cip/layers/internal/CIPInternalLayer.js b/src/layers/cip/layers/internal/CIPInternalLayer.js index 0f7dbe1..b86c520 100644 --- a/src/layers/cip/layers/internal/CIPInternalLayer.js +++ b/src/layers/cip/layers/internal/CIPInternalLayer.js @@ -27,10 +27,8 @@ class CIPInternalLayer extends Layer { } } - sendRequest(connected, request, callback) { + sendRequest(connected, request, callback, timeout) { return CallbackPromise(callback, (resolver) => { - const timeout = null; - const context = this.contextCallback((error, message) => { try { if (error) { From 06967c01f770cc63ea313b067a42a9bc095f70f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 21:43:50 +0000 Subject: [PATCH 07/10] Add EIP encapsulation and layer tests Covers RegisterSession golden frames, EIP header status round-trips, ListServices CPF item decoding, session registration through the scripted transport, and listIdentity for single and multiple hosts. Three tests fail against the current code, documenting known bugs fixed in the next commit: - EIPPacket.Encode reads .code off the numeric status argument, so any nonzero status is silently encoded as 0 - the ListServices name scan runs one byte past the 16-byte field - listIdentity with multiple hosts drops its reply callback after the first response and only ever returns one device https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- test/unit/eip.layer.test.js | 147 ++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 test/unit/eip.layer.test.js diff --git a/test/unit/eip.layer.test.js b/test/unit/eip.layer.test.js new file mode 100644 index 0000000..0d6861d --- /dev/null +++ b/test/unit/eip.layer.test.js @@ -0,0 +1,147 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import EIPLayer from '../../src/layers/cip/layers/EIP/index.js'; +import EIPPacket from '../../src/layers/cip/layers/EIP/packet.js'; +import CPF from '../../src/layers/cip/layers/EIP/cpf.js'; +import ScriptedTransport from '../harness/ScriptedTransport.js'; + +/** + * EIP encapsulation tests (CIP Vol 2): 24-byte header + * (command, length, session, status, sender context, options) followed + * by command-specific data, all little-endian except sockaddr fields. + */ + +function hex(s) { + return Buffer.from(s.replace(/\s+/g, ''), 'hex'); +} + +function withTimeout(promise, label, ms = 3000) { + return new Promise((resolve, reject) => { + const handle = setTimeout(() => { + reject(new Error(`${label} did not settle within ${ms}ms`)); + }, ms); + promise.then( + (value) => { clearTimeout(handle); resolve(value); }, + (err) => { clearTimeout(handle); reject(err); }, + ); + }); +} + +/** ListIdentity CPF item: version, sockaddr (big-endian family/port/address), + * identity attributes 1-7, state */ +function listIdentityResponse(addressByte) { + const item = hex( + '0100' /** encapsulation protocol version 1 */ + + '0002 af12' /** sin_family AF_INET, sin_port 44818 (big-endian) */ + + `c0a801${addressByte.toString(16).padStart(2, '0')}` /** sin_addr 192.168.1.x */ + + '0000000000000000' /** sin_zero */ + + '0100' /** vendor 1 (Rockwell) */ + + '0c00' /** device type 0x0C (communications adapter) */ + + '9900' /** product code */ + + '0101' /** revision 1.1 */ + + '0000' /** status */ + + '78563412' /** serial number */ + + '0158' /** product name: short string 'X' */ + + '03', /** state: operational */ + ); + const cpf = Buffer.concat([ + hex('0100 0c00'), + Buffer.from([item.length, 0]), + item, + ]); + return Buffer.concat([ + hex('6300'), + Buffer.from([cpf.length, 0]), + hex('00000000 00000000 0000000000000000 00000000'), + cpf, + ]); +} + +describe('EIP packet codec', () => { + it('encodes a RegisterSession request (CIP Vol 2, 2-4.4)', () => { + assert.deepEqual( + EIPPacket.RegisterSessionRequest(Buffer.alloc(8)), + hex('6500 0400 00000000 00000000 0000000000000000 00000000 01000000'), + ); + }); + + it('preserves a nonzero status through fromBuffer/toBuffer', () => { + const original = hex('6500 0000 00000000 69000000 0102030405060708 00000000'); + const packet = EIPPacket.fromBuffer(original, { current: 0 }); + assert.equal(packet.status.code, 0x69); + assert.deepEqual(packet.toBuffer(), original); + }); +}); + +describe('EIP CPF decoding', () => { + it('caps ListServices names at the 16-byte field', () => { + const name = 'A'.repeat(16); + const item = Buffer.concat([ + hex('0100 2001'), /** version 1, flags 0x0120 */ + Buffer.from(name, 'ascii'), + ]); + const buffer = Buffer.concat([ + hex('0100 0001'), /** one item, type 0x0100 ListServices */ + Buffer.from([item.length, 0]), + item, + Buffer.from([0x42]), /** stray trailing byte must not leak into the name */ + ]); + const items = CPF.Packet.Decode(buffer, { current: 0 }); + assert.equal(items.length, 1); + assert.equal(items[0].value.name, name); + assert.equal(items[0].value.flags.supportsCIPPacketEncapsulationViaTCP, true); + assert.equal(items[0].value.flags.supportsCIPClass0or1UDPBasedConnections, true); + }); +}); + +describe('EIP layer', () => { + it('registers a session and stores the assigned handle', async () => { + const transport = new ScriptedTransport(); + const layer = new EIPLayer(transport); + transport.reply(hex('6500 0400 44332211 00000000 0000000000000000 00000000 01000000')); + + await withTimeout(new Promise((resolve) => layer.connect(resolve)), 'connect'); + + assert.deepEqual(transport.sent, [ + hex('6500 0400 00000000 00000000 0000000000000000 00000000 01000000'), + ]); + assert.equal(layer._sessionHandle, 0x11223344); + await transport.close(); + }); + + it('listIdentity resolves a single device', async () => { + const transport = new ScriptedTransport(); + const layer = new EIPLayer(transport); + transport.reply(listIdentityResponse(10)); + + const identity = await withTimeout(layer.listIdentity(), 'listIdentity'); + + assert.deepEqual(transport.sent, [ + hex('6300 0000 00000000 00000000 0000000000000000 00000000'), + ]); + assert.equal(identity.value.socket.address, '192.168.1.10'); + assert.equal(identity.value.socket.port, 44818); + await transport.close(); + }); + + it('listIdentity with multiple hosts returns every device', async () => { + const transport = new ScriptedTransport(); + const layer = new EIPLayer(transport); + transport.reply(listIdentityResponse(10)); + transport.reply(listIdentityResponse(20)); + + const identities = await withTimeout( + layer.listIdentity({ hosts: ['192.168.1.10:44818', '192.168.1.20:44818'] }), + 'listIdentity multi-host', + 5000, + ); + + assert.equal(transport.sent.length, 2); + assert.deepEqual( + identities.map((identity) => identity.value.socket.address), + ['192.168.1.10', '192.168.1.20'], + ); + await transport.close(); + }); +}); From c90520ba9f0aca85203c6642cee8aca2d1b97e68 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 21:44:31 +0000 Subject: [PATCH 08/10] Fix EIP status encoding, name scan, and multi-host listIdentity - EIPPacket.Encode read .code off its numeric status argument, so any nonzero status was silently encoded as 0 (every caller passes a number; toBuffer passes this.status.code) - the ListServices name scan checked one byte past the 16-byte name field, letting a stray 17th byte leak into the decoded name - listIdentity's reply handler now returns true for host-specified scans so it stays registered after the first reply; previously only the first device discovered was ever returned https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- src/layers/cip/layers/EIP/cpf.js | 2 +- src/layers/cip/layers/EIP/index.js | 4 ++-- src/layers/cip/layers/EIP/packet.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/layers/cip/layers/EIP/cpf.js b/src/layers/cip/layers/EIP/cpf.js index 44ba386..548f472 100644 --- a/src/layers/cip/layers/EIP/cpf.js +++ b/src/layers/cip/layers/EIP/cpf.js @@ -126,7 +126,7 @@ class Packet { value.flags.supportsCIPClass0or1UDPBasedConnections = !!getBits(flags, 8, 9); let nameLength; - for (nameLength = 0; nameLength <= 16; nameLength++) { + for (nameLength = 0; nameLength < 16; nameLength++) { if (buffer[offsetRef.current + nameLength] === 0) { break; } diff --git a/src/layers/cip/layers/EIP/index.js b/src/layers/cip/layers/EIP/index.js index 71588da..6b14e11 100644 --- a/src/layers/cip/layers/EIP/index.js +++ b/src/layers/cip/layers/EIP/index.js @@ -307,8 +307,8 @@ export default class EIPLayer extends Layer { clearTimeout(timeoutHandler); if (hostsSpecified) { timeoutHandler = setTimeout(finalizer, resetTimeout); - return; - // return true; + /** keep this callback registered for replies from the remaining hosts */ + return true; } finalizer(); } else { diff --git a/src/layers/cip/layers/EIP/packet.js b/src/layers/cip/layers/EIP/packet.js index 7f5aa0c..94108de 100644 --- a/src/layers/cip/layers/EIP/packet.js +++ b/src/layers/cip/layers/EIP/packet.js @@ -158,7 +158,7 @@ export default class EIPPacket { buffer.writeUInt16LE(command, OFFSET_COMMAND); buffer.writeUInt16LE(dataLength, OFFSET_DATA_LENGTH); buffer.writeUInt32LE(sessionHandle, OFFSET_SESSION_HANDLE); - buffer.writeUInt32LE(status.code, OFFSET_STATUS); + buffer.writeUInt32LE(status, OFFSET_STATUS); (senderContext || NullSenderContext).copy(buffer, OFFSET_SENDER_CONTEXT, 0, 8); buffer.writeUInt32LE(options, OFFSET_OPTIONS); if (dataLength > 0) { From cfb9d2490f5d870859481c6d24b1852d09532f3e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 23:18:29 +0000 Subject: [PATCH 09/10] Add TCP lifecycle tests and a connected-messaging integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration test exercises ForwardOpen, sequenced connected GetAttributeSingle requests, and ForwardClose against OpENer — the same machinery the Logix5000 unit tests emulate with LogixResponder, now validated against a real third-party CIP stack. Running it exposed a TCP layer bug, captured by the new unit test (against a real net.Server): a deferred sendNextMessage wakeup after close() reopens the connection, because the disconnected branch reconnects unconditionally. This leaks a fresh socket after every close and keeps the process alive. Fixed in the next commit. https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- test/integration/cip-connected.js | 48 +++++++++++++++++++++++++ test/unit/tcp.layer.test.js | 60 +++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 test/integration/cip-connected.js create mode 100644 test/unit/tcp.layer.test.js diff --git a/test/integration/cip-connected.js b/test/integration/cip-connected.js new file mode 100644 index 0000000..5c184e0 --- /dev/null +++ b/test/integration/cip-connected.js @@ -0,0 +1,48 @@ +import assert from 'assert'; +import { TCP, CIP } from '../../src/index.js'; + +/** + * Exercises connected class-3 explicit messaging against a real + * third-party CIP stack (OpENer): ForwardOpen through the + * CIPConnectionLayer, a connected GetAttributeSingle on the Identity + * object, and ForwardClose on shutdown. + * + * This validates the same handshake and sequenced-message framing the + * Logix5000 unit tests emulate with the LogixResponder harness. + */ + +const EPath = CIP.Core.EPath.default; +const CIPRequest = CIP.Core.Request.default; + +const tcpLayer = new TCP({ host: '127.0.0.1', port: 44818 }); +const cipLayer = new CIP(tcpLayer); + +(async () => { + let error; + try { + /** GetAttributeSingle: Identity object (0x01), instance 1, attribute 1 (Vendor ID) */ + const path = EPath.Encode(true, [ + new EPath.Segments.Logical.ClassID(0x01), + new EPath.Segments.Logical.InstanceID(0x01), + new EPath.Segments.Logical.AttributeID(0x01), + ]); + + const reply = await cipLayer.sendRequest(true, new CIPRequest(0x0E, path), null, 5000); + assert.strictEqual(reply.status.code, 0, 'GetAttributeSingle status'); + assert.strictEqual(reply.data.readUInt16LE(0), 1, 'Vendor ID (OpENer reports 1)'); + + /** a second request must reuse the connection with the next sequence count */ + const reply2 = await cipLayer.sendRequest(true, new CIPRequest(0x0E, path), null, 5000); + assert.strictEqual(reply2.status.code, 0, 'second connected request status'); + } catch (err) { + error = err; + } finally { + await tcpLayer.close(); + } + + if (error) { + throw error; + } else { + console.log('cip connected messaging success'); // eslint-disable-line no-console + } +})(); diff --git a/test/unit/tcp.layer.test.js b/test/unit/tcp.layer.test.js new file mode 100644 index 0000000..a33516d --- /dev/null +++ b/test/unit/tcp.layer.test.js @@ -0,0 +1,60 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import net from 'node:net'; + +import TCPLayer from '../../src/layers/tcp/index.js'; + +function listen() { + return new Promise((resolve) => { + const connections = []; + const server = net.createServer((socket) => { + connections.push(socket); + socket.on('data', () => {}); + socket.on('error', () => {}); + }); + server.listen(0, '127.0.0.1', () => resolve({ server, connections })); + }); +} + +function sleep(ms) { + return new Promise((resolve) => { setTimeout(resolve, ms); }); +} + +describe('TCP layer', () => { + it('connects on demand and writes queued messages', async () => { + const { server, connections } = await listen(); + const layer = new TCPLayer({ host: '127.0.0.1', port: server.address().port }); + + const received = new Promise((resolve) => { + server.on('connection', (socket) => socket.once('data', resolve)); + }); + + layer.send(Buffer.from([1, 2, 3]), null, false); + assert.deepEqual(await received, Buffer.from([1, 2, 3])); + assert.equal(connections.length, 1); + + await layer.close(); + server.close(); + }); + + it('does not reconnect after close', async () => { + const { server, connections } = await listen(); + const layer = new TCPLayer({ host: '127.0.0.1', port: server.address().port }); + + layer.send(Buffer.from([1, 2, 3]), null, false); + await sleep(100); + assert.equal(connections.length, 1); + + await layer.close(); + + /** a deferred wakeup with an empty queue (e.g. from a write callback) + * must not reopen the connection */ + layer.sendNextMessage(); + await sleep(200); + + assert.equal(connections.length, 1, 'closed layer must not reconnect'); + assert.ok(layer.socket.destroyed, 'socket should remain destroyed after close'); + + server.close(); + }); +}); From 138b8aaf2a26be3fc5ebbba00d49acc90bcfd2aa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 23:19:18 +0000 Subject: [PATCH 10/10] Fix TCP reconnect after close and run connected messaging in CI TCPLayer.sendNextMessage reconnected unconditionally whenever the state was disconnected, so any deferred wakeup arriving after close() (e.g. from the final write's callback) silently opened a fresh socket that nothing would ever close. It now reconnects only when the request queue is non-empty. Adds the OpENer connected-messaging integration test to CI. https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby --- .github/workflows/ci.yml | 3 +++ src/layers/tcp/index.js | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 34a8fbb..1e321ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,9 @@ jobs: - working-directory: node-drivers run: node test/integration/eip.js + - working-directory: node-drivers + run: node test/integration/cip-connected.js + - uses: actions/setup-python@v5 with: python-version: '3.11' diff --git a/src/layers/tcp/index.js b/src/layers/tcp/index.js index f53dae3..b4f1cf2 100644 --- a/src/layers/tcp/index.js +++ b/src/layers/tcp/index.js @@ -240,8 +240,11 @@ export default class TCPLayer extends Layer { }); } } else if (this._connectionState === 0) { - /** Reconnect */ - connect(this); + /** Reconnect only when there is something to send, otherwise a + * deferred wakeup after close() would reopen the connection */ + if (this.hasRequest()) { + connect(this); + } } }