Skip to content

refactor(knx): extract sans-io KNX/IP tunneling state machine shared by the tokio and Embassy transports #135

@lxsaah

Description

@lxsaah

Context

docs/design/034-technical-debt-review.md §3.7: the KNX connector implements the entire KNX/IP tunneling lifecycle twice

File Lines Transport
aimdb-knx-connector/src/tokio_client.rs ~994 tokio::net::UdpSocket
aimdb-knx-connector/src/embassy_client.rs ~1,061 embassy_net::udp

The wire frames come from knx-pico; everything else — ConnectRequest → ConnectResponse handshake, TunnelingRequest/Ack sequence-counter bookkeeping, keepalive (ConnectionStateRequest) timing, disconnect handling, reconnect-with-backoff — is pure protocol logic that does not need to know where the bytes come from. Today every bug fix and feature lands twice or diverges.

This is the largest single duplication in the repo and the design doc's recommended first Phase 3 item. The repo already proves the target pattern at scale: the session engine (aimdb-core/src/session/) is a runtime-neutral engine driven through thin transports, and M17 moved the connector spines onto it.

Breaking changes are fine (internal APIs of the connector crate; the public KnxConnector builder surface should stay recognizable).

Tasks

  1. Extract a sans-io state machine (new module, no tokio/embassy imports — no_std + alloc):
    • Inputs: received datagrams, elapsed-time/deadline ticks, outbound GroupValueWrite commands.
    • Outputs: datagrams to send, next deadline to arm, events (connected, telegram received, disconnected/reconnect-after).
    • Owns: handshake state, channel id, sequence counters, ack bookkeeping, keepalive schedule, backoff policy.
  2. Reduce the runtime files to transport shims (~150 lines each): UDP socket I/O + a select loop feeding the state machine (datagram | deadline | command), mapping outputs back to socket sends. Timing via the runtime's primitives (or RuntimeOps once refactor(core)!: remove runtime type parameter R from the object graph — non-generic AimDb/TypedRecord/RuntimeContext (R-removal part 2) #131 lands).
  3. Keep the existing KnxSink/KnxSource + pump_sink/pump_source integration unchanged.
  4. Test the state machine without sockets: unit tests for handshake, ack timeout/retransmit, keepalive cadence, reconnect backoff — currently untestable without a live gateway or heavy mocking.

Acceptance criteria

  • One implementation of the tunneling lifecycle; grep -c "ConnectRequest" src/*.rs confirms handshake logic exists in exactly one module
  • tokio_client.rs and embassy_client.rs contain only socket/transport glue
  • tokio-knx-connector-demo and embassy-knx-connector-demo behave unchanged
  • State-machine unit tests cover handshake, ack retransmit, keepalive, and reconnect paths
  • Net LOC reduction in aimdb-knx-connector of roughly −1,200 (≈2,050 → ≈700–900)

Afterwards, apply the same review to the MQTT connector (harder — the duplication hides inside rumqttc vs mountain-mqtt; tracked separately if worthwhile).

Related: design doc §3.7; #130/#131 (RuntimeOps would slot in as the timing capability).

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions