Skip to content

fix(protocol): retransmit and fail fast on the write path - #1

Closed
mbillow wants to merge 1 commit into
mainfrom
claude/smartthings-local-write-retry-i4kea9
Closed

fix(protocol): retransmit and fail fast on the write path#1
mbillow wants to merge 1 commit into
mainfrom
claude/smartthings-local-write-retry-i4kea9

Conversation

@mbillow

@mbillow mbillow commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Picks up the half of mbillow/localthings#384 that QuiteYellow left open after QuiteYellow#51 claimed the pacing half.

The asymmetry

post() sends its datagram exactly once:

self._send_dgram(datagram)
if not ev.wait(timeout):
    raise SessionTimeoutError()

get() goes through _exchange_block, which retransmits each block up to _BLOCK_MAX_ATTEMPTS at _BLOCK_ACK_TIMEOUT per attempt. So one lost datagram — request or ACK — is an unrecoverable write, while a read absorbs the identical loss silently. That matches the report in #384: reads keep working, three unrelated resources (/power/vs/0, /temperature/desired/0, /wind/direction/vs/0) intermittently don't.

The bare ev.wait() is a second, separate gap: it skips the _wait_for_block liveness slicing get() uses, so a reader thread dying mid-write burns the full 8 s and reports a device timeout for what is actually a dead session.

What this does

Liveness (active). _wait_for_block becomes _wait_live, and post() waits through it. A reader death mid-write now raises SessionClosedError within one liveness poll instead of at the end of the caller's timeout.

Retransmission (off by default). post() retransmits the CON up to write_max_attempts times inside the caller's deadline, with §4.2 backoff, pacing each retransmit, and the final attempt taking whatever budget is left.

The datagram is built once and resent verbatim. That MID reuse is the load-bearing part: a server implementing RFC 7252 §4.5 recognises the duplicate and answers from its dedupe cache instead of re-running the write. A caller-side retry can't offer that — post() mints a fresh MID and token per call, so a retry from above is a genuinely new request the device has no way to dedupe.

It defaults to 1 attempt — byte-for-byte today's behaviour — per @QuiteYellow's ordering caution on #384: retransmitting into a device that's already dropping under load turns one lost write into several, and §4.5 dedupe is unverified on RT-OCF, which doesn't reliably emit RST either. With QuiteYellow#51 landed we can see whether writes are still being lost before turning this on, and the flag is then a one-line change rather than a new feature.

refresh_observes() dereg sweep (active). Paced. Unlike the teardown dereg in close(), which wants out quickly, this one runs against a session that has to keep working afterwards, and an unpaced OBSERVE burst is what wedges an appliance (mbillow/localthings#396).

What this deliberately does not touch

Pacing of subscribe(), post(), and block zero — that's QuiteYellow#51, and putting a second layer at the call sites would only have to be unwound when it lands. The time.sleep(0.05) in the refresh_observes subscribe sweep is left alone for the same reason: subscribe() is where that belongs.

Tests

tests/test_dtls_session_post_retry.py, on the existing _FakeConn/_FakeSock harness:

  • the default sends exactly once — no new load on a device nobody has measured yet
  • a dropped first datagram is recovered when the flag is on
  • retransmits carry the same MID and token as the original
  • attempts stay inside the caller's deadline
  • a reader death mid-post() raises SessionClosedError fast, not SessionTimeoutError
  • refresh_observes paces its dereg sweep

Full suite: 275 passed.


Generated by Claude Code

post() sent its datagram exactly once and waited on a bare ev.wait(),
while get() retransmits every block through _exchange_block. One lost
datagram was therefore an unrecoverable write and a silent no-op for a
read -- the asymmetry behind three unrelated resources on one AC all
failing with SessionTimeoutError (LocalThings#384).

- Extract _wait_live() from _wait_for_block() and wait through it in
  post(). A reader thread dying mid-write now raises SessionClosedError
  within a liveness poll instead of burning the caller's whole timeout
  and reporting a dead session as a device timeout.

- Retransmit the CON up to write_max_attempts times inside the caller's
  deadline, reusing the same Message ID. The MID reuse is what makes
  retransmitting a non-idempotent POST safe: a server implementing
  RFC 7252 4.5 answers the duplicate from its dedupe cache rather than
  re-running the write, which a caller-side retry can never offer since
  it mints a fresh MID. Defaults to 1 attempt -- byte-for-byte today's
  behaviour -- because a device already dropping under load turns one
  lost write into several, and RT-OCF's dedupe is unverified. Enable it
  once pacing has been shown insufficient on real hardware.

- Pace the dereg sweep in refresh_observes(). Unlike the teardown dereg
  in close(), which wants out quickly, this one runs against a session
  that has to keep working afterwards, and an unpaced OBSERVE burst is
  what wedges an appliance until something forces a new session
  (LocalThings#396).

Pacing of the request paths themselves is deliberately left alone:
subscribe(), post() and block zero are QuiteYellow#51,
and a second layer at the call sites would only have to be unwound when
it lands.
@mbillow mbillow closed this Aug 19, 2026
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.

1 participant