Skip to content

refactor(connection): remove _resolve_discovery_fn test shim and document MH201 retry semantics - #133

Merged
GreenGrassBlueOcean merged 2 commits into
masterfrom
fix/pr132-review-followup
Oct 20, 2026
Merged

GreenGrassBlueOcean merged 2 commits into
masterfrom
fix/pr132-review-followup

Conversation

@GreenGrassBlueOcean

@GreenGrassBlueOcean GreenGrassBlueOcean commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #132 (review) by @fedem95.


1. What is the problem?

  1. Runtime _resolve_discovery_fn test-shimming helper in OWNd/connection/gateway.py (Review Note 3):
    • _resolve_discovery_fn, _ORIG_FIND_GATEWAYS, _ORIG_GET_GATEWAY, _ORIG_GET_PORT, and import sys existed in OWNd/connection/gateway.py solely so legacy tests could patch OWNd.connection.{find_gateways,get_gateway,get_port} instead of OWNd.connection.gateway.*.
  2. MH201 exhausted session-slot behavior vs fast send() reconnect documentation (Review Note 1):
    • In feat(connection): add typed exceptions, fast send reconnect, and API hardening #132, OWNCommandSession._locked_send() switched its internal reconnect when streams are None to a single attempt (_connect_max_attempts_override = 1) with a SEND_RECONNECT_COOLDOWN = 2.0s window so queued commands behind _send_lock do not stall for ~13.3 s of exponential backoff during a gateway outage.
    • On single-slot gateways like BTicino MH201 (which accepts TCP and closes the socket during *99*0## negotiation when its session slot is occupied), the interaction between OWNd's single-attempt mid-flight reconnect in send() and MyHOME's CommandWorkerPool.sending_loop() (which closes idle sockets after 15.0s and calls await session.connect() directly with the full 5-attempt / ~13.3 s retry window when reopening from idle) was not documented in OWNd/connection/command_session.py.
  3. Duplicate tests & remaining Codecov branch partials in tests/test_connection.py:

2. How is it solved?

  1. Remove _resolve_discovery_fn from OWNd/connection/gateway.py:
    • Calls find_gateways(), get_gateway(), and get_port() directly in OWNGateway.get_first_available_gateway(), OWNGateway.find_from_address(), and OWNGateway.build_from_discovery_info(), removing _resolve_discovery_fn, _ORIG_*, and import sys from the runtime path (paired with OpenWebNet-HA/MyHOME#758 updating MyHOME's test patches to OWNGateway.__module__).
  2. Document MH201 session-slot retry & MyHOME worker cross-reference in OWNd/connection/command_session.py:
    • Adds an explicit inline cross-reference in OWNCommandSession._locked_send() (matching the companion comment in MyHOME's custom_components/myhome/gateway_sessions.py in OpenWebNet-HA/MyHOME#758) explaining why callers that recycle idle sockets (sending_loop()) call await session.connect() directly before send() (preserving the full MAX_CONNECT_ATTEMPTS = 5 / ~13.3 s retry window when an MH201 closes TCP during negotiation), whereas _locked_send()'s single-attempt reconnect only runs on direct send() calls with closed streams or on attempt 2 after a mid-flight transport drop.
  3. Deduplicate tests/test_connection.py & cover all 4 Codecov branch partials:
    • Removes the duplicate test functions and legacy patch("OWNd.connection.find_gateways", ...) test block in tests/test_connection.py, and adds assertions covering _notify_auth_error without a logger, test_connection(raise_on_error=False) on auth error, connect(raise_on_error=True) on fatal non-auth negotiation error, and event_session.get_next(raise_on_auth_error=True) on non-auth reconnect failure.

3. State Diagram

stateDiagram-v2
    direction TB
    [*] --> IdleClosed: Session created / closed after 15.0s idle

    state "Closed (is_open == False)" as IdleClosed
    state "Direct connect() (MAX_CONNECT_ATTEMPTS = 5, ~13.3s backoff)" as DirectConnect
    state "Open (is_open == True)" as SessionOpen
    state "_locked_send() Attempt 1" as SendAttempt1
    state "Internal Reconnect (_connect_max_attempts_override = 1)" as FastReconnect
    state "Cooldown Active (SEND_RECONNECT_COOLDOWN = 2.0s)" as Cooldown

    IdleClosed --> DirectConnect: MyHOME sending_loop() sees not is_open\ncalls await session.connect()
    DirectConnect --> SessionOpen: TCP + *99*0## + Auth -> *#*1## ACK\n(retries up to 5x if MH201 slot busy)
    DirectConnect --> IdleClosed: Exhausted 5 attempts -> returns failure

    IdleClosed --> FastReconnect: Caller invokes send() directly while closed
    SessionOpen --> SendAttempt1: await session.send(frame)
    SendAttempt1 --> SessionOpen: *#*1## ACK -> returns collected / True
    SendAttempt1 --> IdleClosed: *#*0## NACK or written non-query EOF\n(close streams, no replay, return None)
    SendAttempt1 --> FastReconnect: Pre-write EOF or idempotent query EOF\n(close streams -> attempt 2)

    FastReconnect --> SendAttempt1: 1-attempt connect() succeeds
    FastReconnect --> Cooldown: 1-attempt connect() fails\n(_last_reconnect_failure = monotonic())
    Cooldown --> Cooldown: Subsequent send() within 2.0s\nfails fast without TCP connect
    Cooldown --> FastReconnect: Cooldown elapsed (>= 2.0s)
Loading

4. Where are the traces (real ones)?

Hardware Gateway Firmware / Profile Real Trace / Log File Observed Session & Command Behavior
BTicino MH201 Single command session slot (max_workers=1, 15.0s idle close) MyHOME/tests/fixtures/traces/mh201-fresh-discovery-20260930.log Lines 4–10: worker 0 dequeues *#2*0## when session is closed, runs explicit Opening command session $\rightarrow$ Negotiating command session $\rightarrow$ Received nonce $\rightarrow$ Command session established successfully. Lines 331–332: Command session idle for 15.0s; closing socket to release gateway resource.
Legrand F455 WHO=1013 code 8 (20000/tcp) MyHOME/tests/fixtures/traces/issue_674/home-assistant_2026-10-09T17-34-15.609Z.log Captures 16 consecutive command timeouts (161.3s worker lockup) demonstrating why _locked_send() must avoid compounding multi-attempt reconnect backoffs while holding _send_lock.
BTicino F461 Firmware 2.0.11 (WHO=13 Dim 15 code 200, 134.15.5.0) MyHOME/tests/fixtures/traces/issue_667/myhome_trace_F461_impulse_intercom_2026-10-19.json Live dual-session (cmd + evt) capture showing normal ~90ms command ACKs and 2.07s bus timeout NACKs (*#*0##) on an open command session.

5. What does the Firmware Oracle / Encyclopedia say?

  1. OpenWebNet Encyclopedia (protocol/sessions.md — Connection and Sessions):
    • Specifies the 3-phase TCP 20000 connection state machine:
      • Awaiting server greeting: Server sends initial *#*1## (ACK) upon TCP accept.
      • Awaiting session result: Client sends session selector (*99*9## / *99*0## for command/actions, *99*1## for event stream); server responds with open-range *#*1##, legacy challenge (*#<nonce>##), HMAC declaration (*98*1## / *98*2##), *#*0## (NACK), or connection close (EOF / IncompleteReadError when session slots are exhausted).
      • State-machine requirement: TCP connection establishment (open_connection) must not be equated with an active session until greeting, selector acceptance, and authentication succeed (session.is_connected vs session.is_open).
  2. Gateway Session Slot Limits (MyHOME/docs/configuration/gateways.md):
    • Single-slot gateways (MH200, MH200N, MH201, F452, F453AV, H4890) allow only 1 concurrent command worker, so releasing the socket after 15.0s of idle and using full 5-attempt connect() on session reopen accommodates temporary slot contention without stalling in-flight send() calls.
  3. Empirical Firmware Oracle (openwebnet-mcp — lookup_firmware_verdict):
    • Across all 12 QEMU-emulated gateway firmwares (F450 020010, F453 010100, F453AV 030014, F454 020051, F459 020105, F460 020012, F461 020011, H4684 020054, MH200 020102, MH200N 010108, MH202 010024, MyHomeServer1 028206), command frames and gateway model queries (*#13**15##) are processed only after the *99*0## / *99*9## negotiation handshake completes with *#*1##.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@GreenGrassBlueOcean GreenGrassBlueOcean changed the title refactor(connection): remove _resolve_discovery_fn test shim from OWNGateway refactor(connection): remove _resolve_discovery_fn test shim and document MH201 retry semantics Oct 20, 2026

@fedem95 fedem95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up. Tested 99b7b85: 1780 passed locally, CI green including the MyHOME suite. OWNd/connection/gateway.py is back to exactly its pre-#132 content (same blob, 996c684), no test still patches OWNd.connection.find_gateways/get_gateway/get_port, and the two duplicated tests now exist once each.

I also checked the answer to my question in MyHOME (gateway_sessions.py on v2, lines 1242-1290): the worker closes the command session after 15 s idle, calls connect() itself (the full 5-attempt loop) when the session isn't open, and only then calls send(); a None from send() cancels the task without re-queuing. So on an MH201 the single-attempt path only covers a drop on an already-open session. Approving.

@GreenGrassBlueOcean
GreenGrassBlueOcean merged commit a2ce7fb into master Oct 20, 2026
18 of 20 checks passed
@GreenGrassBlueOcean
GreenGrassBlueOcean deleted the fix/pr132-review-followup branch October 20, 2026 15:03
GreenGrassBlueOcean added a commit to GreenGrassBlueOcean/OWNd that referenced this pull request Oct 20, 2026
…penWebNet-HA#132, OpenWebNet-HA#133 to the changelog

Brings in OpenWebNet-HA#132 (typed exceptions, fast send reconnect), OpenWebNet-HA#133 (discovery
shim removal, MH201 retry notes) and OpenWebNet-HA#129 (expose builders, query helpers,
tri-state profile flags). OWNd/__init__.py keeps master's exception
re-exports with __version__ = "2.0.0b11". Changelog dated 2026-10-20.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants