Add scripted-transport harness and Modbus golden-frame tests#18
Merged
Conversation
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
- 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
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
- 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
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
- 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
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
- 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
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
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
count/bit packing) and mutates the caller's array
register count
contains several
https://claude.ai/code/session_01AuAYuP6dTn8ALpkKErQuby