Fix partner S7 Communication Setup and bsend/brecv PDU format#669
Open
gijzelaerr wants to merge 9 commits intomasterfrom
Open
Fix partner S7 Communication Setup and bsend/brecv PDU format#669gijzelaerr wants to merge 9 commits intomasterfrom
gijzelaerr wants to merge 9 commits intomasterfrom
Conversation
The partner module was only completing the COTP handshake but skipping the S7 Communication Setup negotiation, causing real PLCs to stay in "awaiting connection" status. Additionally, the bsend/brecv PDUs used a minimal custom format instead of proper S7 USERDATA PDUs with R-ID, making them incompatible with real Siemens PLCs. Changes: - Add S7 Communication Setup after COTP connect (active mode) - Handle incoming COTP CR and S7 Setup for passive mode - Rewrite partner data PDU to use S7 USERDATA format (type 0x07) with push function group (0x06), proper parameter section, and R-ID - Rewrite partner ACK PDU to use S7 USERDATA response format - Add r_id attribute for bsend/brecv matching - Update tests for new PDU format, add R-ID coverage Fixes #668 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Re-exports snap7.Partner and PartnerStatus from s7, so users can do `from s7 import Partner` just like Client, AsyncClient, and Server. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The bsend data push was using subfunction 0x01 (push notification) instead of 0x06 (BSend), causing the PLC to reject the PDU with error 0x8404. Also fix the parameter sub-length from 0x08 to 0x04 for requests (no data_unit_ref/last_data_unit/error_code needed), and add error code checking in the ACK parser. Fixes #668 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The BSend USERDATA PDU was rejected by real PLCs (error 0x8104 "object does not exist") because the parameter section used request semantics instead of the PBC push format. Changes: - Method byte: 0x12 (push) instead of 0x11 (request) - Type/Group byte: 0x06 (push|PBC) instead of 0x46 (request|PBC) - Sub-length: 0x08 to include dur/ldu/error_code fields - Add variable specification block (12 06 82 41 ...) with payload length - Add PBC prefix (12 00) before payload in data section - Parse and strip PBC prefix in _parse_partner_data_pdu for incoming data Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Based on feedback from real PLC testing (issue #668), the PDU format needed further corrections: - Type/Group byte: 0x46 (request|PBC) was correct after all - Subfunction: 0x01 (not 0x06) - Sequence number in parameter: always 0 for PBC - R-ID moved from parameter to data section variable specification - Data section format: var_spec (12 06 13 00) + R-ID + payload_len + data - Parser updated to strip 10-byte PBC var spec prefix from incoming data This format was confirmed working against a real S7-1500 PLC by the issue reporter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add the missing async receive functionality so partners can receive data non-blocking, matching the existing async send pattern: - as_b_recv(): start async receive in background listener thread - wait_as_b_recv_completion(timeout): block until data arrives - check_as_b_recv_completion(): poll for completion (enhanced with error state reporting) - _recv_listener(): background thread that monitors the socket for incoming data, parses the PDU, sends ACK, and queues the result Also implemented: - set_recv_callback(callback): register callback for incoming data (was a no-op stub) - set_send_callback(callback): register callback for send completion (was a no-op stub) - Thread-safe I/O via _io_lock to coordinate between async send and receive threads on the shared socket Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The recv listener thread could complete before wait_as_b_recv_completion was called, causing a spurious RuntimeError. Track whether an async recv was started so the wait method returns the result instead of raising when the listener already finished. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Based on real PLC testing feedback (issue #668): - ACK must echo the PDU reference from the incoming data PDU, not use a new sequence number — the PLC needs this to correlate the response - Remove R-ID from ACK parameter section (not needed) - Add 4-byte data section to ACK (return code 0x0a, transport 0x00) - _parse_partner_data_pdu now returns (payload, r_id, pdu_ref) tuple, extracting R-ID and PDU reference from the variable specification block and S7 header respectively Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
Fixes #668 — the partner module was connecting at the COTP transport layer but skipping the S7 Communication Setup negotiation, causing real PLCs to stay in "awaiting connection" status. Additionally, the bsend/brecv PDUs used a minimal custom format without R-ID support, making them incompatible with real Siemens PLCs.
r_idattribute (default 0) that is included in both data and ACK PDUs, matching the R-ID used in PLC bsend/brecv blocksTest plan
🤖 Generated with Claude Code