Skip to content

Add configurable handshake timeout#2064

Merged
syssi merged 5 commits into
rytilahti:masterfrom
Acrobot:configurable-handshake
Jun 27, 2026
Merged

Add configurable handshake timeout#2064
syssi merged 5 commits into
rytilahti:masterfrom
Acrobot:configurable-handshake

Conversation

@Acrobot

@Acrobot Acrobot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

The current lazy_discover flag only offers two extremes: handshake once on first request (True), or before every request (False). Devices that need a recent handshake but not on every request (referenced in HA issues #59215, #92774) have no middle ground.

This adds a handshake_timeout parameter (in seconds) to MiIOProtocol and Device. It tracks when the last handshake occurred and only re-handshakes when enough time has passed. Setting it to 0 matches lazy_discover=False, and leaving it unset preserves existing behavior.

The lazy_discover parameter is kept for backward compatibility - it's translated into the timeout internally.

Changes:

  • miioprotocol.py: _needs_handshake() replaces the inline boolean check, _last_handshake timestamp recorded on each handshake
  • device.py: threads handshake_timeout kwarg through to MiIOProtocol
  • 8 new tests covering all timeout/lazy_discover combinations

Full test suite: 1294 passed (same as master), no new failures. Ruff clean.

Addresses #1683
Fixes #1683

@codecov

codecov Bot commented Mar 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.64%. Comparing base (d7af466) to head (2a88a64).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2064      +/-   ##
==========================================
+ Coverage   83.49%   83.64%   +0.14%     
==========================================
  Files         199      199              
  Lines       19522    19537      +15     
  Branches     1058     1063       +5     
==========================================
+ Hits        16300    16341      +41     
+ Misses       3036     3006      -30     
- Partials      186      190       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@Acrobot

Acrobot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

These changes were generated using Claude Code under my supervision. I reviewed and approved all code before submitting. I'm not very familiar with this codebase, so apologies in advance for any mistakes.

@Acrobot Acrobot marked this pull request as ready for review March 27, 2026 00:51
@Acrobot Acrobot mentioned this pull request Mar 27, 2026
22 tasks
@rytilahti rytilahti added this to the 0.6.0 milestone Jun 27, 2026
@syssi syssi force-pushed the configurable-handshake branch from d2b2f65 to 49ab961 Compare June 27, 2026 15:26
@syssi syssi requested a review from rytilahti June 27, 2026 15:32
Comment thread miio/miioprotocol.py Outdated
Comment on lines +120 to +122
The decision is based on _handshake_timeout, which is set in __init__
either from the explicit handshake_timeout parameter, or derived from
the legacy lazy_discover flag:

@rytilahti rytilahti Jun 27, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It is enough to describe the logic briefly, no need to name the exact variable names etc. in this docstring.

Maybe the main docstring for the class could describe why you want to use these variables (lazy_discover was never really properly documented), but it is also good as it is.

@rytilahti rytilahti left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Really nice to clean up the logic behind the handshaking, thanks @Acrobot!

Btw, "addresses #x" does not trigger the issue linking, I modified the description to make this PR to close the issue.

Acrobot and others added 5 commits June 27, 2026 18:13
Some devices need a recent handshake to respond, but the current
lazy_discover flag only offers two extremes: handshake once (True)
or before every request (False).

This adds a handshake_timeout parameter (in seconds) that re-handshakes
only when enough time has passed since the last one. The lazy_discover
flag is preserved for backward compatibility and translated into the
new timeout internally.

Addresses rytilahti#1683
Covers the two lines flagged by Codecov: the _last_handshake assignment
in send_handshake() and the _needs_handshake() call in send().
@syssi syssi force-pushed the configurable-handshake branch from 06e59df to 2a88a64 Compare June 27, 2026 16:13
@syssi syssi merged commit c8ed9a5 into rytilahti:master Jun 27, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace lazy discovery in favor of configurable time-since-last-handshake

3 participants