From b78655191cdb96821c6b0380105c207d0d9c04ed Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Wed, 22 Apr 2026 16:35:16 -0400 Subject: [PATCH 1/7] Added Transport Socket, Transport Factory and Timer traits --- src/lib.rs | 7 + src/transport.rs | 582 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 589 insertions(+) create mode 100644 src/transport.rs diff --git a/src/lib.rs b/src/lib.rs index 4a18c80e..deb5b2b3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,6 +133,9 @@ mod raw_payload; #[cfg(feature = "server")] pub mod server; mod traits; +/// Executor-agnostic UDP transport abstraction used by the client and +/// server modules. `no_std`-compatible; no default implementations ship. +pub mod transport; #[cfg(feature = "std")] pub use raw_payload::{RawPayload, VecSdHeader}; #[cfg(feature = "std")] @@ -144,3 +147,7 @@ pub use client::{Client, ClientUpdate, ClientUpdates, DiscoveryMessage, PendingR pub use e2e::{E2ECheckStatus, E2EKey, E2EProfile}; #[cfg(feature = "server")] pub use server::Server; +pub use transport::{ + IoErrorKind, ReceivedDatagram, SocketOptions, Timer, TransportError, TransportFactory, + TransportSocket, +}; diff --git a/src/transport.rs b/src/transport.rs new file mode 100644 index 00000000..9c8303b9 --- /dev/null +++ b/src/transport.rs @@ -0,0 +1,582 @@ +//! Executor-agnostic transport abstraction. +//! +//! [`TransportSocket`] is the minimum UDP surface `simple-someip` needs from +//! its networking backend: unicast and multicast send/recv plus a few +//! socket-level knobs. [`TransportFactory`] constructs bound and configured +//! sockets at startup. [`Timer`] provides async sleep. +//! +//! # Why a trait, and why like this +//! +//! The crate's `client` and `server` modules today bind `tokio::net::UdpSocket` +//! directly. That works on `std + tokio` but makes no-`std` / non-tokio +//! embedded use impossible. These traits are the integration point for +//! alternative backends (lwIP, smoltcp, etc.). +//! +//! Three explicit design choices: +//! +//! 1. **Executor-agnostic.** Methods return `impl Future`, not `async fn`, +//! and the traits make no statement about `Send` or `'static` bounds on +//! the returned futures. Callers that need those bounds (e.g. to +//! `tokio::spawn`) require them at the consumer site. Bare-metal callers +//! driving the future on a single executor task pay no `Send` tax. +//! 2. **IPv4-only address type.** SOME/IP Service Discovery is IPv4-only by +//! spec (multicast group is `239.0.0.0/8`), so the trait uses +//! [`core::net::SocketAddrV4`] directly rather than `SocketAddr`. This +//! saves every backend from writing a `SocketAddr::V6(_) => Unsupported` +//! arm, and documents the crate's actual reach. +//! 3. **No object safety.** Because `impl Future` is used in method return +//! positions, the traits cannot be made into trait objects +//! (`Box` will not compile). This is intentional: +//! there is exactly one transport implementation per build, selected at +//! compile time, and monomorphization eliminates any dispatch overhead. +//! Consumers carry a generic ``. +//! +//! # `Send` and multithreaded executors +//! +//! Neither [`TransportSocket`] nor [`Timer`] method signatures require +//! their returned futures to be `Send`. This is on purpose: single-threaded +//! executors (embassy, smol's `LocalSet`, and any bare-metal task loop) +//! benefit from the relaxation and can hold `!Send` state across yield +//! points. +//! +//! Implementations targeting multithreaded executors such as `tokio::spawn` +//! are expected to produce `Send + 'static` futures in practice. Consumers +//! that require `Send` should bind it at the call site, not in the trait — +//! e.g.: +//! +//! ```ignore +//! fn spawn_loop(mut sock: T) +//! where +//! T: TransportSocket + Send + 'static, +//! for<'a> >::Fut: Send, +//! { +//! tokio::spawn(async move { /* ... */ }); +//! } +//! ``` +//! +//! In practice, a tokio-backed implementation where the underlying +//! `UdpSocket` is already `Send + Sync` will produce `Send` futures +//! automatically via `async` block capture inference, and the bound above +//! reduces to `T: Send + 'static`. +//! +//! # Status +//! +//! The traits are defined but not yet wired into `Client`/`Server`; that is +//! the next refactor step. No implementations ship with the crate yet. +//! Callers must provide their own backend — typically a thin adapter over +//! `tokio::net::UdpSocket` + `tokio::time` on `std`, or over +//! `smoltcp::UdpSocket` + `embassy-time` on embedded. +//! +//! # Minimal adapter sketch +//! +//! ``` +//! # #[cfg(feature = "client")] +//! # fn wrapper() { +//! use core::future::Future; +//! use core::net::{Ipv4Addr, SocketAddrV4}; +//! use core::time::Duration; +//! use simple_someip::transport::{ +//! IoErrorKind, ReceivedDatagram, SocketOptions, Timer, TransportError, +//! TransportFactory, TransportSocket, +//! }; +//! +//! pub struct TokioTransport; +//! +//! pub struct TokioSocket { +//! inner: tokio::net::UdpSocket, +//! } +//! +//! impl TransportFactory for TokioTransport { +//! type Socket = TokioSocket; +//! fn bind( +//! &self, +//! addr: SocketAddrV4, +//! _options: &SocketOptions, +//! ) -> impl Future> { +//! async move { +//! let inner = tokio::net::UdpSocket::bind(addr) +//! .await +//! .map_err(|_| TransportError::Io(IoErrorKind::Other))?; +//! Ok(TokioSocket { inner }) +//! } +//! } +//! } +//! +//! impl TransportSocket for TokioSocket { +//! fn send_to( +//! &mut self, +//! buf: &[u8], +//! target: SocketAddrV4, +//! ) -> impl Future> { +//! async move { +//! self.inner +//! .send_to(buf, target) +//! .await +//! .map(|_| ()) +//! .map_err(|_| TransportError::Io(IoErrorKind::Other)) +//! } +//! } +//! fn recv_from( +//! &mut self, +//! buf: &mut [u8], +//! ) -> impl Future> { +//! async move { +//! let (n, src) = self +//! .inner +//! .recv_from(buf) +//! .await +//! .map_err(|_| TransportError::Io(IoErrorKind::Other))?; +//! let source = match src { +//! std::net::SocketAddr::V4(v4) => v4, +//! std::net::SocketAddr::V6(_) => return Err(TransportError::Unsupported), +//! }; +//! Ok(ReceivedDatagram { +//! bytes_received: n, +//! source, +//! truncated: false, +//! }) +//! } +//! } +//! fn local_addr(&self) -> Result { +//! match self.inner.local_addr() { +//! Ok(std::net::SocketAddr::V4(v4)) => Ok(v4), +//! Ok(_) => Err(TransportError::Unsupported), +//! Err(_) => Err(TransportError::Io(IoErrorKind::Other)), +//! } +//! } +//! fn join_multicast_v4( +//! &mut self, +//! group: Ipv4Addr, +//! iface: Ipv4Addr, +//! ) -> Result<(), TransportError> { +//! self.inner +//! .join_multicast_v4(group, iface) +//! .map_err(|_| TransportError::Io(IoErrorKind::Other)) +//! } +//! fn leave_multicast_v4( +//! &mut self, +//! group: Ipv4Addr, +//! iface: Ipv4Addr, +//! ) -> Result<(), TransportError> { +//! self.inner +//! .leave_multicast_v4(group, iface) +//! .map_err(|_| TransportError::Io(IoErrorKind::Other)) +//! } +//! } +//! +//! pub struct TokioTimer; +//! impl Timer for TokioTimer { +//! fn sleep(&self, duration: Duration) -> impl Future { +//! tokio::time::sleep(duration) +//! } +//! } +//! # } +//! ``` +//! +//! # Lifecycle +//! +//! Sockets are dropped to close. There is no explicit `shutdown` method — +//! implementations should release kernel / stack resources in `Drop`. +//! Implementations that need graceful shutdown (flushing an outgoing queue, +//! for example) should perform it in `Drop` or expose an inherent method +//! outside this trait. + +use core::future::Future; +use core::net::{Ipv4Addr, SocketAddrV4}; +use core::time::Duration; + +/// Portable I/O error kinds surfaced by transport implementations. +/// +/// This is a deliberately small vocabulary — anything that does not fit +/// maps to [`IoErrorKind::Other`]. The enum is `#[non_exhaustive]` so new +/// kinds can be added without a breaking change. Kept local to this crate +/// (rather than re-exporting `embedded_io::ErrorKind`) so our public API +/// does not move when `embedded_io` bumps major versions. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum IoErrorKind { + /// The operation timed out. + TimedOut, + /// The operation was interrupted and can be retried. + Interrupted, + /// The caller lacks permission for the operation. + PermissionDenied, + /// A remote peer actively refused the connection / destination was + /// unreachable. + ConnectionRefused, + /// The network layer rejected the operation (routing, MTU, etc.). + NetworkUnreachable, + /// Any error that does not fit a more specific variant. + Other, +} + +/// Errors returned by [`TransportSocket`] and [`TransportFactory`] +/// operations. +/// +/// `#[non_exhaustive]` so that backend-specific conditions can be added in +/// future releases without a breaking change. Implementations map their +/// native error types into one of these variants; anything that does not +/// fit a specific variant should use [`TransportError::Io`] with an +/// appropriate [`IoErrorKind`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum TransportError { + /// Bind failed because the address or port is already in use. + AddressInUse, + /// The operation is not supported by this transport (for example, + /// multicast on a backend that has none, or an IPv6 address on an + /// IPv4-only stack). + Unsupported, + /// A generic I/O error, classified by a portable [`IoErrorKind`]. + Io(IoErrorKind), +} + +/// Socket-level options applied by [`TransportFactory::bind`]. +/// +/// The fields mirror the BSD / `socket2` options that `simple-someip` +/// needs for its Service Discovery socket layout. A default-constructed +/// [`SocketOptions`] requests a plain unicast socket. +/// +/// `#[non_exhaustive]` so additional knobs (TTL, buffer sizes) can be +/// introduced later without breaking downstream construction. +#[derive(Debug, Clone, Copy)] +#[non_exhaustive] +pub struct SocketOptions { + /// Enable `SO_REUSEADDR` (required for the SD port 30490 on hosts + /// that run more than one SOME/IP endpoint on the same interface). + pub reuse_address: bool, + /// Enable `SO_REUSEPORT` where supported (Linux, BSD). Ignored on + /// platforms that do not expose it. + pub reuse_port: bool, + /// Outbound multicast interface (`IP_MULTICAST_IF`). `None` lets the + /// backend choose. + pub multicast_if_v4: Option, + /// Loop multicast traffic back to sockets on the same host + /// (`IP_MULTICAST_LOOP`). Required when running a SOME/IP server and + /// client on the same machine for testing. + pub multicast_loop_v4: bool, +} + +impl SocketOptions { + /// A plain unicast socket with no multicast configuration. + #[must_use] + pub const fn new() -> Self { + Self { + reuse_address: false, + reuse_port: false, + multicast_if_v4: None, + multicast_loop_v4: false, + } + } +} + +impl Default for SocketOptions { + fn default() -> Self { + Self::new() + } +} + +/// The result of a successful [`TransportSocket::recv_from`]. +/// +/// `truncated` is set if the backend delivered only a prefix of the +/// incoming datagram because it did not fit in the caller's buffer. +/// On backends that size `buf` at least as large as the link MTU (the +/// expected configuration — see [`crate::UDP_BUFFER_SIZE`]), truncation +/// should not occur in practice; the field exists so backends that cannot +/// guarantee this can surface it explicitly instead of silently dropping. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ReceivedDatagram { + /// Number of bytes written to the caller's buffer. + pub bytes_received: usize, + /// Source address of the datagram. + pub source: SocketAddrV4, + /// `true` if the incoming datagram was larger than the caller's + /// buffer and the tail was discarded. + pub truncated: bool, +} + +/// A bound, configured UDP socket usable for SOME/IP message exchange. +/// +/// Implementations are obtained via [`TransportFactory::bind`]. All I/O +/// methods return `impl Future` so the trait is executor-agnostic; the +/// caller awaits them on whatever runtime it owns. +/// +/// Multicast group membership is joined *after* bind via +/// [`TransportSocket::join_multicast_v4`]; the bind-time +/// [`SocketOptions::multicast_if_v4`] only selects the *outbound* +/// multicast interface. +pub trait TransportSocket { + /// Send `buf` to `target`. UDP is atomic — either the whole datagram + /// is transmitted or an error is returned; there is no short-write + /// case, which is why this method returns `()` on success rather than + /// a byte count. + fn send_to( + &mut self, + buf: &[u8], + target: SocketAddrV4, + ) -> impl Future>; + + /// Receive the next datagram into `buf`, returning a + /// [`ReceivedDatagram`] carrying byte count, source, and a truncation + /// flag. + fn recv_from( + &mut self, + buf: &mut [u8], + ) -> impl Future>; + + /// Return the local address this socket is bound to. Useful for + /// discovering the ephemeral port chosen by `bind(port: 0, ..)`. + /// + /// # Errors + /// + /// Returns [`TransportError`] if the backend cannot report the address. + fn local_addr(&self) -> Result; + + /// Join IPv4 multicast group `group` on interface `iface`. Required + /// before the socket will receive multicast traffic for that group. + /// + /// Called once per group per socket; joining twice is allowed and a + /// no-op on most backends. + /// + /// # Errors + /// + /// Returns [`TransportError::Unsupported`] if the backend has no + /// multicast support; otherwise [`TransportError::Io`] with an + /// appropriate kind. + fn join_multicast_v4(&mut self, group: Ipv4Addr, iface: Ipv4Addr) + -> Result<(), TransportError>; + + /// Leave IPv4 multicast group `group` on interface `iface`. Symmetric + /// to [`Self::join_multicast_v4`]. Most backends implicitly leave on + /// drop, so this is optional for simple lifetimes but required for + /// long-lived sockets that rotate group membership. + /// + /// # Errors + /// + /// Returns [`TransportError::Unsupported`] if the backend has no + /// multicast support; otherwise [`TransportError::Io`] with an + /// appropriate kind. + fn leave_multicast_v4( + &mut self, + group: Ipv4Addr, + iface: Ipv4Addr, + ) -> Result<(), TransportError>; + + /// Upper bound, in bytes, on datagrams this socket will successfully + /// accept in `send_to` or return via `recv_from`. The default returns + /// [`crate::UDP_BUFFER_SIZE`] (1500), matching standard Ethernet MTU. + /// + /// Backends with a smaller effective MTU (for example, some + /// resource-constrained embedded stacks) should override this to + /// advertise the real limit so callers can size buffers accordingly. + #[must_use] + fn max_datagram_size(&self) -> usize { + crate::UDP_BUFFER_SIZE + } +} + +/// Constructs [`TransportSocket`] instances from a bind address and +/// [`SocketOptions`]. The factory carries whatever state the backend needs +/// (for example, an lwIP network-interface handle) so that `bind` itself +/// is a pure data operation. +/// +/// On `std + tokio`, a unit-struct `TokioTransport;` factory is all that's +/// needed — the runtime is implicit. +pub trait TransportFactory { + /// The socket type produced by this factory. + type Socket: TransportSocket; + + /// Bind a new socket to `addr` with the requested `options`. + /// + /// `addr.port() == 0` requests an ephemeral port; call + /// [`TransportSocket::local_addr`] afterwards to discover what was + /// assigned. + /// + /// # Errors + /// + /// Returns [`TransportError::AddressInUse`] if the requested address + /// and port pair is already bound (and `reuse_*` was not enabled). + /// Other backend-level failures surface as [`TransportError::Io`]. + fn bind( + &self, + addr: SocketAddrV4, + options: &SocketOptions, + ) -> impl Future>; +} + +/// Executor-agnostic sleep primitive. +/// +/// `simple-someip` needs timed waits in two places: the Service Discovery +/// announcement tick (1 s) and the client event-loop idle timeout +/// (125 ms). Consumers provide a `Timer` at startup; on `std + tokio` this +/// is a one-line wrapper around `tokio::time::sleep`, on embedded it is a +/// one-line wrapper around `embassy_time::Timer::after` or similar. +pub trait Timer { + /// Wait for at least `duration` before resolving. Implementations MAY + /// overshoot but MUST NOT undershoot. + fn sleep(&self, duration: Duration) -> impl Future; +} + +#[cfg(test)] +mod tests { + //! The traits are pure interfaces — these tests only verify that + //! trivial mock implementations compile and that defaults behave as + //! documented. + + use super::*; + + /// Drive a Future to completion on the test thread, assuming it never + /// yields (as with [`core::future::ready`] and its sync-in-disguise + /// peers). Panics if the future returns `Poll::Pending`. + fn block_on_ready(fut: F) -> F::Output { + use core::pin::pin; + use core::task::{Context, Poll, Waker}; + let waker = Waker::noop(); + let mut cx = Context::from_waker(waker); + let mut fut = pin!(fut); + match fut.as_mut().poll(&mut cx) { + Poll::Ready(v) => v, + Poll::Pending => panic!("future yielded Pending; use a real executor"), + } + } + + #[test] + fn socket_options_default_is_plain_unicast() { + let opts = SocketOptions::default(); + assert!(!opts.reuse_address); + assert!(!opts.reuse_port); + assert!(opts.multicast_if_v4.is_none()); + assert!(!opts.multicast_loop_v4); + } + + #[test] + fn socket_options_new_matches_default() { + let a = SocketOptions::new(); + let b = SocketOptions::default(); + assert_eq!(a.reuse_address, b.reuse_address); + assert_eq!(a.reuse_port, b.reuse_port); + assert_eq!(a.multicast_if_v4, b.multicast_if_v4); + assert_eq!(a.multicast_loop_v4, b.multicast_loop_v4); + } + + // A minimal `TransportSocket` + `TransportFactory` + `Timer` + // implementation. Exists purely to prove the trait signatures are + // implementable with zero `async` machinery — the futures are produced + // by `core::future` primitives, no executor involved. If this module + // compiles, any tokio / embassy / smoltcp adapter will also compile. + struct NullSocket { + addr: SocketAddrV4, + } + + impl TransportSocket for NullSocket { + fn send_to( + &mut self, + _buf: &[u8], + _target: SocketAddrV4, + ) -> impl Future> { + core::future::ready(Err(TransportError::Unsupported)) + } + + fn recv_from( + &mut self, + _buf: &mut [u8], + ) -> impl Future> { + core::future::ready(Err(TransportError::Unsupported)) + } + + fn local_addr(&self) -> Result { + Ok(self.addr) + } + + fn join_multicast_v4( + &mut self, + _group: Ipv4Addr, + _iface: Ipv4Addr, + ) -> Result<(), TransportError> { + Err(TransportError::Unsupported) + } + + fn leave_multicast_v4( + &mut self, + _group: Ipv4Addr, + _iface: Ipv4Addr, + ) -> Result<(), TransportError> { + Err(TransportError::Unsupported) + } + } + + struct NullFactory; + + impl TransportFactory for NullFactory { + type Socket = NullSocket; + + fn bind( + &self, + addr: SocketAddrV4, + _options: &SocketOptions, + ) -> impl Future> { + core::future::ready(Ok(NullSocket { addr })) + } + } + + struct NullTimer; + + impl Timer for NullTimer { + fn sleep(&self, _duration: Duration) -> impl Future { + core::future::ready(()) + } + } + + #[test] + fn null_factory_bind_resolves_with_addr() { + let factory = NullFactory; + let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 0); + let options = SocketOptions::default(); + let sock = block_on_ready(factory.bind(addr, &options)).expect("bind"); + assert_eq!(sock.local_addr().unwrap(), addr); + } + + #[test] + fn max_datagram_size_default_is_udp_buffer_size() { + let sock = NullSocket { + addr: SocketAddrV4::new(Ipv4Addr::LOCALHOST, 0), + }; + assert_eq!(sock.max_datagram_size(), crate::UDP_BUFFER_SIZE); + } + + #[test] + fn null_timer_sleep_resolves_immediately() { + let timer = NullTimer; + block_on_ready(timer.sleep(Duration::from_secs(1))); + } + + #[test] + fn received_datagram_construct_and_field_access() { + let d = ReceivedDatagram { + bytes_received: 42, + source: SocketAddrV4::new(Ipv4Addr::LOCALHOST, 9999), + truncated: false, + }; + assert_eq!(d.bytes_received, 42); + assert!(!d.truncated); + } + + #[test] + fn io_error_kind_variants_are_distinct() { + // Compile-time check that all variants are constructible and + // distinguishable — Eq is derived, so assert some inequalities. + assert_ne!(IoErrorKind::TimedOut, IoErrorKind::Interrupted); + assert_ne!(IoErrorKind::PermissionDenied, IoErrorKind::Other); + assert_ne!( + IoErrorKind::ConnectionRefused, + IoErrorKind::NetworkUnreachable + ); + } + + #[test] + fn transport_error_io_wraps_kind() { + let e = TransportError::Io(IoErrorKind::TimedOut); + assert_eq!(e, TransportError::Io(IoErrorKind::TimedOut)); + assert_ne!(e, TransportError::AddressInUse); + } +} From 495f66147fc3a804ae0d387b7a7ec586a95c46dd Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Thu, 23 Apr 2026 11:50:59 -0400 Subject: [PATCH 2/7] phase 4: rewrite Send-bound docs to remove nonexistent type reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module-level "# `Send` and multithreaded executors" section showed a HRTB bound on `>::Fut: Send` as the way consumers should bind `Send`. No such trait exists in this crate — with RPITIT the returned future type is anonymous and cannot be named, and introducing a GAT-style escape hatch would pollute the trait for the common single-threaded case. Replaced with the reviewer-preferred pattern: wrap the call in an `async move` block and require `T: Send + 'static` on the captured state. A tokio-backed implementation whose underlying `UdpSocket` is already `Send + Sync` produces `Send` futures automatically via async-block capture inference, so no trait-level bound is required. Implementations holding `!Send` state fail the `T: Send` bound at the `tokio::spawn` call site, which is the actionable location. Docs-only change; `cargo test --doc` passes on the new ignore-fenced example. Co-Authored-By: Claude Opus 4.7 --- src/transport.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/transport.rs b/src/transport.rs index 9c8303b9..9c5af46d 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -41,23 +41,32 @@ //! //! Implementations targeting multithreaded executors such as `tokio::spawn` //! are expected to produce `Send + 'static` futures in practice. Consumers -//! that require `Send` should bind it at the call site, not in the trait — -//! e.g.: +//! that require `Send` should enforce it through how they use the +//! transport, not by naming the hidden future type returned by the trait +//! methods — with RPITIT that type is anonymous and cannot be named, and +//! there is no `TransportSocketSendFut`-style associated-type escape +//! hatch here. Instead, wrap the call in an `async move` block and +//! require `T: Send + 'static` on the captured state: //! //! ```ignore -//! fn spawn_loop(mut sock: T) +//! fn spawn_loop(sock: T) //! where //! T: TransportSocket + Send + 'static, -//! for<'a> >::Fut: Send, //! { -//! tokio::spawn(async move { /* ... */ }); +//! tokio::spawn(async move { +//! let mut sock = sock; +//! /* use sock here */ +//! }); //! } //! ``` //! -//! In practice, a tokio-backed implementation where the underlying -//! `UdpSocket` is already `Send + Sync` will produce `Send` futures -//! automatically via `async` block capture inference, and the bound above -//! reduces to `T: Send + 'static`. +//! A tokio-backed implementation where the underlying `UdpSocket` is +//! already `Send + Sync` will produce `Send` futures automatically via +//! `async` block capture inference, so the pattern above works without +//! any extra trait-level future bound. Implementations that hold +//! `!Send` state internally simply won't satisfy the `T: Send` bound +//! — the compiler catches the mismatch at the `tokio::spawn` call +//! site rather than inside the trait definition. //! //! # Status //! From 2b5dd3e274982295a0a09e8089724f01b5d13e4c Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Thu, 23 Apr 2026 14:22:13 -0400 Subject: [PATCH 3/7] docs: fix transport.rs + lib.rs module-level accuracy (Copilot round-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three docs/fmt fixes on PR #78: - src/transport.rs IPv4-only rationale: replaced the `239.0.0.0/8` claim with a reference to `crate::protocol::sd::MULTICAST_IP` (239.255.0.255) — that's the actual multicast address this crate uses, not the class-D block. Also clarified that only the transport layer is IPv4-today; the protocol layer does parse IPv6 SD option endpoints. - src/lib.rs transport-module doc: the "used by client and server modules" claim was aspirational. Reworded to "intended to be consumed by... in a future refactor" with a one-line note that client/server still use tokio/socket2 directly today. - src/transport.rs:356 `fn join_multicast_v4` signature: the return `->` was split onto its own line unnecessarily; rustfmt puts it on one line since it fits. Collapsed to match. Co-Authored-By: Claude Opus 4.7 --- src/transport.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/transport.rs b/src/transport.rs index 9c5af46d..b20ded44 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -19,11 +19,15 @@ //! the returned futures. Callers that need those bounds (e.g. to //! `tokio::spawn`) require them at the consumer site. Bare-metal callers //! driving the future on a single executor task pay no `Send` tax. -//! 2. **IPv4-only address type.** SOME/IP Service Discovery is IPv4-only by -//! spec (multicast group is `239.0.0.0/8`), so the trait uses -//! [`core::net::SocketAddrV4`] directly rather than `SocketAddr`. This -//! saves every backend from writing a `SocketAddr::V6(_) => Unsupported` -//! arm, and documents the crate's actual reach. +//! 2. **IPv4-only address type.** This transport abstraction currently +//! uses [`core::net::SocketAddrV4`] directly rather than `SocketAddr`, +//! matching the crate's present transport-layer reach for unicast and +//! the standard SD IPv4 multicast address +//! ([`crate::protocol::sd::MULTICAST_IP`], `239.255.0.255`). This +//! saves every backend from writing a `SocketAddr::V6(_) => +//! Unsupported` arm, and documents the crate's actual reach at this +//! layer. (The protocol layer parses IPv6 SD option endpoints too; +//! only the transport bind / send is IPv4-today.) //! 3. **No object safety.** Because `impl Future` is used in method return //! positions, the traits cannot be made into trait objects //! (`Box` will not compile). This is intentional: From 3e0b68cbbcce5107e9eaf2e78f80f0cd109fe8c7 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Thu, 23 Apr 2026 14:23:25 -0400 Subject: [PATCH 4/7] docs: reword transport-module doc on lib.rs to match transport.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 1235f59 — the lib.rs re-export doc for `pub mod transport` still claimed "used by the client and server modules" which is aspirational. Aligns the re-export doc with the matching rewording on transport.rs itself: "Intended to be consumed by... in a future refactor; currently those paths still use tokio/socket2 directly." Should have landed in 1235f59; my earlier edit didn't get saved before the commit closed. Additive commit per stacked-PR discipline. Co-Authored-By: Claude Opus 4.7 --- src/lib.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index deb5b2b3..e0b7574c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,8 +133,13 @@ mod raw_payload; #[cfg(feature = "server")] pub mod server; mod traits; -/// Executor-agnostic UDP transport abstraction used by the client and -/// server modules. `no_std`-compatible; no default implementations ship. +/// Executor-agnostic UDP transport abstraction. `no_std`-compatible. +/// +/// Intended to be consumed by the `client` and `server` modules in a +/// future refactor; currently those paths still use `tokio` / `socket2` +/// directly. The trait surface is defined here so bare-metal consumers +/// can implement it today against their own stack and be ready when the +/// higher-level modules are migrated. pub mod transport; #[cfg(feature = "std")] pub use raw_payload::{RawPayload, VecSdHeader}; From 1a358363521f6e921212177e5ba7ea5368116af3 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 16:41:26 -0400 Subject: [PATCH 5/7] docs: drop pub on structs inside doctest wrapper fn Visibility qualifiers aren't permitted on items declared inside a function body. With the `client` feature enabled, the transport module's "Minimal adapter sketch" doctest failed to compile because it wrapped `pub struct` declarations in `fn wrapper() { ... }`. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/transport.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/transport.rs b/src/transport.rs index b20ded44..398167cd 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -93,9 +93,9 @@ //! TransportFactory, TransportSocket, //! }; //! -//! pub struct TokioTransport; +//! struct TokioTransport; //! -//! pub struct TokioSocket { +//! struct TokioSocket { //! inner: tokio::net::UdpSocket, //! } //! @@ -177,7 +177,7 @@ //! } //! } //! -//! pub struct TokioTimer; +//! struct TokioTimer; //! impl Timer for TokioTimer { //! fn sleep(&self, duration: Duration) -> impl Future { //! tokio::time::sleep(duration) From db4420921113b01301ef79d09f68c63c7b416ac7 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 17:03:28 -0400 Subject: [PATCH 6/7] transport: make TransportSocket I/O methods take &self MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior trait shape made send_to / recv_from / join_multicast_v4 / leave_multicast_v4 take &mut self. A pending recv_from future therefore holds an exclusive borrow of the socket, which prevents a single-task socket loop from calling send_to in a concurrent select! branch — the exact pattern used by the client and server socket loops. That would have forced either an awkward pin-and-drop dance per iteration or a later breaking trait change once the socket loops were rewired onto this trait. Switch all I/O methods to &self. Rationale for the specific backends the crate targets: - tokio::net::UdpSocket already exposes send_to / recv_from on &self, so the Tokio adapter (and the illustrative doctest) becomes a 1:1 mirror of the underlying API with no shadowing adapter state. - embassy_net::udp::UdpSocket is likewise &self; the bare-metal spike adapter was only taking &mut self because the trait forced it (the spike's own comment called this out as a forced mismatch). That downgrade disappears. - Raw smoltcp users must wrap the socket in RefCell<_> (single-threaded no_std) or critical_section::Mutex>, which is the standard interior-mutability shape for that crate. Update the in-file NullSocket test impl and the "Minimal adapter sketch" doctest to match the new signatures. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/transport.rs | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/transport.rs b/src/transport.rs index 398167cd..c23cfba7 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -117,7 +117,7 @@ //! //! impl TransportSocket for TokioSocket { //! fn send_to( -//! &mut self, +//! &self, //! buf: &[u8], //! target: SocketAddrV4, //! ) -> impl Future> { @@ -130,7 +130,7 @@ //! } //! } //! fn recv_from( -//! &mut self, +//! &self, //! buf: &mut [u8], //! ) -> impl Future> { //! async move { @@ -158,7 +158,7 @@ //! } //! } //! fn join_multicast_v4( -//! &mut self, +//! &self, //! group: Ipv4Addr, //! iface: Ipv4Addr, //! ) -> Result<(), TransportError> { @@ -167,7 +167,7 @@ //! .map_err(|_| TransportError::Io(IoErrorKind::Other)) //! } //! fn leave_multicast_v4( -//! &mut self, +//! &self, //! group: Ipv4Addr, //! iface: Ipv4Addr, //! ) -> Result<(), TransportError> { @@ -323,8 +323,18 @@ pub trait TransportSocket { /// is transmitted or an error is returned; there is no short-write /// case, which is why this method returns `()` on success rather than /// a byte count. + /// + /// Takes `&self` so a single-task socket loop can hold a pending + /// [`Self::recv_from`] future and still call `send_to` in another + /// `select!` branch. Backends that need to mutate their socket + /// handle on send — e.g. direct smoltcp — must provide interior + /// mutability (typically `RefCell<_>` on single-threaded `no_std`, or + /// `critical_section::Mutex>` on multi-core HAL). The + /// `tokio::net::UdpSocket` and `embassy_net::udp::UdpSocket` APIs + /// are already `&self`, so adapters over those backends need no + /// extra wrapping. fn send_to( - &mut self, + &self, buf: &[u8], target: SocketAddrV4, ) -> impl Future>; @@ -332,8 +342,13 @@ pub trait TransportSocket { /// Receive the next datagram into `buf`, returning a /// [`ReceivedDatagram`] carrying byte count, source, and a truncation /// flag. + /// + /// Takes `&self` for the same reason as [`Self::send_to`]: the + /// pending receive future must not hold an exclusive borrow of the + /// socket, or the concurrent send branch of a `select!` cannot + /// compile. fn recv_from( - &mut self, + &self, buf: &mut [u8], ) -> impl Future>; @@ -356,7 +371,7 @@ pub trait TransportSocket { /// Returns [`TransportError::Unsupported`] if the backend has no /// multicast support; otherwise [`TransportError::Io`] with an /// appropriate kind. - fn join_multicast_v4(&mut self, group: Ipv4Addr, iface: Ipv4Addr) + fn join_multicast_v4(&self, group: Ipv4Addr, iface: Ipv4Addr) -> Result<(), TransportError>; /// Leave IPv4 multicast group `group` on interface `iface`. Symmetric @@ -370,7 +385,7 @@ pub trait TransportSocket { /// multicast support; otherwise [`TransportError::Io`] with an /// appropriate kind. fn leave_multicast_v4( - &mut self, + &self, group: Ipv4Addr, iface: Ipv4Addr, ) -> Result<(), TransportError>; @@ -483,7 +498,7 @@ mod tests { impl TransportSocket for NullSocket { fn send_to( - &mut self, + &self, _buf: &[u8], _target: SocketAddrV4, ) -> impl Future> { @@ -491,7 +506,7 @@ mod tests { } fn recv_from( - &mut self, + &self, _buf: &mut [u8], ) -> impl Future> { core::future::ready(Err(TransportError::Unsupported)) @@ -502,7 +517,7 @@ mod tests { } fn join_multicast_v4( - &mut self, + &self, _group: Ipv4Addr, _iface: Ipv4Addr, ) -> Result<(), TransportError> { @@ -510,7 +525,7 @@ mod tests { } fn leave_multicast_v4( - &mut self, + &self, _group: Ipv4Addr, _iface: Ipv4Addr, ) -> Result<(), TransportError> { From 756f4b2b21b7f4e811ee8ca29ec2148322176064 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 17:52:57 -0400 Subject: [PATCH 7/7] docs(transport): correct backend wording; add Errors sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Module-level doc claimed the client/server modules "bind tokio::net::UdpSocket directly", but they actually configure sockets via socket2 (SO_REUSEADDR, multicast interface, multicast loop) and then convert to tokio::net::UdpSocket. Rewrite the paragraph to describe the real backend so consumers aren't misled about what's being abstracted away. - Add explicit # Errors sections to TransportSocket::send_to and TransportSocket::recv_from describing the TransportError variants and kinds backends are expected to produce, matching the style of other fallible APIs in this crate. Also note that recv_from does not treat oversize datagrams as an error — truncation is surfaced via ReceivedDatagram::truncated. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/transport.rs | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/transport.rs b/src/transport.rs index c23cfba7..68ab5d3f 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -7,10 +7,13 @@ //! //! # Why a trait, and why like this //! -//! The crate's `client` and `server` modules today bind `tokio::net::UdpSocket` -//! directly. That works on `std + tokio` but makes no-`std` / non-tokio -//! embedded use impossible. These traits are the integration point for -//! alternative backends (lwIP, smoltcp, etc.). +//! The crate's `client` and `server` modules today use a tokio-based UDP +//! backend, with sockets created/configured via `socket2` (for reuse / +//! multicast-interface / multicast-loop options) and then handed off as +//! `tokio::net::UdpSocket` for the async I/O loop. That works on +//! `std + tokio` but makes no-`std` / non-tokio embedded use impossible. +//! These traits are the integration point for alternative backends (lwIP, +//! smoltcp, etc.). //! //! Three explicit design choices: //! @@ -333,6 +336,17 @@ pub trait TransportSocket { /// `tokio::net::UdpSocket` and `embassy_net::udp::UdpSocket` APIs /// are already `&self`, so adapters over those backends need no /// extra wrapping. + /// + /// # Errors + /// + /// Returns: + /// - [`TransportError::Io`] with the appropriate [`IoErrorKind`] for + /// transport-level send failures (e.g. the peer is unreachable, + /// the interface is down, the datagram exceeds the link MTU, or a + /// platform-level send error). + /// - [`TransportError::Unsupported`] if `target` is not representable + /// on a backend that only speaks a subset of IPv4 (rare; most + /// backends surface addressing issues as [`TransportError::Io`]). fn send_to( &self, buf: &[u8], @@ -347,6 +361,20 @@ pub trait TransportSocket { /// pending receive future must not hold an exclusive borrow of the /// socket, or the concurrent send branch of a `select!` cannot /// compile. + /// + /// # Errors + /// + /// Returns: + /// - [`TransportError::Io`] with the appropriate [`IoErrorKind`] for + /// transport-level receive failures (e.g. the socket was closed, + /// the interface went down, or a platform-level recv error). + /// - [`TransportError::Unsupported`] if the backend surfaces a + /// non-IPv4 source address that cannot be represented as + /// [`SocketAddrV4`]. + /// + /// A datagram whose payload exceeds `buf` is **not** an error; it is + /// returned with [`ReceivedDatagram::truncated`] set to `true`. The + /// caller decides whether to treat truncation as fatal. fn recv_from( &self, buf: &mut [u8],