feat(052): runtime-neutral connector I/O - #248
Merged
Merged
Conversation
…r enhanced connection handling
…cross multiple files
… for embassy adapter
…s for datagram handling
… Send future wrapper
…ared channel on std
…ent datagram handling
… update Makefile for MQTT checks
…rdize PLL configuration constants
Six fixes from review of the wave-A branch. All are in the new code; the existing tokio_*/embassy_* connector paths are untouched. embassy-adapter: restore the cancel and yield guards lost in the port The neutral `net` module reproduced `embassy_transport.rs`'s happy paths but not the edge cases that module's guards and comments exist for. - `EmbassyTcpDialer::connect` took the socket out of its slot and returned it only on connect's `Err` branch. A dial cancelled mid-`connect()` (a select timeout, a task shutdown) dropped the socket with the future, leaving the slot permanently empty and every later dial failing with a bare `TransportError::Io`. Both the dial and accept paths now hold the socket in a `SlotReturn` drop guard, as the sibling module does. - Neither path yielded before reporting a synchronously-failing attempt. Core's `serve` logs an accept error and re-enters `accept()` with no backoff, so a port-0 `InvalidPort` spun a non-yielding loop and starved the single-core cooperative executor — a config typo hanging the device rather than warn-looping. `yield_now().await` restored on both, with the comment naming the case. knx-connector: a rebind that cannot learn its address falls back to NAT `engine` outlives the bind loop and `set_local_endpoint` was called only inside `if let Some(..) = local_addr()`. On Embassy `local_addr()` is `None` whenever the stack has no address — DHCP renewal, link flap, which is what causes the rebind — so the next CONNECT_REQUEST re-advertised the previous cycle's port, the gateway replied to a dead port and the tunnel could never re-establish. Strictly worse than the `0.0.0.0:0` the explicit HPAI exists to avoid, so the `None` case is now explicit and falls back. Covered by a regression test that fails against the previous code. knx-connector: restore the select fairness tokio gave us `embassy_futures::select3` polls in declaration order, where the `tokio::select!` it replaced chose among ready arms at random. Sustained inbound traffic meant the command arm was never reached and outbound `GroupWrite`s stalled until the channel dropped them. The two contended arms now swap each pass. knx-connector: leave the critical-section impl to the final binary `tokio-runtime` enabled `critical-section/std`. That impl is registered by symbol name and is global to the binary, so per critical-section's own docs only the final binary may pick one; a downstream binary that also linked an impl got duplicate symbols with no way to opt out. The choice moves to an opt-in `critical-section-std-impl` feature, with a dev-dependency covering this crate's own test binaries. Makefile: actually check the new module's docs `RUSTDOCFLAGS=-D warnings` was added and `net` reached the tokio adapter's `cargo doc` line but not the embassy adapter's, leaving the largest new file in the crate unchecked — and failing with three errors when run. Feature added and the broken doc links fixed, so `make doc` covers it. Verified: `make doc` and `make examples` end to end; clippy `-D warnings` on each touched feature configuration including `thumbv7em` cross-compiles; knx (37 lib + 35 integration), embassy-adapter `alloc,net`, tokio-adapter `net`, serial, and tcp `neutral_pool` against two real embassy stacks; `cargo fmt --check`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019H6kuPb5RKWRX1irYZwHrZ
`neutral` named these modules by contrast with the per-runtime modules they
displace, not by what they contain — both declarations said as much ("the
runtime-independent replacement for the two client modules"). Wave B deletes
that contrast partner, after which `neutral` distinguishes the module from
nothing: everything left in each crate is runtime-neutral, as `tunnel.rs`
already was without needing the word in its name.
The decay had started: in `aimdb-knx-connector/src/lib.rs`, `pub mod neutral`
sat under a `// Platform-specific implementations` header saying the opposite
of what it is.
Renaming now because `pub mod neutral` is public API. Wave A is additive and
unreleased, so this is the last moment the change is free rather than
breaking.
aimdb-knx-connector/src/neutral.rs -> src/client.rs
aimdb-serial-connector/src/neutral.rs -> src/framer.rs
aimdb-tcp-connector/tests/neutral_pool.rs -> tests/accept_pool.rs
aimdb-serial-connector/tests/neutral_framed.rs -> tests/framed.rs
aimdb-embassy-adapter/tests/neutral_udp.rs -> tests/udp.rs
This also settles on the convention core and the adapters already used for
the same layer — `session::io` and `net`, both named for their contents.
Pure rename: no content changes beyond the module declarations, one import,
the misfiled header comment, the Makefile's `--test` target, and the
changelog and design-doc pointers into these paths. Prose uses of the word
where it is a genuine adjective ("runtime-neutral", "role-neutral") are
untouched.
Verified: clippy `-D warnings` on both connectors for tokio and the
`thumbv7em` embassy cross-compile; knx (37 lib + 35 integration), serial,
`accept_pool` over two real embassy stacks, `udp`; `make doc`;
`cargo fmt --check`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019H6kuPb5RKWRX1irYZwHrZ
`framer.rs` next to `framing.rs` was a confusable pair for one subject: the COBS codec and that codec behind core's `Framer` trait. They are now one module, with the module doc naming the two layers and why only the second is feature-gated. The gate moves from the module declaration onto the items that need it — `CobsFramer`, the chunk sizes and the `FramedConnection` aliases name `aimdb_core::session`, which core gates on `connector-session`. `encode_frame` and `FrameAccumulator` never needed it and stay ungated, so the codec still builds with no runtime feature at all. Public paths change from `framer::*` to `framing::*`; the items keep their names. Verified: clippy `-D warnings` for tokio, the `thumbv7em` embassy cross-compile (which type-checks `_same_framed_connection_serves_the_uart`, the assertion that the two runtime paths have not diverged), and no runtime feature at all; serial tests; rustdoc on both runtimes; `make doc`; `cargo fmt --check`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019H6kuPb5RKWRX1irYZwHrZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements wave A of design 052 — runtime-neutral connectors: the layer that lets an adapter own sockets, clocks and name resolution while a connector owns framing and protocol. A new runtime then costs one adapter crate and zero connector edits.
This PR is additive. Nothing consumes the new code on the live paths yet — the existing
tokio_*/embassy_*connector modules are untouched and still own every production path. Migrating them (and deleting ~4 130 lines) is wave B.What lands
aimdb-coresession::io—ByteStream,StreamDialer,StreamListener,Datagram,DatagramBinder,Delay,Framer;FramedConnection+FramingDialer/FramingListener;OneShot<T>aimdb-tokio-adapternetfeature —TokioNet::tcp/listen/udp,TokioDelay. Every future a plainasync fn, nounsafeaimdb-embassy-adapternetfeature —EmbassyNet::tcp/listen::<N>/udp,EmbassyUart,EmbassyDelay. All force-Sendfor these paths lives hereaimdb-knx-connectorneutral::connection_task— one task for both runtimes;TunnelIo::sendgains+ Send;embassy-sync/embassy-futureson stdaimdb-serial-connectorneutral— the crate reduced to a COBSFramer; byte sources come from the adaptersaimdb-tcp-connectortests/neutral_pool.rs— the accept-pool proofaimdb-mqtt-connectorTlsOptions::newrequires aSendRNG;TlsSlotbecomesOneShot. Crate now carries zerounsafe impls (was two)Design decisions worth review
The
Sendbound sits on each trait's return type, not at the use site. Generic connector code must produceSendfutures at the boxing boundary, and return-type notation is still experimental on the pinned 1.98 toolchain. A std impl writes a plainasync fnand the compiler discharges it; an Embassy impl returns the adapter's force-Sendnewtype.aimdb-core/src/session/io.rscarries a compile-time assertion, so dropping a bound fails in core rather than three crates away.The Embassy listener stores one pending accept per slot rather than rebuilding them.
TcpSocket::acceptis a synchronouslisten()plus a barepoll_fn, so dropping the future does not un-listen the socket — but re-enteringaccept()on a listening socket is an error, and theabort()that makes it re-enterable is what drops theLISTEN.tests/neutral_pool.rsholds both halves to real sockets, with a rebuild-and-cancel pool as a negative control that loses a SYN arriving between accepts.Datagram::local_addris part of the contract. Without it, unifying the KNX task would have silently downgraded every Tokio deployment to the NAT-style0.0.0.0:0HPAI that some gateways reject.unified_task_advertises_the_real_local_endpointreads the CONNECT_REQUEST off the wire and asserts the real bound address.embassy-syncandembassy-futuresare executor-independent despite the names — neither pulls an executor, andembassy-futureshas no dependencies at all. Both now back the KNX task on std.CriticalSectionRawMutexis the onlySyncraw mutexembassy-syncoffers, and it is a link-time obligation on std, sotokio-runtimeenablescritical-section/stditself andtests/shared_channel_on_std.rsproves the binary links.Deviations from the design doc
EmbassyUart::new(rx, tx), not::split— the caller has already split the UART; this joins the halves.aimdb-serial-connector'stokio-runtimenow depends onaimdb-tokio-adapter, so its byte source comes from the adapter on both runtimes instead of being duplicated. Recorded in that crate's CHANGELOG as an explicit reversal.TlsOptionsgate is on the connector'sembassy-tlspath, not the demo — see below.Incidental fixes
RUSTDOCFLAGS=-D warningsonmake doc, which CI already runs — so this cannot regress silently.make exampleswas broken and never run in CI.stm32-metapac21 renamed the RCC enum variants, so all five embedded examples failed to compile. Fixed, and all five now build forthumbv8m.main-none-eabihf— their actual board architecture — instead ofthumbv7em.thumbv8m.mainis pinned inrust-toolchain.tomland the devcontainer.aimdb-corewithconnector-session; it does now.Verification
Every commit was verified in isolation: unit + integration tests, clippy
-D warningson each feature configuration,thumbv7emcross-compilation, rustdoc, andcargo fmt --check.make examplespasses end to end. Acceptance criterion 1 holds — core cross-compiles with the new traits and still contains zerounsafe.The three remaining
unsafe impls in connector crates are all inaimdb-tcp-connector/src/embassy_transport.rs, the module wave B deletes.Not yet run: a full
make checkacross the branch. Per-crate runs can miss feature-unification effects, and this branch changed feature graphs in four crates — CI is the check.