Repository navigation
fix(connection): only retry idempotent queries after mid-send transport loss - #131
Merged
Merged
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Oct 19, 2026
fedem95
approved these changes
Oct 19, 2026
fedem95
left a comment
Contributor
There was a problem hiding this comment.
Looks good. The retry guard now distinguishes idempotent queries from state-changing frames, and the tests cover both paths. CI is passing. Approved.
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
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.
What is the problem
In
OWNCommandSession._locked_send()(OWNd/connection/command_session.py), when a socket closed mid-send (ConnectionResetError,asyncio.IncompleteReadError,asyncio.LimitOverrunError, orOSError) before the gateway's final*#*1##(ACK) /*#*0##(NACK) was read, the transport-loss retry branch checked onlyif is_status_request:.Because callers such as
MyHOME'sLightingResyncManageror declarative expose engine may passis_status_request=Trueto suppressNACKretries on fire-and-forget writes, checkingis_status_requestalone could replay a non-idempotent action command (*1*1*WHERE##,*2*1*WHERE##), a dimension write (*#WHO*WHERE*#DIM*VAL##), or a dimension response injection (*#WHO*WHERE*DIM*VAL##) that the gateway had already forwarded onto the physical SCS bus before the TCP socket dropped.How is it solved
_is_idempotent_query_frame(message)inOWNd/connection/command_session.py, which parses the outgoing frame viaOWNMessage.parse(raw)and returnsTruestrictly whenparsed._mode in (OWNMessageMode.STATUS_REQUEST, OWNMessageMode.DIMENSION_REQUEST)(*#WHO*WHERE##or*#WHO*WHERE*DIM##).OWNCommandSession._locked_send()to requireif is_status_request and _is_idempotent_query_frame(message):before reconnecting and re-transmitting the frame.COMMAND,DIMENSION_WRITE,DIMENSION_RESPONSE) never replay after mid-send transport loss once bytes have been written to the socket.How is it tested
tests/test_connection.py:ConnectionResetError/IncompleteReadErrorretries idempotent status/dimension queries (*#1*0##,*#4*1*0##) whenis_status_request=True, and never retries action commands (*1*1*21##), dimension writes (*#4*1*#14*0215*1##), or dimension response injections (*#4*1*0*0215##) even whenis_status_request=True.OWNdtest suite passing with 100.0% coverage onOWNd/connection/command_session.py,mypy OWNd(0 issues), andruff checkclean.State Diagram
stateDiagram-v2 [*] --> WriteFrame : _locked_send(message, is_status_request) state "Write Frame to TCP/Serial Stream" as WriteFrame state "Await Gateway Reply (*#*1## / *#*0##)" as AwaitReply state "Mid-Send Transport Loss\n(ConnectionResetError / IncompleteReadError / OSError)" as TransportLoss state "Check Guard:\nis_status_request AND _is_idempotent_query_frame(message)" as RetryGuard state "Reconnect Once & Replay Query\n(*#WHO*WHERE## or *#WHO*WHERE*DIM##)" as ReplayQuery state "Close Socket & Return None / Raise OWNConnectionError\n(Never Replay Action / Write / Expose Frame)" as AbortNoReplay WriteFrame --> AwaitReply : writer.drain() AwaitReply --> [*] : ACK (*#*1##) / Collected Replies AwaitReply --> TransportLoss : Socket drops before ACK/NACK TransportLoss --> RetryGuard RetryGuard --> ReplayQuery : True (STATUS_REQUEST or DIMENSION_REQUEST) RetryGuard --> AbortNoReplay : False (COMMAND, DIMENSION_WRITE, or DIMENSION_RESPONSE) ReplayQuery --> [*] AbortNoReplay --> [*]Where are the traces
tests/golden/corpus.json,tests/golden/firmware_oracle.json):tests/golden/corpus.json(verifiesOWNMessageModeclassification acrossCOMMAND,STATUS_REQUEST,DIMENSION_REQUEST,DIMENSION_WRITE, andDIMENSION_RESPONSEframes).tests/golden/firmware_oracle.json(6,500 empirical gateway verdicts across 12 emulated firmwares).2.0.11) Captures (2026-10-18):MyHOME/tests/fixtures/traces/f461_live_2026-10-18/myhome_trace_F461_live_2026-10-18.json(liveF461command and status request traffic across WHO=1, WHO=2, and WHO=4).What does the Oracle / Encyclopedia say
own-firmware-oracle/tests/golden/firmware_oracle.json):bt_luci,bt_fls,bt_termo,coso), an incoming command or dimension write is transmitted onto the 2-wire SCS bus ($03.../$003...) before the gateway writes the final*#*1##(ACK) or*#*0##(NACK) back to the TCP command socket.F460/F461/MyHomeServer1), the SCS bus frame has already been emitted; replaying a non-query frame can double-toggle an impulse relay (*1*1*WHERE##), double-step a shutter, or duplicate a bus write.protocol/frame-grammar.md):*#WHO*WHERE##status request and*#WHO*WHERE*DIM##dimension request) from state-mutating frames (*WHO*WHAT*WHERE##command,*#WHO*WHERE*#DIM*VAL##dimension write, and*#WHO*WHERE*DIM*VAL##dimension response/expose). Only the two read-only query forms are idempotent on the SCS bus.