feat!: asynchronous rewrite (roombapy 2.0)

[johnnyh1975]

Breaking. The library is asynchronous throughout; the threaded client is
gone. Ready for an initial review

Why

Home Assistant's integration bridges the thread boundary in 111 places:
72 async_add_executor_job, 27 run_coroutine_threadsafe, 12
call_soon_threadsafe. 49 of the executor jobs wrap a roombapy call; the
other 23 are CPU-bound rendering that belongs there anyway. All 39 of
the run_coroutine_threadsafe/call_soon_threadsafe sites exist purely
because register_on_message_callback delivers on paho's thread. So this
removes 88 of the 111, and with them a class of intermittent bug: every
one of those sites is a hand-written rule that has to be remembered.

Structure

remote_client.py, roomba_factory.py and the threaded roomba.py are
removed. roomba.py, discovery.py, getpassword.py and cli.py are the
async implementations under their existing names.

state.py holds the state machine, extracted verbatim: the logic that has
been running against real robots for years is byte-identical, only its
location changed. tls.py holds the SSL context shared by MQTT and
password retrieval; the posture stays permissive on purpose, since these
robots present self-signed certificates and older firmware needs legacy
renegotiation.

Transport

aiomqtt, pinned >=2.5,<3. It drives paho from the event loop via
add_reader/add_writer rather than a thread, so it is a wrapper rather
than a second stack. The pin matters: 3.x replaces paho with an
MQTTv5-only backend and these robots speak 3.1.1.

The library owns reconnection with exponential backoff. This is not an
addition but a replacement: aiomqtt sets reconnect_on_failure=False,
whereas loop_start() gave every consumer automatic reconnection for
free. Without it the swap would have been a silent regression.

RoombaAuthError never triggers a retry. A rejected credential is not
transient, and a robot re-provisioned mid-session would otherwise be
hammered forever with a stale password.

API

• connect() either establishes a session or raises. Previously an auth
  rejection arrived via on_connect and merely left roomba_connected
  False, so callers polled a flag.
• Callbacks run on the event loop, accept async def, and return an
  unsubscribe handle. Previously a registered callback could never be
  detached.
• register_on_connection_state_callback reports both directions, so a
  caller need not infer recovery from the absence of further failures.
• watch() offers an async iterator with per-watcher queues and
  drop-oldest backpressure.
• master_state keeps dict[str, Any]. TypedDicts and the typed reported
  view are additive: the same dictionary, checked, opt-in per caller.
• send_command refuses a room-scoped command carrying an empty regions
  list. The robot omits the key in that case and cleans the whole house;
  gated on the vendor app's own emit condition.

Data

ROOMBA_STATES gains spot and completed, both present in the robot's
phase enum. An unlisted phase reaches the branch that logs 'please
create a new issue'.

vendor_errors.py carries iRobot's own fault texts in eight languages.
Provenance and the unverified overlap with our own labels are stated in
the module, and a test keeps the overlap set honest as either table
changes.

Robustness

Four failures found by probing the implementation rather than by report,
each with a test that fails without its fix:

• The read loop caught only MqttError, so anything else — a malformed
  payload the state machine choked on, a synchronous callback that
  raised — escaped the supervisor task and ended it permanently: no
  reconnect, no callback, and the exception unretrieved until someone
  awaited the task. It now backs off and returns like any other loss.
• disconnect() cancelled callbacks that were still running, so a
  consumer persisting state from one lost it on unload. It now waits
  briefly, then cancels what is stuck and says so.
• A second connect() was a silent no-op that looked like success. It
  raises.
• The backoff had no jitter, so every robot in a household would return
  in lockstep after a router restart.
• aiomqtt's own MqttError escaped send_command: the connected check is
  only a pre-check, and the session can drop between it and the publish.
  A caller told to catch RoombaError met a third-party type instead.
• Discovery could not tell a hostname typo from a robot that is switched
  off. The datagram never left, the error landed on error_received where
  nobody was looking, and the caller waited out the whole window for a
  None. Addresses are now resolved first.

The public surface was then audited call by call — every await into
aiomqtt or asyncio across all four modules — so these are the ones that
were found rather than the ones that exist.

Compatibility

Python floor raised to 3.11 for asyncio.timeout and Self. CI tests
3.12-3.14 while the project declared >=3.10, so the two were already out
of step; worth aligning either way.

Two things I could not check and would rather flag than have you
discover:

Python 3.14. aiomqtt 2.5.1 declares >=3.8 but its classifiers stop
at 3.13, so the 3.14 job is the most likely place for this to go red
despite everything else passing. I developed against 3.12.

Hardware. None of this has run against a robot. It was developed
against this repository's own CI broker configuration and against
recorded payloads, so the wire format, the state transitions and the
reconnect mechanics are verified against a broker that can be killed on
demand — but not against firmware. In particular: whether the robot
sends a full state on connect or only deltas, and whether a sleeping
robot closes the session or merely goes quiet until the keepalive
expires. The second decides whether the backoff constants are sensible.

Everything else was run as CI runs it: uv sync --locked, pytest, ruff
check, ruff format --check, mypy, and each pre-commit hook including
codespell.

[johnnyh1975]

@pschmitt
The description covers the what and why, so just a note on how to approach the diff — 6,900 added lines is a lot to put in front of someone.

About 4,000 of those are generated data: vendor_errors.py is the fault-text table and uv.lock. Neither rewards line-by-line reading. The table is worth a spot-check on the module docstring (provenance, and the UNVERIFIED_OVERLAP caveat you asked for) and otherwise trusting the test that keeps that overlap set matching reality.

That leaves roughly 2,900 lines of actual code, and if you only look at three things I'd suggest:

• roomba.py, the _supervise loop. Everything about reconnection lives there, and it's the part with no equivalent in 1.x — so it's the part that had no prior art to be wrong about. It's also where the four robustness fixes landed.
• state.py. The claim is that the state machine is byte-identical to what's been running for years, only moved off a class whose constructor built a thread. tests/test_state.py is what turns that into something checkable rather than something I assert.
• types.py and the reported property. The additive typing you approved. Worth confirming it really is additive — master_state still returns the same dict[str, Any], and reported is a view of the same object rather than a copy.

Two further things:

If reviewing it whole turns out to be worse than reviewing it in pieces, say so and I'll split it. You asked for one PR and I think that was right, but you're the one who has to read it.

And the region guard on send_command wasn't in the design you approved — it came out of the vendor app's own emit condition while I was building. If you'd rather the library didn't have an opinion there, it comes out cleanly; it's one function and one exception type.

[Copilot]

🔵 Needs a closer look

It’s a large breaking transport/API rewrite with lifecycle supervision changes, and the review found at least two concrete correctness/robustness issues that should be addressed before merge.

Pull request overview

This PR delivers the breaking 2.0 rewrite of roombapy to be asynchronous end-to-end, replacing the prior threaded/paho client with an event-loop-driven MQTT transport (aiomqtt), and reworking discovery, password retrieval, CLI usage, and tests to match the new async API surface.

Changes:

• Replaced the threaded client stack (remote_client.py, roomba_factory.py, old Roomba) with an async RoombaClient featuring supervised reconnect/backoff, async callbacks with unsubscribe handles, and a watch() async iterator.
• Extracted shared components: state machine into state.py, TLS context generation into tls.py, and added opt-in typed views of reported state via types.py.
• Updated CLI/docs and added a broad new async-focused test suite; bumped version to 2.0.0, raised Python floor to 3.11, and pinned aiomqtt>=2.5,<3.0.

File summaries

File	Description
uv.lock	Updates lockfile for Python >=3.11 and adds/pins aiomqtt; removes now-unneeded backports/typing deps.
pyproject.toml	Bumps to 2.0.0, raises Python requirement, switches MQTT dep to aiomqtt, and adjusts lint/tool configuration.
README.md	Documents the new async API and provides upgrade guidance from 1.x to 2.0.
roombapy/roomba.py	Introduces the new async RoombaClient with supervisor/reconnect/backoff, callbacks, watch(), and outbound commands.
roombapy/state.py	Extracts the state machine logic into a transport-independent component.
roombapy/tls.py	Centralizes permissive TLS context generation shared across MQTT and password retrieval.
roombapy/types.py	Adds TypedDict-based opt-in typed views over master_state (e.g., reported).
roombapy/discovery.py	Rewrites discovery to async UDP, with explicit address resolution to fail fast on typos.
roombapy/getpassword.py	Rewrites local password retrieval to async TLS sockets with timeouts and safe teardown.
roombapy/cli.py	Updates CLI to use the async client and removes the prior busy-wait connect loop.
roombapy/init.py	Refreshes the public API exports for the 2.0 async surface and newly exposed helpers/types.
roombapy/const.py	Extends ROOMBA_STATES to include spot and completed phases.
roombapy/remote_client.py	Removes the legacy threaded/paho remote client implementation.
roombapy/roomba_factory.py	Removes the legacy factory used to construct the threaded client.
tests/conftest.py	Removes old threaded-client fixtures and adds broker diagnostics for intermittent failures.
tests/test_roomba.py	Removes legacy Roomba-class message handling test.
tests/test_roomba_integration.py	Removes legacy threaded integration test that crossed executor boundaries.
tests/test_remote_client.py	Removes legacy remote-client logging tests.
tests/test_client.py	Adds async “vertical slice” test against a real broker.
tests/test_commands.py	Adds async command and callback-contract tests (payload envelope, async callbacks, error behavior).
tests/test_message_handling.py	Adds parity tests for topic exclusion and periodic re-derive behavior.
tests/test_reconnect.py	Adds reconnection test using a private mosquitto instance (skips in CI without binary).
tests/test_supervisor.py	Adds supervisor behavior tests (auth failure no-retry, state callbacks, unsubscribe).
tests/test_robustness.py	Adds robustness regression tests (unexpected errors, second connect, callback draining, transport error wrapping).
tests/test_watch.py	Adds tests for watch() fan-out, backpressure/drop-oldest, and watcher detachment.
tests/test_scope.py	Adds tests for the region-scope guard and phase coverage.
tests/test_types.py	Adds tests that typed views are a direct view over the same dict and tolerate unknown keys.
tests/test_vendor_errors.py	Adds tests validating the vendor error text table and overlap invariants.
tests/test_discovery.py	Rewrites discovery/password tests for async behavior with local fakes.
tests/test_decode.py	Updates discovery decode test to use the new ROOMBA_MESSAGE constant.

Review details

• Files reviewed: 29/31 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[johnnyh1975]

Both review comments were valid; one was understated.

connect() refused whenever _task was set, but the supervisor deliberately
ends rather than retrying when credentials are rejected — so _task stayed
set and done, and reconnecting after re-provisioning the robot was
blocked. The error message even named that case as the reason to try. It
now refuses only while the supervisor is still running, and clears a task
that has ended.

The broker diagnostics hook read the log unguarded. Worse than described:
with the log path unreadable, pytest aborted the entire run with
INTERNALERROR rather than reporting the assertion. The read and the whole
diagnostics block are now guarded, since that text is never worth losing
a test result over.

Looking for the pattern rather than the instance turned up a third: the
same "set is not usable" conflation in discovery._open(), which handed
back a protocol whose socket asyncio had already closed. _send then
called into a dead transport and a bare AttributeError escaped the public
API. The protocol now records connection_lost.

Each fix has a test that fails without it. The first test written for the
connect() fix passed without it — a first connect that fails cleans up
after itself, so the defect only exists mid-session.

[Copilot]

🟡 Changes recommended

There are API-contract issues in the async client (disconnect callbacks firing during initial connect attempts and non-idempotent unsubscribe handles) plus a docstring markup error that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

roombapy/roomba.py:216

• The unsubscribe handle returned by register_on_message_callback is not idempotent: calling it twice (or after another removal) raises ValueError from list.remove. Unsubscribe handles are commonly used in finally blocks, so making this safe prevents cleanup paths from raising unexpectedly.

This issue also appears in the following locations of the same file:

• line 218
• line 225

        self, callback: MessageCallback
    ) -> Unsubscribe:
        """Register a message callback; returns a detach handle."""
        self._on_message.append(callback)
        return lambda: self._on_message.remove(callback)

roombapy/state.py:58

• The dict_merge docstring uses invalid reStructuredText markup (`:meth:``dict.update()```). This will render incorrectly (and may be flagged by doc tooling).

        """Recursive dict merge.

        Inspired by :meth:``dict.update()``, instead
        of updating only top-level keys, dict_merge recurses down into dicts
        nested to an arbitrary depth, updating keys. The ``merge_dct`` is

roombapy/roomba.py:223

• The unsubscribe handle returned by register_on_disconnect_callback is not idempotent: calling it more than once will raise ValueError. Making the handle safe to call repeatedly avoids cleanup errors and matches typical unsubscribe semantics.

    def register_on_disconnect_callback(
        self, callback: ErrorCallback
    ) -> Unsubscribe:
        """Register a disconnect callback; returns a detach handle."""
        self._on_disconnect.append(callback)
        return lambda: self._on_disconnect.remove(callback)

roombapy/roomba.py:235

• The unsubscribe handle returned by register_on_connection_state_callback can raise ValueError if called twice. Since unsubscribe handles are often called defensively (e.g., in teardown), suppressing ValueError makes this API safer to use.

    def register_on_connection_state_callback(
        self, callback: StateCallback
    ) -> Unsubscribe:
        """Register for connection-state changes; returns a detach handle.

        Fires on every transition, not only on loss, so a caller can mark an
        entity unavailable and available again without inferring the second
        half from the absence of the first.
        """
        self._on_state.append(callback)
        return lambda: self._on_state.remove(callback)

• Files reviewed: 29/31 changed files
• Comments generated: 1
• Review effort level: Lite

[johnnyh1975]

@pschmitt
All three valid, fixed.

The unsubscribe handles are now safe to call repeatedly — that one bites exactly where handles are used, in finally blocks, where a ValueError from list.remove turns tidy-up into a second failure.

Disconnect callbacks during the initial connect: confirmed, two of them before any session existed, since the retry budget lets two attempts fail before connect() gives up. A consumer would have marked an entity unavailable that had never been available. A loss is now only reported for a session that actually existed.

The dict_merge docstring markup came from 1.x verbatim along with the state machine — the extraction carried it across rather than introducing it. Fixed anyway.

Each has a test that fails without it. 81 tests now.

[pschmitt]

Let's a go! Thanks for your work on this.

[johnnyh1975]

Thanks — and thanks for the three fixes you put on top rather than just merging.

The _resolve one is a follow-on bug of my own making: I added the resolution but kept comparing against the input, so get("roomba.local") would have resolved fine and then never matched the reply. Good catch. Same for running discovery and the password fetch concurrently in _connect — _discover already did that, so I was inconsistent with myself two functions up.

Also sorry about the stray .coverage file in the diff.