Repository navigation
Continue C# parity: TCP, live discovery, and vendor CPA interoperability - #14
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eee6c16e76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (received > 0) { | ||
| bytes.resize(static_cast<std::size_t>(received)); | ||
| return bytes; |
There was a problem hiding this comment.
When one TCP read contains multiple TPKT frames—which TCP explicitly permits—this returns the whole coalesced chunk. MmsAssociationRuntime::receive_application_payload() stops after the first complete application payload and leaves remaining frames in tpkt_decoder_, but its next iteration or call invokes transport_.receive() before trying to pop those buffered frames. A response coalesced behind an information report therefore remains unread while the runtime waits for new socket data and eventually times out; drain the decoder before blocking on the socket.
Useful? React with 👍 / 👎.
| const bool has_presentation_accept = | ||
| contains_octet(presentation, 0x31U) || | ||
| contains_octet(presentation, 0xA0U); | ||
| const bool has_acse_aare = contains_octet(presentation, 0x61U); | ||
| const bool has_user_information = contains_octet(presentation, 0xBEU); | ||
| const bool has_mms_initiate_response = contains_octet(presentation, 0xA9U); | ||
|
|
||
| if (!has_presentation_accept || !has_acse_aare) { |
There was a problem hiding this comment.
When strict CPA/AARE decoding fails because of a vendor-specific or malformed field, this fallback treats the mere presence of byte values 0x31/0xA0 and 0x61 anywhere in Session user data as acceptance. A rejected AARE still contains the 0x61 AARE tag and normally sits inside those Presentation markers, so such a response can incorrectly transition the runtime to associated; the compatibility path must minimally parse and verify the AARE result instead of scanning for octets.
Useful? React with 👍 / 👎.
| const int resolve_result = getaddrinfo( | ||
| endpoint.host.c_str(), port.c_str(), &hints, &raw_addresses); |
There was a problem hiding this comment.
For hostname endpoints under slow or unavailable DNS, getaddrinfo() is a synchronous blocking call and neither the supplied deadline nor the stop token is observed until it returns. Consequently the advertised connect timeout and cancellation behavior can be exceeded indefinitely before the nonblocking socket logic begins; use cancellable/asynchronous resolution or otherwise include resolution in the bounded operation.
Useful? React with 👍 / 👎.
| if (read_ready) { | ||
| FD_SET(socket, &read_set); | ||
| } | ||
| if (write_ready) { | ||
| FD_SET(socket, &write_set); |
There was a problem hiding this comment.
On POSIX processes whose file-descriptor limit is raised above FD_SETSIZE, socket() can return a descriptor of 1024 or greater; passing it to FD_SET writes outside the fixed-size fd_set, causing undefined behavior or memory corruption. This library can therefore fail in services with many open descriptors even though the socket itself was created successfully; use poll/ppoll or reject descriptors outside the supported range before calling FD_SET.
Useful? React with 👍 / 👎.
1 participant
Scope
This draft PR now delivers Phase 4C and Phase 4C.1:
live-ied-model-v1hierarchy and deterministic fingerprint;TCP and live discovery
std::stop_tokencancellation.Phase 4C.1 live-model parity
LN$FC$DO$DA...into Logical Device / Logical Node / Data Object / Data Attribute hierarchy.ariec61850_live_discover --model-json.--manifest.scripts/compare-live-model-parity.py.Current interoperability hardening
0x83) before the context-result list.--ied-nameremains an optional model identity override only.Physical read-only interoperability
scripts/run-live-readonly-interop.pyreconnects and performs a fresh discovery per cycle.interop-summary.jsonare retained.Read-only safety boundary
The discovery path constructs only GetNameList, GetVariableAccessAttributes, GetNamedVariableListAttributes, and Read. It does not construct MMS Write, control, GI, RCB reservation/enable, dynamic DataSet mutation, or file-service requests. The scripted regression decodes every emitted discovery request and rejects service tag 5.
Validation completed locally
py_compilepassed.Acceptance still pending
No physical IED result is claimed. Before merge:
The PR remains draft until those external acceptance gates are complete.