Feat/platform agnostic tcp connectors - #250
Open
lxsaah wants to merge 14 commits into
Open
Conversation
…connectors with length-prefix framing
… listener for improved framing support
- Updated `Cargo.toml` and `Cargo.lock` to remove unnecessary dependencies and streamline the project. - Modified `endpoint.rs` to utilize the new `framed_dialer_at` function for TCP connections. - Enhanced `connector.rs` with a new `split_host_port` function to handle host:port parsing and added a `framed_dialer_at` function for cleaner dialing. - Removed the `embassy_transport.rs` and `tokio_transport.rs` files as they are superseded by the new connector implementation. - Updated `lib.rs` to reflect the changes in module structure and removed deprecated transport modules. - Adjusted example in `tcp_demo.rs` to align with the new dialing approach.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ing and update related functions
…er error reporting
…ove server host handling in tests
…recated runtime aliases
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.
Wave B of design 052, first connector: the TCP connector moves onto the runtime-neutral I/O layer wave A added. The adapter owns the socket; this crate contributes only length-prefix framing.
What this delivers
tokio_transport.rs(302 lines) andembassy_transport.rs(591) are deleted, and with them the entireall(tokio-runtime, embassy-runtime)aliasing block —TokioTcpServer/EmbassyTcpServer,TokioTcpConnection/EmbassyTcpConnectionand friends existed only to disambiguate two implementations of the same concept.aimdb-tcp-connector/srcgoes from 1 044 lines to 487.unsafe impls in any connector crate. The three here were the last outside the adapters.tokio,embassy-net,embassy-futuresorembedded-io-async— checked withcargo tree, not by deleting files and assuming.tokiois a dev-dependency;embassy-netis reachable only under the_test-embassy-loopbackfeature.Breaking
TcpServer::newtakes an already-bound listener rather than a bind string — the adapter does the binding.Framer::encodereturnsResult<(), FrameFault>, andnext_frame's error widens from()toFrameFault(aimdb-core). Both production framers and every test double are updated.TokioNet::listenreturnsstd::io::Resultinstead ofTransportResult(aimdb-tokio-adapter). Callers using?or.expect(..)need no change.split_host_portis fallible, returningEndpointError, and now lives inaimdb_core::session::endpoint— re-exported here under the same path.TransportErrorgainsFramingandBusy. It is#[non_exhaustive], so this breaks no downstream match.What review changed
The first four commits did the migration. The remaining nine came out of review and are most of this diff's insertions. Almost all of it restores behaviour the first cut dropped, rather than adding anything new.
Errors the migration stopped reporting. Each of these had become silent:
FrameFault::Fatal→TransportError::Framing, connection closessendreturnedOk(())IoAddrInUse/PermissionDenied/AddrNotAvailablesurviveIo, indistinguishable from a dead peerTransportError::Busy, and the existing dial-failed log names itConfigurability the migration dropped.
max_framehad become unreachable: the framed type aliases usedfn() -> LengthFramer, which is nameable but stateless, so it could only ever produce the 64 KiB default. ALengthFramersfactory carries the bound and is still nameable, restoringTcpServer::max_frame(n),TcpClient::bounded(..)and the*_boundedconstructors.Endpoint grammar: one implementation instead of three.
split_host_portmangled unbracketed IPv6 (fe80::1→ hostfe80:, port 1) and silently substituted the default port for a malformed one, so a typo dialled a different — possibly live — service. It is now correct, fallible, and moved intoaimdb-corebecauseaimdb-clienthad grown a 56-line private copy and cannot depend on this crate:parse_endpointresolvestcp://URLs whether or not the TCP transport is compiled in. That copy is gone; the client keeps only its own policy, that a URL must name its port.One related bug is not fixed here:
aimdb_core::connector::parse_connector_urlhas the same unbracketed-IPv6 and silent-port behaviour. It is pre-existing onmain, backsConnectorUrlfor mqtt/knx/ws, and changing it needs its own testing per connector.Worth a look in review
Framerreports how badly it failed, not just that it did. A self-delimiting format resyncs on its next delimiter; a length prefix has none. Only the framer knows which case it is in, soFrameFaultlets it say, rather than the connection guessing. COBS keeps its existing skip-and-resync behaviour unchanged.TcpServeris single-use. The listener is moved in and taken on firstbuild, so a secondbuildfails rather than silently serving nothing.AimDbBuilderconsumes its connectors, so this is unreachable through the normal path; a direct caller wanting to serve again constructs a new server with a new listener.EmbassyTcpDialer: Cloneshares one socket. The derive exists so a framed dialer can satisfySessionClientConnector's bound. A clone dialing while another handle holds the link now getsBusy— waiting instead would be worse, sinceTcpSocketSlotholds room for exactly one waiter. A second concurrent connection needs a secondEmbassyNet::tcpwith its own buffers.Verification
Test counts on the paths this touches: framing 10, connector round-trip 5, tokio round-trip 1, Embassy loopback 5, core endpoint grammar 7. Run per feature leg rather than as one sweep, since
--all-featuresdoes not compose here:aimdb-core,aimdb-client(default /transport-serial/transport-tcp),aimdb-tcp-connector(_test-tokio,_test-embassy-loopback),aimdb-tokio-adapter,aimdb-embassy-adapter.Two of the new tests are mutation-verified — stubbing the code they guard makes them fail, which the previous suite would not have done for either:
a_policy_allowed_write_lands_through_the_built_server— dropswith_config→ the write returnsDenied; stubsapply_writable→record.listreportswritable: false.a_cloned_dialer_reports_a_busy_socket— a clone dialing over a live link must reportBusy, notIo.Also covered per feature leg:
clippy -D warnings,thumbv7em-none-eabihffor theno_stdbuild, andcargo docwithRUSTDOCFLAGS=-D warnings.