diff --git a/CHANGELOG.md b/CHANGELOG.md index 871ae902..7e135008 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,146 @@ # Changelog -## [Unreleased] +## [0.8.0] -### Added +### Client/Server API symmetry & ergonomics + +The 0.8.0 ergonomics pass aligning the public Client and Server surfaces, removing the tokio-only `Server::new` cliff, and improving discoverability for bare-metal adopters. Six bundled changes: generic-parameter alignment, tokio-defaulted `Deps` builders, channel-types rustdoc, `ServerConfig` fluent builder, `Server::new` constructor reshape (the one breaking change in this set), and `SubscriptionHandle` GAT promotion. Migration shapes below are written against the previous published version (0.7.0); `cargo build` will surface every remaining call-site. + +#### Breaking — `SubscriptionHandle::SubscribeFuture` / `UnsubscribeFuture` are now [generic associated types] + +The `subscribe` and `unsubscribe` methods on `SubscriptionHandle` previously returned `impl Future<…> + '_` (return-position impl Trait). They are now named GATs: + +```rust +pub trait SubscriptionHandle: Clone + 'static { + type SubscribeFuture<'a>: Future> + 'a where Self: 'a; + type UnsubscribeFuture<'a>: Future + 'a where Self: 'a; + + fn subscribe(...) -> Self::SubscribeFuture<'_>; + fn unsubscribe(...) -> Self::UnsubscribeFuture<'_>; + // for_each_subscriber stayed RPIT — no Server::run-side bound needs it. +} +``` + +Implementors must now spell their concrete return type (typically `Pin + Send + 'a>>`). All four in-tree implementations (`Arc>`, `StaticSubscriptionHandle`, the example `InMemorySubscriptions`, three test `MockSubscriptions` variants) were converted; downstream implementors will hit a compile error that names the missing associated types verbatim. + +Why this is worth a breaking change: it lets [`Server::run`]'s where clause spell `for<'a> Sub::SubscribeFuture<'a>: Send`, which in turn lets the function declare `+ Send` on its return type instead of relying on auto-trait inference. Compile errors for `tokio::spawn`-vs-`!Send`-handle mismatches now surface at the library boundary instead of deep inside `tokio::spawn`'s bound check. + +[generic associated types]: https://blog.rust-lang.org/2022/10/28/gats-stabilization.html + +##### Migration + +Before: + +```rust +impl SubscriptionHandle for MyHandle { + fn subscribe(&self, ...) -> impl Future> + '_ { + async move { /* … */ } + } + fn unsubscribe(&self, ...) -> impl Future + '_ { + async move { /* … */ } + } +} +``` + +After: + +```rust +impl SubscriptionHandle for MyHandle { + type SubscribeFuture<'a> = + Pin> + Send + 'a>>; + type UnsubscribeFuture<'a> = Pin + Send + 'a>>; + + fn subscribe(&self, ...) -> Self::SubscribeFuture<'_> { + Box::pin(async move { /* … */ }) + } + fn unsubscribe(&self, ...) -> Self::UnsubscribeFuture<'_> { + Box::pin(async move { /* … */ }) + } +} +``` + +Drop `+ Send` from the type aliases on `!Send` handles (rare). The `Box::pin` allocation happens at SD-rate (typically ≤ 1 Hz subscribes during steady-state SD churn), small cost relative to the wire activity it gates. + + + +#### Breaking — `Server::new` now returns a `(Server, ServerHandles, run-future)` tuple + +`Server::new` (and the `new_with_loopback`, `new_passive`, `new_with_deps`, `new_passive_with_deps` variants) now mirrors `Client::new`'s shape: the constructor returns a three-tuple of `(Server, ServerHandles, impl Future + 'static)` instead of just `Server`. The single returned run-future drives the receive loop *and* the SD `OfferService` announcement loop concurrently, so callers no longer have to remember to spawn `announcement_loop` separately. The runtime check that used to live on `Server::announcement_loop` ("called on passive server" → `Err`) is now structurally impossible because the announcement loop has no separate entry point. The dispatcher topology where a co-located `Client` drives SD announcements is now opted into via [`ServerConfig::with_announce(false)`] instead of "just don't call this method". + +`Server::new_with_handles` and `Server::new_passive_with_handles` are unchanged — bare-metal callers still get back `Self` and call `server.run_with_buffers(unicast, sd)` directly with their own static buffers. The combined receive + announce select runs in `run_with_buffers` too. + +The previous `ServerHandles` type (the no-alloc deps bundle accepted by `new_with_handles`) was renamed to **`ServerStorage`** to free the `ServerHandles` name for the new post-construction accessor struct returned from `Server::new`. The eight fields are unchanged. + +##### Migration + +Before: + +```rust +let mut server = Server::new(config).await?; +let publisher = server.publisher(); +tokio::spawn(server.announcement_loop()?); +tokio::spawn(async move { + if let Err(e) = server.run().await { /* … */ } +}); +``` + +After: + +```rust +let (_server, handles, run) = Server::new(config).await?; +let publisher = handles.publisher; +tokio::spawn(async move { + if let Err(e) = run.await { /* … */ } +}); +``` + +Bare-metal-no-alloc, before: + +```rust +let server = Server::new_with_handles(deps, config)?; +let announce = server.announcement_loop_local()?; +spawn_local(announce); +spawn_local(server.run_with_buffers(unicast_buf, sd_buf)); +``` + +After: + +```rust +let server = Server::new_with_handles(deps, config)?; +spawn_local(server.run_with_buffers(unicast_buf, sd_buf)); +``` + +(`run_with_buffers` is now the combined receive + announce future.) + +Dispatcher topology (was: implicit "just don't call announcement_loop") becomes explicit: + +```rust +let config = ServerConfig::new(svc, inst).with_announce(false); +let (_server, _handles, run) = Server::new(config).await?; +tokio::spawn(run); // receive only; co-located Client drives SD +``` + +#### Removed + +- **`Server::announcement_loop` / `Server::announcement_loop_local`** — folded into the combined run-future. The `announcement_loop_started: AtomicBool` latch that protected against two simultaneously-driven announcement futures is gone with them (single entry point makes the failure mode structurally impossible). +- **`Server::set_local_port`** — vestigial. The bind-time back-fill on `new_with_deps` / `new_with_handles` already records the kernel-assigned port into `config.local_port` before `Server::new` returns, and mutating it post-construction would lie to peers about an endpoint the unicast socket isn't actually bound to (the same failure mode `new_with_handles` rejects with `local_port_mismatch`). Both in-tree call sites were no-ops. + +#### Added (additive — no migration) + +- **`ServerHandles`** — post-construction accessor bundle returned alongside `Server` from `Server::new`. Single public field today: `publisher`. Reserved for future fields (e.g. a `BindCompleted` oneshot if a real adopter asks). +- **`ServerConfig::announce: bool`** + **`with_announce(bool)`** fluent setter. Defaults to `true`. Set to `false` for the dispatcher topology. +- **`ServerStorage`** (replaces the old `ServerHandles` deps bundle name). +- **21a — generic-parameter alignment** across `Client`, `Server`, `ClientDeps`, `ServerDeps`. Letter-collisions between Spawner (`S` on Client) and SubscriptionHandle (`S` on Server) resolved (`Sp` and `Sub` respectively). +- **21c — tokio-defaulted `Deps` builders.** `ClientDeps::tokio()` and `ServerDeps::tokio()` produce a fully-defaulted bundle; chain `with_factory` / `with_timer` / `with_e2e_registry` / `with_subscriptions` / `with_spawner` / `with_local_spawner` to override individual fields without spelling the rest out. +- **21d — discoverable channel types.** New `ClientChannelTypes` trait alias surfaces the set of channel types that `define_static_channels!` must populate; `client::channels` re-exports the relevant types with rustdoc that names each as "channel type for `define_static_channels!`." +- **21e — `ServerConfig` fluent builder.** Existing struct-literal route stays open; `ServerConfig::new(svc, inst).with_interface(…).with_local_port(…).with_ttl(…).with_event_group(…).with_announce(…)` is the recommended path in docs. + +### Added — correctness & no_std cleanup - **`simple-someip-embassy-net::LINK_MTU`** — `pub const usize = 1500` shared by the loopback driver and example consumers for sizing `SocketPool` RX/TX buffers and `Capabilities::max_transmission_unit`. Distinct from `simple_someip::UDP_BUFFER_SIZE` (an *application*-payload cap) — they coincide at 1500 today but are conceptually orthogonal. - **Per-package pedantic clippy CI gates** for `simple-someip` under `client+bare_metal`, `server+bare_metal`, and `client+server+bare_metal`. The pre-existing `--workspace --all-features` gate is feature-unified and could mask feature-set regressions; per-package gates surface a regression against its responsible feature flag. -### Changed +### Changed — correctness & no_std cleanup - **`SocketOptions` docs** — explicit Linux-side guidance that the SD socket needs both `SO_REUSEADDR` and `SO_REUSEPORT` (Linux ties multicast-group membership to the REUSEPORT group). - **`SdStateManager::with_initial` and `next_session_id_with_reboot_flag`** lifted from `pub(super)` to `pub` so external test harnesses can pre-seed counter state and validate wrap-around behaviour without a full Server lifecycle. The remaining racy accessors stay `pub(super) + cfg(test)`. @@ -18,7 +151,7 @@ - **`Server::run_with_buffers` doc example** — replaced unsound `&mut UNICAST_BUF` on `static mut` (hard error in Rust 2024) with a `static UnsafeCell<[u8; …]>` + `unsafe impl Sync` pattern. - **Three event-loop sites** (`client/inner.rs`, `client/socket_manager.rs`, `server/mod.rs`) — comments referenced `select!` while the code used `select_biased!`. Server and socket_manager's 2-arm selects now flip arm priority each iteration to approximate the fairness `select!` would give without pulling `std`. Comments rewritten to match. -### Fixed +### Fixed — correctness & no_std cleanup - **`tools/size_probe::someip_header_encode`** — `MessageType::try_from(byte & 0xBF)` masked off bit 6 before validation (`0x40` silently coerced to `Request`); switched to `MessageTypeField::try_from(byte)`. The encoder also ignored the caller's `length` field and hardcoded `payload_len = 0`; now derives `payload_len = length - 8` with `checked_sub`. - **`simple-someip-embassy-net::EmbassyNetFactory`** — dropped a bogus `'pool` lifetime parameter and an identity-only `mem::transmute<&SocketPool, &'static>`. Factory now takes `&'static SocketPool` directly. Marked `!Send + !Sync` via `PhantomData<*const ()>` because embassy-net's `Stack` interior `RefCell` is not safe to drive `bind()` on from multiple threads. @@ -36,9 +169,7 @@ - **Embassy-net loopback test rename pretext** — `client_send_request_server_runloop_stable` was vacuous (passive server's `run()` returns `Err(InvalidUsage)` immediately). Removed the no-op spawn and rewrote the doc to honestly describe what the test verifies (the client's send path). - **Adversarial-pass micro-issues**: `payload_len + 12` / `payload_len + 4` 32-bit wrap in size_probe (now `checked_add`); `PanicAllocator` → `NullAllocator` (it returns null, doesn't panic); `EmbassyNetBindFuture::poll` panicked on second poll (now wraps `core::future::Ready` for stdlib panic message + standard semantics); `EventPublisher`'s `PhantomData` → `PhantomData T>` (no redundant `Send + Sync` re-imposition). -## [0.8.0] - -### Added +### Added — 0.7.0 → 0.8.0 surface baseline - **`client::Error::Capacity(&'static str)`** — new variant returned when a fixed-capacity internal structure is full. Current tags: `"unicast_sockets"`, `"udp_buffer"`, `"pending_responses"`, `"request_queue"`. Because `client::Error` is not `#[non_exhaustive]`, this is a breaking change for downstream crates that match the enum exhaustively. - **`client::Error::Transport(crate::transport::TransportError)`** — new variant surfacing failures from the pluggable transport backend (`#[from]`-converted, displays transparently). Same exhaustive-match caveat as above. @@ -57,7 +188,7 @@ - **`E2ERegistryFull`** — new typed error returned by `E2ERegistry::register` (and propagated through `E2ERegistryHandle::register` / `Client::register_e2e` / `Server::register_e2e`) when the fixed-capacity registry is at its `E2E_REGISTRY_CAP` limit. Replacing an already-registered key still always succeeds. - **`PayloadWireFormat::for_each_offered_endpoint` / `for_each_service_instance`** — visitor-pattern methods replacing the previous `Vec`-returning `offered_endpoints` / `service_instances`. Lets the `Client` run loop iterate SD entries without per-message heap allocation, which was the last bare-metal blocker on the receive path. The `Vec`-returning forms are preserved as `cfg(feature = "std")` convenience wrappers that delegate to the visitors, so std consumers keep the original ergonomic shape. -### Changed +### Changed — 0.7.0 → 0.8.0 surface baseline - **Breaking: `Client::new(interface)` return shape** — previously returned `(Client, ClientUpdates)`; now returns `(Client, ClientUpdates, impl Future + Send + 'static)`. The third element is the run-loop future, which the caller MUST drive (typically via `tokio::spawn`) for any `Client` method to make progress. Migration: change destructuring to a 3-tuple and spawn or otherwise actively poll the future. - **Breaking: `Server::start_announcing` removed → `Server::announcement_loop`** — the new method returns `Result + Send + 'static, Error>` (annotated `#[must_use]`). Spawn the returned future to fire announcements; calling and dropping the future is a silent no-op. @@ -71,7 +202,7 @@ - **Breaking: `Server::new` type signature now `Server::::new`** — the `Server` struct gained type parameters for the pluggable backends. The tokio-default convenience constructor is now gated behind the `server-tokio` feature (was `server`). Migration: add `features = ["server-tokio"]` to continue using `Server::new`; trait-surface consumers use `Server::new_with_deps`. - **Breaking: `SubscriptionHandle` trait redesigned** — the previous `get_subscribers(&self, …) -> impl Future>` method has been replaced with `for_each_subscriber(&self, …, f: FnMut)` visitor pattern. This allows `EventPublisher::publish_event` to copy subscriber addresses into a stack buffer (`heapless::Vec<_, 16>`) instead of allocating per-event. Implementors of custom `SubscriptionHandle` must migrate. - **Breaking: `SubscriptionHandle` RPITIT futures no longer `+ Send`** — the `subscribe`, `unsubscribe`, and `for_each_subscriber` methods now return `impl Future<…>` without a `+ Send` bound. This enables single-threaded lock-free implementations on bare-metal targets, but means `SubscriptionHandle` trait objects cannot be held across `.await` points in multi-threaded executors. Direct usage with the default `Arc>` is unaffected. -- **Breaking: `client` and `server` features no longer imply `std`** — previously `client = ["std", "dep:futures"]` and `server = ["std", "dep:futures"]`; now `client = ["dep:futures-util"]` and `server = ["dep:futures-util"]`. The `std` feature moved to `client-tokio` / `server-tokio`, which is where it belongs (the tokio backends genuinely require std). Bare-metal trait-surface consumers (`features = ["client", "bare_metal"]`) compile in pure no_std now. `server` still pulls `extern crate alloc` because `Server` holds `Arc` and `EventPublisher` holds `Arc` — documented in `lib.rs`; refactor to `&'static` borrows is tracked for a future phase. +- **Breaking: `client` and `server` features no longer imply `std`** — previously `client = ["std", "dep:futures"]` and `server = ["std", "dep:futures"]`; now `client = ["dep:futures-util"]` and `server = ["dep:futures-util"]`. The `std` feature moved to `client-tokio` / `server-tokio`, which is where it belongs (the tokio backends genuinely require std). Bare-metal trait-surface consumers (`features = ["client", "bare_metal"]`) compile in pure no_std now. `server` still pulls `extern crate alloc` because `Server` holds `Arc` and `EventPublisher` holds `Arc` — documented in `lib.rs`; refactor to `&'static` borrows is tracked in #115. - **Breaking: optional dep `futures` replaced with `futures-util`** — direct dependency on `futures-util` with features `["async-await", "async-await-macro"]`. The `futures` umbrella crate's `select!` macro re-export is gated on its `std` feature, which transitively pulls `slab` / `memchr` / `futures-io` and breaks no_std cross-compiles. `futures-util` provides `select_biased!`, `pin_mut!`, and `FutureExt` under just `async-await(-macro)`. - **Breaking: internal `select!` → `select_biased!`** — `Inner::run_future`, `socket_loop_future`, and `server::run` now poll their select arms top-first instead of pseudo-randomly. For these workloads the bias gives slightly better behavior (control messages, sends, and unicast recvs get priority over their lower-priority siblings) and there is no genuine starvation path because the higher-priority arms are sporadic. The change is observable only under contrived workloads where every arm is permanently ready simultaneously. - **Breaking: `PayloadWireFormat::offered_endpoints` / `service_instances` replaced by visitor-pattern methods** — see `for_each_offered_endpoint` / `for_each_service_instance` in "Added" above. Implementors of custom `PayloadWireFormat` types must override the visitors instead of the `Vec`-returning forms. The `Vec`-returning forms remain as default-implemented `cfg(feature = "std")` convenience wrappers, so std callers' code keeps compiling unchanged. @@ -81,13 +212,13 @@ - **Breaking: `server::Error::Io(std::io::Error)` now `cfg(feature = "std")`** — the variant is gated on `feature = "std"` because `std::io::Error` is itself std-only. No-std consumers receive transport failures via `Error::Transport(TransportError)` which carries the portable `IoErrorKind`. - **Breaking: misuse paths on `Server::announcement_loop` / `Server::run` return `Error::InvalidUsage(...)`** — previously these returned `Error::Io(std::io::Error::new(InvalidInput, ..))` with a formatted message. The new variant is no_std-friendly and carries a machine-readable `&'static str` tag (`"passive_server_announcement_loop"`, `"announcement_loop_already_started"`, `"passive_server_run"`); the diagnostic moves to `tracing::warn!`. - **Breaking: `server::SubscriptionManager::get_subscribers` now `cfg(feature = "std")`** — convenience accessor returning a heap `Vec`. Production code paths use `for_each_subscriber` (visitor) since 0.8.0; this accessor remains for std consumers' tests and ad-hoc tooling. No_std consumers must use `for_each_subscriber`. -- **Breaking: `server::ServiceInfo` / `server::EventGroupInfo` now `cfg(feature = "std")`** — both types' `pub` fields hold `Vec<...>`. Bare-metal consumers don't construct these types today; if the use case emerges, a future port will switch to `heapless::Vec`. `Subscriber` is unaffected and stays no_std. +- **Breaking: `server::ServiceInfo` / `server::EventGroupInfo` now `cfg(feature = "std")`** — both types' `pub` fields hold `Vec<...>`. Bare-metal consumers don't construct these types today; if the use case emerges, a port to `heapless::Vec` is tracked in #116. `Subscriber` is unaffected and stays no_std. - **Breaking: `E2ERegistry` API change** — backing storage migrated from `std::collections::HashMap` to `heapless::index_map::FnvIndexMap` (cap = `E2E_REGISTRY_CAP = 32`, exposed). `E2ERegistry::register` now returns `Result<(), E2ERegistryFull>`; replacing an already-registered key always succeeds, adding a new key past the cap returns `Err`. `E2ERegistry::new()` is now `const`. The module is no longer `cfg(feature = "std")` — `E2ERegistry` works in pure no_std. - **Breaking: `E2ERegistryHandle::register` trait method now returns `Result<(), E2ERegistryFull>`** — propagates the new typed overflow from `E2ERegistry::register` through every handle impl. Callers (`Client::register_e2e`, `Server::register_e2e`) lift the `Result` through to their public surface. - `client::Error::Transport` adopts `#[error(transparent)]` Display delegation (the previous wrapping with `{:?}` debug-formatted the inner `TransportError`); user-facing error strings are now stable. - Subscribe-NACK reason strings normalized to `snake_case` for log consistency: `wrong_service_id`, `wrong_instance_id`, `wrong_major_version`, `no_endpoint_in_options`, `subscribers_per_group_full`, `event_groups_full`. Wire format is unchanged (NACK is signalled by `TTL=0`). -### Fixed +### Fixed — 0.7.0 → 0.8.0 surface baseline - **`server::EventPublisher::publish_event` no longer silently sends UNPROTECTED payloads on E2E protect failure** — counter exhaustion / key-lookup races etc. now surface as `Err(Error::E2e(_))` rather than logging and falling through (which had been emitting an unprotected message claiming an E2E-protected channel). - **SD `Subscribe` with mismatched `major_version` is now NACKed** — previously an Ack would be returned and the subscription registered, leaving the application stack to silently mis-decode incompatible-version traffic. @@ -99,8 +230,8 @@ ### Notes - **Crate version bumped to 0.8.0** — reflects the breaking changes above. Downstream `Cargo.toml` snippets in `README.md` were updated accordingly. -- **Bare-metal compile gate is now literal.** `cargo build --target thumbv7em-none-eabihf --no-default-features --features client,server,bare_metal` succeeds; `client + bare_metal` is verified alloc-free (zero `__rust_alloc` references in the resulting rlib). CI runs this matrix on every PR. The cortex-m4f target is the closest no_std proxy mainline Rust supports — the project's actual production target (Infineon AURIX TriCore) requires HighTec's commercial Rust distribution because mainline Rust + LLVM don't have a TriCore backend; a future phase will swap or layer in a TriCore CI runner once that infrastructure is in place. See `bare_metal_plan_v3.md`. -- **Known limitation: `server` feature pulls `extern crate alloc`.** `Server` holds `Arc` and `EventPublisher` holds `Arc`; both require an allocator. Pure no_std-without-allocator consumers can use the `client` feature alone (alloc-free) but will need a global allocator for the server side. A refactor to `&'static` borrows is on the v3 phase 21+ backlog. +- **Bare-metal compile gate is now literal.** `cargo build --target thumbv7em-none-eabihf --no-default-features --features client,server,bare_metal` succeeds; `client + bare_metal` is verified alloc-free (zero `__rust_alloc` references in the resulting rlib). CI runs this matrix on every PR. The cortex-m4f target is the closest no_std proxy mainline Rust supports — the project's actual production target (Infineon AURIX TriCore) requires HighTec's commercial Rust distribution because mainline Rust + LLVM don't have a TriCore backend; a TriCore CI runner is tracked in #117. +- **Known limitation: `server` feature pulls `extern crate alloc`.** `Server` holds `Arc` and `EventPublisher` holds `Arc`; both require an allocator. Pure no_std-without-allocator consumers can use the `client` feature alone (alloc-free) but will need a global allocator for the server side. A refactor to `&'static` borrows is tracked in #115. ### Test runner diff --git a/Cargo.toml b/Cargo.toml index 08b957c1..e0a529ae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,7 +63,7 @@ tokio = { version = "1", default-features = false, features = [ "sync", "time", ], optional = true } -tracing = { version = "0.1", default-features = false } +tracing = { version = "0.1", default-features = false, optional = true } [dev-dependencies] # `critical-section/std` provides a host-platform impl so integration @@ -76,7 +76,15 @@ tracing-subscriber = "0.3" [features] default = ["std"] -std = ["embedded-io/std", "thiserror/std", "tracing/std", "_alloc"] +# `tracing` pulls in `tracing-core`, which (since 0.1.33) declares +# `extern crate alloc` unconditionally and therefore requires the +# `alloc` crate to be present in the target sysroot. Bare-metal +# targets that ship a `core`-only sysroot (e.g. the AURIX TriCore +# LLVM-IR proxy used by the halo build) cannot satisfy this. Gate +# tracing behind a feature so those builds can opt out; std users +# pick it up automatically through the `std` feature below. +tracing = ["dep:tracing"] +std = ["embedded-io/std", "thiserror/std", "tracing", "tracing/std", "_alloc"] # Feature split: `client` exposes the protocol/trait-surface client # (no tokio, no socket2); `client-tokio` layers the tokio + socket2 # convenience defaults on top. Consumers of the bare-metal trait surface diff --git a/examples/bare_metal_server/src/main.rs b/examples/bare_metal_server/src/main.rs index 1a5e46cc..65082bf2 100644 --- a/examples/bare_metal_server/src/main.rs +++ b/examples/bare_metal_server/src/main.rs @@ -249,10 +249,12 @@ async fn main() { let subs = StaticSubscriptionHandle::new(&SUBS_STORAGE); // service_id=0x1234, instance_id=1, bound to LOCALHOST:30490. - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30490, 0x1234, 1); + let config = ServerConfig::new(0x1234, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30490); - let server = - Server::::new_with_deps( + let (_server, _handles, run) = + Server::::new_with_deps( ServerDeps { factory, timer: MockTimer, @@ -265,14 +267,10 @@ async fn main() { .await .expect("Server::new_with_deps failed"); - // The announcement loop periodically multicasts SD OfferService - // entries so clients on the network can discover this service. - // It is Send + 'static and can be handed to any executor. - let announce_handle = tokio::spawn( - server - .announcement_loop() - .expect("non-passive server must have an announcement loop"), - ); + // The combined run-future drives both receive and announce. It + // is `'static` and can be handed to any executor (here tokio for + // the canary harness). + let announce_handle = tokio::spawn(run); // Yield twice: the announcement loop fires its first SD offer on the // first poll before the inter-announcement timer starts. diff --git a/examples/client_server/src/main.rs b/examples/client_server/src/main.rs index d873b79a..5da2c64c 100644 --- a/examples/client_server/src/main.rs +++ b/examples/client_server/src/main.rs @@ -119,25 +119,24 @@ async fn main() -> Result<(), Box> { major_version: 1, minor_version: 0, ttl: 3, - ..ServerConfig::new( - interface, - MY_SERVER_PORT, - MY_SERVER_SERVICE_ID, - MY_SERVER_INSTANCE_ID, - ) + ..ServerConfig::new(MY_SERVER_SERVICE_ID, MY_SERVER_INSTANCE_ID) + .with_interface(interface) + .with_local_port(MY_SERVER_PORT) }; - let mut server = Server::new(config).await?; + // Dispatcher topology — the client drives all SD traffic via + // its own `sd_announcements_loop`, so we suppress the server's + // own announcement arm with `with_announce(false)`. The single + // returned run-future drives only the receive loop. + let config = config.with_announce(false); + let (_server, handles, run) = Server::new(config).await?; info!("Server bound on port {MY_SERVER_PORT}"); - // NOTE: We intentionally do NOT spawn server.announcement_loop(). - // The client's sd_announcements_loop handles all SD traffic. - - let _publisher = server.publisher(); + let _publisher = handles.publisher; // Spawn the server event loop (handles incoming subscriptions). let _server_handle = tokio::spawn(async move { - if let Err(e) = server.run().await { + if let Err(e) = run.await { error!("Server error: {e}"); } }); diff --git a/examples/embassy_net_client/src/main.rs b/examples/embassy_net_client/src/main.rs index a37b752f..fbf27263 100644 --- a/examples/embassy_net_client/src/main.rs +++ b/examples/embassy_net_client/src/main.rs @@ -269,22 +269,26 @@ type SubKey = (u16, u16, u16, SocketAddrV4); struct InMemorySubscriptions(Arc>>); impl SubscriptionHandle for InMemorySubscriptions { + type SubscribeFuture<'a> = + core::pin::Pin> + 'a>>; + type UnsubscribeFuture<'a> = core::pin::Pin + 'a>>; + fn subscribe( &self, service_id: u16, instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future> + '_ { + ) -> Self::SubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut g = this.lock().unwrap(); let k = (service_id, instance_id, event_group_id, subscriber_addr); if !g.contains(&k) { g.push(k); } Ok(()) - } + }) } fn unsubscribe( @@ -293,12 +297,12 @@ impl SubscriptionHandle for InMemorySubscriptions { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future + '_ { + ) -> Self::UnsubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut g = this.lock().unwrap(); g.retain(|e| *e != (service_id, instance_id, event_group_id, subscriber_addr)); - } + }) } fn for_each_subscriber<'a, F>( @@ -363,7 +367,7 @@ async fn main() { Box::leak(Box::new(SocketPool::new())); let server_factory = EmbassyNetFactory::new(stack_a, server_pool); let server_e2e: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); - let server_config = ServerConfig::new(IP_A, 30500, SERVICE_ID, INSTANCE_ID); + let server_config = ServerConfig::new(SERVICE_ID, INSTANCE_ID).with_interface(IP_A).with_local_port(30500); let server_deps = ServerDeps { factory: server_factory, @@ -372,24 +376,28 @@ async fn main() { subscriptions: InMemorySubscriptions::default(), }; - // Phase 19f: default `H = Arc`. Annotation - // is explicit because type inference can't chase H - // across the `ServerDeps` indirection. - let server: Server<_, _, _, _, Arc> = - Server::new_with_deps(server_deps, server_config, false) - .await - .expect("server construction over embassy-net"); - - // `_local` because `EmbassyNetSocket: !Sync` (it borrows - // from `Stack`'s `RefCell`-bearing - // internals); the Send-bounded `announcement_loop` - // doesn't typecheck for our `H`. - let announce_fut = server - .announcement_loop_local() - .expect("announcement_loop_local"); - tokio::task::spawn_local(announce_fut); + // Default `H = Arc`. Annotation is explicit + // because type inference can't chase H across the + // `ServerDeps` indirection. We use `run_with_buffers` + // instead of the alloc-backed `run` returned from the + // constructor because `EmbassyNetSocket: !Sync` makes + // the `run`-future `!Send`; ignoring it and re-building + // via `run_with_buffers` keeps us on the `spawn_local` + // path. + let (server, _handles, _run): ( + Server<_, _, _, _, Arc>, + _, + _, + ) = Server::new_with_deps(server_deps, server_config, false) + .await + .expect("server construction over embassy-net"); + + tokio::task::spawn_local(server.run_with_buffers( + Box::leak(Box::new([0u8; 65535])), + Box::leak(Box::new([0u8; 65535])), + )); println!( - "[server] announcement loop spawned, emitting OfferService(0x{SERVICE_ID:04X}) every 1s" + "[server] run loop spawned, emitting OfferService(0x{SERVICE_ID:04X}) every 1s" ); // ── Client on stack B ──────────────────────────────── diff --git a/simple-someip-embassy-net/Cargo.toml b/simple-someip-embassy-net/Cargo.toml index a208c98f..6d9fb8a6 100644 --- a/simple-someip-embassy-net/Cargo.toml +++ b/simple-someip-embassy-net/Cargo.toml @@ -17,7 +17,6 @@ readme = "README.md" # Sized for: bare-metal Rust embedded targets running embassy-net + # embassy-executor (cortex-m, RISC-V). Does not require alloc. # -# See `bare_metal_plan_v3.md` for the surrounding plan (phase 19). [dependencies] simple-someip = { path = "..", version = "0.8", default-features = false, features = [ diff --git a/simple-someip-embassy-net/README.md b/simple-someip-embassy-net/README.md index ace85691..2f9a05b5 100644 --- a/simple-someip-embassy-net/README.md +++ b/simple-someip-embassy-net/README.md @@ -11,12 +11,11 @@ add, without writing their own transport adapter. ## Status -Phase 19 of the [bare-metal roadmap][plan-v3]. As of phase 19a, this -crate is a scaffolded skeleton; the full `TransportFactory` / -`TransportSocket` impl lands incrementally in 19b–19c, with a host -loopback integration test in 19e and an in-tree example in 19f. +Reference adapter implementing the full `TransportFactory` / +`TransportSocket` surface, with a host loopback integration test and +an in-tree example. -## Quick sketch (target shape, post-19c) +## Quick sketch ```rust,ignore use simple_someip::{Client, ClientDeps}; @@ -51,4 +50,3 @@ MIT OR Apache-2.0, matching `simple-someip`. [embassy-net]: https://crates.io/crates/embassy-net [embassy-executor]: https://crates.io/crates/embassy-executor [`simple-someip`]: https://crates.io/crates/simple-someip -[plan-v3]: https://github.com/luminartech/simple_someip diff --git a/simple-someip-embassy-net/src/lib.rs b/simple-someip-embassy-net/src/lib.rs index 441fdb83..5eb26e26 100644 --- a/simple-someip-embassy-net/src/lib.rs +++ b/simple-someip-embassy-net/src/lib.rs @@ -9,21 +9,14 @@ //! //! # Why this crate exists //! -//! Phase 18 of the bare-metal effort closed the literal compile gate: -//! `simple-someip` + `client,server,bare_metal` cross-compiles for -//! `thumbv7em-none-eabihf`. But "compiles" is not "works" — until a -//! real backend satisfies the trait surface against an actual `no_std` -//! network stack, the trait surface is unverified. This crate is the -//! verification: an end-to-end working backend that bare-metal Rust -//! consumers can either depend on directly or treat as the worked -//! example for their own (lwIP, smoltcp-direct, vendor-stack) adapters. -//! -//! # Status -//! -//! Phase 19 in progress (per `bare_metal_plan_v3.md`). 19a (this -//! commit) is the scaffold; 19b implements [`EmbassyNetFactory`], -//! 19c implements [`EmbassyNetSocket`], 19e wires up the loopback -//! integration test, 19f produces an in-tree example. +//! `simple-someip` with `client,server,bare_metal` cross-compiles for +//! `thumbv7em-none-eabihf` — the literal compile gate is closed. But +//! "compiles" is not "works": until a real backend satisfies the +//! trait surface against an actual `no_std` network stack, that trait +//! surface is unverified. This crate is the verification — an +//! end-to-end working backend that bare-metal Rust consumers can +//! either depend on directly or treat as the worked example for +//! their own (lwIP, smoltcp-direct, vendor-stack) adapters. //! //! # Pairing with `simple-someip` //! diff --git a/simple-someip-embassy-net/src/socket.rs b/simple-someip-embassy-net/src/socket.rs index 27dd11b8..b0c19a4b 100644 --- a/simple-someip-embassy-net/src/socket.rs +++ b/simple-someip-embassy-net/src/socket.rs @@ -1,8 +1,8 @@ //! `TransportSocket` impl wrapping `embassy_net::udp::UdpSocket`. //! -//! Phase 19c lands the real send/recv I/O — named future structs -//! drive `embassy_net`'s `poll_send_to` / `poll_recv_from` directly, -//! so each datagram costs zero heap allocations on the hot path. +//! Named future structs drive `embassy_net`'s `poll_send_to` / +//! `poll_recv_from` directly, so each datagram costs zero heap +//! allocations on the hot path. use core::future::Future; use core::net::{Ipv4Addr, SocketAddrV4}; diff --git a/simple-someip-embassy-net/tests/loopback.rs b/simple-someip-embassy-net/tests/loopback.rs index a6a491e1..f9fb2eec 100644 --- a/simple-someip-embassy-net/tests/loopback.rs +++ b/simple-someip-embassy-net/tests/loopback.rs @@ -1,39 +1,37 @@ -//! Phases 19e + 19g — Loopback integration tests. +//! Loopback integration tests. //! //! Two `embassy_net::Stack` instances bridged by an in-memory //! `LoopbackDriver` pair (no kernel TUN device, no privileges -//! required). Validates the `simple-someip-embassy-net` adapter -//! (Phases 19a–c) and the `Server` `SocketHandle` abstraction -//! (Phase 19f) against a real `embassy_net::Stack`: +//! required). Validates the `simple-someip-embassy-net` adapter and +//! the `Server` `SocketHandle` abstraction against a real +//! `embassy_net::Stack`: //! -//! * **`adapter_udp_roundtrip`** (19e) — bind two -//! `EmbassyNetSocket`s, one per stack, send a UDP datagram -//! from A to B, assert byte-equality + source-address. -//! Tightest test of `bind` / `send_to` / `recv_from` / -//! `local_addr` end-to-end. -//! * **`client_receives_server_sd_announcement`** (19g) — wire -//! a real `simple_someip::Server` on stack A with -//! `announcement_loop_local` (the `!Send` variant added in -//! 19f) and a real `simple_someip::Client` on stack B with -//! `Client::new_with_deps_local`. Assert the SD multicast -//! `OfferService` propagates through the loopback and reaches -//! the Client's update stream. -//! * **`client_send_request_server_runloop_stable`** (19g) — -//! passive Server on stack A, Client on stack B drives -//! `add_endpoint` + `send_to_service` to push a SOME/IP -//! request through the embassy-net loopback. Asserts the -//! request serializes, transits, and lands on the Server's -//! run-loop without panicking. (No response assertion — -//! `simple_someip::Server` exposes no public request-handler -//! API, matching the parent-crate reference test.) +//! * **`adapter_udp_roundtrip`** — bind two `EmbassyNetSocket`s, +//! one per stack, send a UDP datagram from A to B, assert +//! byte-equality + source-address. Tightest test of `bind` / +//! `send_to` / `recv_from` / `local_addr` end-to-end. +//! * **`client_receives_server_sd_announcement`** — wire a real +//! `simple_someip::Server` on stack A with `run_with_buffers` +//! (the `!Send` path) and a real `simple_someip::Client` on +//! stack B with `Client::new_with_deps_local`. Assert the SD +//! multicast `OfferService` propagates through the loopback and +//! reaches the Client's update stream. +//! * **`client_send_request_server_runloop_stable`** — passive +//! Server on stack A, Client on stack B drives `add_endpoint` + +//! `send_to_service` to push a SOME/IP request through the +//! embassy-net loopback. Asserts the request serializes, +//! transits, and lands on the Server's run-loop without +//! panicking. (No response assertion — `simple_someip::Server` +//! exposes no public request-handler API, matching the +//! parent-crate reference test.) //! //! Runtime: `#[tokio::test(flavor = "current_thread")]` plus a //! `LocalSet` driving the per-stack `spawn_local` runners. //! `Stack` is `!Sync` (RefCell internals), so -//! `Stack::run()` is `!Send` — multi-threaded `tokio::spawn` -//! does not type-check. The same constraint propagates through -//! `EmbassyNetSocket` and forces the `_local` Client + -//! `announcement_loop_local` Server paths. +//! `Stack::run()` is `!Send` — multi-threaded `tokio::spawn` does +//! not type-check. The same constraint propagates through +//! `EmbassyNetSocket` and forces the `_local` Client paths plus +//! `Server::run_with_buffers` (no `Send` bound). use core::net::{Ipv4Addr, SocketAddrV4}; use core::task::{Context, Waker}; @@ -240,9 +238,8 @@ fn build_stack(driver: LoopbackDriver, ip: Ipv4Addr, seed: u64) -> &'static Stac // single-threaded test runtime: `#[tokio::test(flavor = // "current_thread")]` plus a `LocalSet` that drives the per-stack // `spawn_local` runners. The same constraint forces the SOME/IP -// integration to use `Client::new_with_deps_local` (matching the -// `LocalSpawner` trait shipped in phase 17 specifically for -// !Send-bound transports). +// integration to use `Client::new_with_deps_local` (the +// `LocalSpawner`-trait counterpart for !Send-bound transports). const IP_A: Ipv4Addr = Ipv4Addr::new(169, 254, 1, 1); const IP_B: Ipv4Addr = Ipv4Addr::new(169, 254, 1, 2); @@ -376,14 +373,13 @@ async fn factory_bind_accepts_wildcard_ip() { .await; } -// ── SOME/IP Client+Server harness (phase 19g) ─────────────────────── +// ── SOME/IP Client+Server harness ─────────────────────────────────── // // Adds a real `simple_someip::Client` + `simple_someip::Server` on // top of the two-stack loopback, exercising the bare-metal -// constructors over `EmbassyNetFactory`. Phase 19f's `SocketHandle` +// constructors over `EmbassyNetFactory`. The `SocketHandle` // abstraction lets `Server` accept `Arc` as its -// `H` parameter even though `EmbassyNetSocket` is `!Sync`; without -// that work the bounds at the impl-block level rejected the type. +// `H` parameter even though `EmbassyNetSocket` is `!Sync`. // // Both tests run on `flavor = "current_thread"` + `LocalSet` because: // - `Stack` is `!Sync` (RefCell internals), so @@ -472,22 +468,31 @@ type SubKey = (u16, u16, u16, SocketAddrV4); struct MockSubscriptions(Arc>>); impl SubscriptionHandle for MockSubscriptions { + // Boxed `!Send` futures — the `spawn_local` paths that exercise + // this loopback don't need `Send` and the `Mutex` is only used + // synchronously inside. + type SubscribeFuture<'a> = core::pin::Pin< + Box> + 'a>, + >; + type UnsubscribeFuture<'a> = + core::pin::Pin + 'a>>; + fn subscribe( &self, service_id: u16, instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl core::future::Future> + '_ { + ) -> Self::SubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut guard = this.lock().unwrap(); let key = (service_id, instance_id, event_group_id, subscriber_addr); if !guard.contains(&key) { guard.push(key); } Ok(()) - } + }) } fn unsubscribe( @@ -496,12 +501,12 @@ impl SubscriptionHandle for MockSubscriptions { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl core::future::Future + '_ { + ) -> Self::UnsubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut guard = this.lock().unwrap(); guard.retain(|e| *e != (service_id, instance_id, event_group_id, subscriber_addr)); - } + }) } fn for_each_subscriber<'a, F>( @@ -593,7 +598,9 @@ async fn client_receives_server_sd_announcement() { let server_subs = MockSubscriptions::default(); // Service id 0x5BAA (just a witness) at port 30500 on // stack A's interface IP. - let server_config = ServerConfig::new(IP_A, 30500, 0x5BAA, 1); + let server_config = ServerConfig::new(0x5BAA, 1) + .with_interface(IP_A) + .with_local_port(30500); let server_deps = ServerDeps { factory: server_factory, @@ -602,24 +609,29 @@ async fn client_receives_server_sd_announcement() { subscriptions: server_subs, }; - // Default `H = Arc` (Phase 19f) — `Arc: + // Default `H = Arc`. `Arc: // WrappableSocketHandle` works for any `T: TransportSocket // + 'static`, so `Arc` (which is // `!Sync`) compiles here. The annotation is explicit so // type inference doesn't have to chase `H` across the // deps-bundle indirection. - let server: Server<_, _, _, _, Arc> = - Server::new_with_deps(server_deps, server_config, false) - .await - .expect("server construction over embassy-net"); - - // `announcement_loop_local`, NOT `announcement_loop`, - // because `EmbassyNetSocket` is `!Sync` — the - // Send-bounded variant doesn't typecheck for our `H`. - let announce_fut = server - .announcement_loop_local() - .expect("announcement_loop_local"); - tokio::task::spawn_local(announce_fut); + let (server, _handles, _run): ( + Server<_, _, _, _, Arc>, + _, + _, + ) = Server::new_with_deps(server_deps, server_config, false) + .await + .expect("server construction over embassy-net"); + + // Receive + announce share the combined run-future. The + // constructor's `_run` is the alloc-backed version; we + // use `run_with_buffers` here because + // `EmbassyNetSocket: !Sync` makes the `_run` future + // `!Send` and we want explicit static buffers anyway. + tokio::task::spawn_local(server.run_with_buffers( + Box::leak(Box::new([0u8; 65535])), + Box::leak(Box::new([0u8; 65535])), + )); // ── Client on stack B ──────────────────────────────── let client_pool: &'static SocketPool<8, LINK_MTU, LINK_MTU> = @@ -709,7 +721,9 @@ async fn client_send_request_server_runloop_stable() { let service_id = 0x5BBB_u16; let instance_id = 1_u16; let server_port = 30600_u16; - let server_config = ServerConfig::new(IP_A, server_port, service_id, instance_id); + let server_config = ServerConfig::new(service_id, instance_id) + .with_interface(IP_A) + .with_local_port(server_port); let server_deps = ServerDeps { factory: server_factory, @@ -722,10 +736,13 @@ async fn client_send_request_server_runloop_stable() { // doesn't have to invent it across the deps-bundle // indirection. Same shape as the equivalent annotation // in `simple_someip`'s SD-NACK test. - let mut server: Server<_, _, _, _, Arc> = - Server::new_passive_with_deps(server_deps, server_config) - .await - .expect("passive server construction"); + let (server, _handles, _run): ( + Server<_, _, _, _, Arc>, + _, + _, + ) = Server::new_passive_with_deps(server_deps, server_config) + .await + .expect("passive server construction"); // NOTE: we do NOT spawn `server.run()` here. A passive // server's `run()` returns `Err(InvalidUsage)` @@ -735,7 +752,7 @@ async fn client_send_request_server_runloop_stable() { // so its unicast socket bind happens — the kernel-level // recv buffer absorbs the client's request bytes // independently of any application run-loop. - let _ = &mut server; // suppress unused-mut warning + let _ = &server; // anchor binding so the unicast bind sticks // ── Client on stack B ──────────────────────────────── let client_pool: &'static SocketPool<8, LINK_MTU, LINK_MTU> = diff --git a/src/client/inner.rs b/src/client/inner.rs index 4c7f3d34..f4b32d76 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -5,7 +5,7 @@ use futures_util::{FutureExt, pin_mut, select_biased}; use heapless::{Deque, index_map::FnvIndexMap}; #[cfg(all(test, feature = "client-tokio"))] use std::sync::{Arc, Mutex}; -use tracing::{debug, error, info, trace, warn}; +use crate::log::{debug, error, info, trace, warn}; #[cfg(all(test, feature = "client-tokio"))] use crate::e2e::E2ERegistry; @@ -637,7 +637,7 @@ where } for port in &dead_ports { unicast_sockets.remove(port); - tracing::warn!("Unicast socket on port {port} closed; evicted from registry"); + crate::log::warn!("Unicast socket on port {port} closed; evicted from registry"); } if let Some(msg) = delivered { Poll::Ready(msg) diff --git a/src/client/mod.rs b/src/client/mod.rs index 4aa28f45..71815258 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -62,29 +62,82 @@ use core::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use inner::Inner; #[cfg(feature = "client-tokio")] use std::sync::{Arc, Mutex, RwLock}; -use tracing::info; - -// Bound bundle the client's internals demand from any -// `C: ChannelFactory` they channel through. Stable Rust does not -// elaborate where-clause bounds on a trait alias, and macros do not -// expand inside `where` clauses, so the bundle is repeated inline at -// each impl block that constructs channels. The list is authored once -// here as documentation and copy-pasted; mismatch surfaces as a -// trait-bound compile error pointing at the missing `OneshotPooled` / -// `BoundedPooled` / `UnboundedPooled` impl. -// -// ```ignore -// Result<(), Error>: OneshotPooled, -// Result: OneshotPooled, -// Result: OneshotPooled, -// ControlMessage: BoundedPooled, -// SendMessage: BoundedPooled, -// Result, Error>: BoundedPooled, -// ClientUpdate

: UnboundedPooled, -// ``` -// -// When stable Rust gains implied bounds for trait where-clauses, this -// collapses back to a single `C: ClientChannels

` supertrait. +use crate::log::info; + +/// Marker trait declaring the channel-pool entries a [`ChannelFactory`] +/// must declare for [`Client`] to compile against it. End users do not +/// implement this trait directly: it has a blanket impl over any +/// [`ChannelFactory`] for which all seven required `OneshotPooled` / +/// `BoundedPooled` / `UnboundedPooled` entries exist. +/// +/// # Required entries +/// +/// For a payload type `P: PayloadWireFormat + 'static`, the +/// [`define_static_channels!`] invocation must declare: +/// +/// | Pool kind | Item type | Cardinality | +/// |---|---|---| +/// | `oneshot` | `Result<(), client::Error>` | per-pool default | +/// | `oneshot` | `Result` | per-pool default | +/// | `oneshot` | `Result` | per-pool default | +/// | `bounded` | `(ControlMessage, 4)` | per-pool default | +/// | `bounded` | `(SendMessage, 16)` | per-pool default | +/// | `bounded` | `(Result, client::Error>, 16)` | per-pool default | +/// | `unbounded` | `ClientUpdate

` | per-pool default | +/// +/// where `C` is the channel-factory type generated by +/// [`define_static_channels!`]. `bare_metal` consumers will typically +/// look at the `examples/bare_metal_client/` example for a copy-pasteable +/// invocation matching this list. +/// +/// # Status +/// +/// Today this trait is **discoverability-only**: stable Rust does not +/// elaborate where-clause bounds on a trait, so a generic function +/// taking `C: ClientChannelTypes

` cannot use that bound to satisfy +/// the seven underlying `OneshotPooled` / `BoundedPooled` / +/// `UnboundedPooled` constraints. Each `impl<…> Client<…>` block +/// repeats the bounds inline, and downstream witness functions would +/// have to do the same. +/// +/// In practical terms: the trait surfaces the required pool entries +/// in one rustdoc page (this one), reachable as +/// [`crate::client::ClientChannelTypes`]. It is intentionally not +/// re-exported at crate root — making it generic-position-named would +/// tempt callers to write `C: ClientChannelTypes

` and hit Rust's +/// unsolved trait-bound elaboration limit at the wrong call site +/// (the bounds you see below in the `where` clause are what +/// implementors actually have to satisfy). When stable Rust gains +/// elaboration for these bounds, the per-impl repetition can +/// collapse to a single `C: ClientChannelTypes

` supertrait without +/// changing the outward contract. +/// +/// [`define_static_channels!`]: crate::define_static_channels +pub trait ClientChannelTypes: ChannelFactory +where + Result<(), Error>: OneshotPooled, + Result: OneshotPooled, + Result: OneshotPooled, + ControlMessage: BoundedPooled, + SendMessage: BoundedPooled, + Result, Error>: BoundedPooled, + ClientUpdate

: UnboundedPooled, +{ +} + +impl ClientChannelTypes

for C +where + P: PayloadWireFormat + 'static, + C: ChannelFactory, + Result<(), Error>: OneshotPooled, + Result: OneshotPooled, + Result: OneshotPooled, + ControlMessage: BoundedPooled, + SendMessage: BoundedPooled, + Result, Error>: BoundedPooled, + ClientUpdate

: UnboundedPooled, +{ +} /// Handle to a pending SOME/IP request-response transaction. /// Resolves when the inner loop receives a matching unicast reply. @@ -208,15 +261,21 @@ impl } /// Bundle of dependencies passed to [`Client::new_with_deps`]. Bundling -/// the five pluggable infrastructure types (`TransportFactory`, -/// `Spawner`, `Timer`, `E2ERegistryHandle`, `InterfaceHandle`) into a -/// single struct keeps the constructor's argument list manageable -/// (consumers see one named field per dependency rather than positional -/// args six deep). +/// the five pluggable infrastructure types (`TransportFactory`, `Timer`, +/// `E2ERegistryHandle`, `InterfaceHandle`, `Spawner`) into a single +/// struct keeps the constructor's argument list manageable (consumers +/// see one named field per dependency rather than positional args six +/// deep). +/// +/// Generic order mirrors [`crate::server::ServerDeps`] for the shared +/// infrastructure (`F`, `Tm`, `R`), then side-specific dependencies +/// (`I` for the client's interface handle, `Sub` for the server's +/// subscription handle), then any side-only extras (`Sp` for the +/// client's spawner — the server has no internal task-spawning). /// /// All five fields are public so callers can construct the struct /// inline; there's no builder ceremony beyond the field assignments. -pub struct ClientDeps +pub struct ClientDeps where F: TransportFactory, Tm: Timer, @@ -225,8 +284,6 @@ where { /// Transport factory used by `bind_*` to construct sockets. pub factory: F, - /// Task-spawner used by `bind_*` to drive per-socket I/O loops. - pub spawner: S, /// Async sleep primitive used by the run-loop's idle tick. pub timer: Tm, /// Shared E2E registry handle for runtime E2E configuration. @@ -234,6 +291,170 @@ where /// Shared interface-address handle. The run-loop reads its current /// value when `bind_*` is invoked. pub interface: I, + /// Task-spawner used by `bind_*` to drive per-socket I/O loops. + pub spawner: Sp, +} + +/// Tokio-defaulted constructor. +/// +/// Available under the `client-tokio` feature. Returns a `ClientDeps` +/// pre-populated with `TokioTransport` / `TokioTimer` / `TokioSpawner` +/// and a fresh `Arc>` / `Arc>`. +/// Combine with the [`ClientDeps::with_factory`] / [`ClientDeps::with_timer`] +/// / [`ClientDeps::with_e2e_registry`] / [`ClientDeps::with_interface`] +/// / [`ClientDeps::with_spawner`] builders to override individual +/// fields without spelling out the rest by hand. +/// +/// ```no_run +/// # #[cfg(feature = "client-tokio")] +/// # fn demo() { +/// use simple_someip::{Client, ClientDeps, RawPayload, TokioChannels}; +/// use std::net::Ipv4Addr; +/// let deps = ClientDeps::tokio(Ipv4Addr::LOCALHOST); +/// let (_client, _updates, _run) = +/// Client::::new_with_deps(deps, false); +/// # } +/// ``` +#[cfg(feature = "client-tokio")] +impl + ClientDeps< + crate::tokio_transport::TokioTransport, + TokioTimer, + Arc>, + Arc>, + TokioSpawner, + > +{ + /// Build a `ClientDeps` with the tokio defaults. + #[must_use] + pub fn tokio(interface: Ipv4Addr) -> Self { + Self { + factory: crate::tokio_transport::TokioTransport, + timer: TokioTimer, + e2e_registry: Arc::new(Mutex::new(E2ERegistry::new())), + interface: Arc::new(RwLock::new(interface)), + spawner: TokioSpawner, + } + } +} + +/// Field-by-field fluent builder. Each `with_*` returns a new +/// `ClientDeps` with that single field replaced (and its corresponding +/// generic parameter updated). Lets callers start from +/// [`ClientDeps::tokio`] and override individual fields without +/// spelling out the full struct literal. +/// +/// ```no_run +/// # #[cfg(feature = "client-tokio")] +/// # fn demo() { +/// # use simple_someip::{ClientDeps, Spawner}; +/// # use std::net::Ipv4Addr; +/// # struct MySpawner; +/// # impl Spawner for MySpawner { +/// # fn spawn(&self, _: impl core::future::Future + Send + 'static) {} +/// # } +/// let deps = ClientDeps::tokio(Ipv4Addr::LOCALHOST) +/// .with_spawner(MySpawner); +/// # let _ = deps; +/// # } +/// ``` +impl ClientDeps +where + F: TransportFactory, + Tm: Timer, + R: E2ERegistryHandle, + I: InterfaceHandle, +{ + /// Replace the `factory` field, returning a `ClientDeps` over the + /// new factory type. + pub fn with_factory(self, factory: F2) -> ClientDeps { + ClientDeps { + factory, + timer: self.timer, + e2e_registry: self.e2e_registry, + interface: self.interface, + spawner: self.spawner, + } + } + + /// Replace the `timer` field, returning a `ClientDeps` over the new + /// timer type. + pub fn with_timer(self, timer: Tm2) -> ClientDeps { + ClientDeps { + factory: self.factory, + timer, + e2e_registry: self.e2e_registry, + interface: self.interface, + spawner: self.spawner, + } + } + + /// Replace the `e2e_registry` field, returning a `ClientDeps` over + /// the new registry-handle type. + pub fn with_e2e_registry( + self, + e2e_registry: R2, + ) -> ClientDeps { + ClientDeps { + factory: self.factory, + timer: self.timer, + e2e_registry, + interface: self.interface, + spawner: self.spawner, + } + } + + /// Replace the `interface` field, returning a `ClientDeps` over the + /// new interface-handle type. + pub fn with_interface( + self, + interface: I2, + ) -> ClientDeps { + ClientDeps { + factory: self.factory, + timer: self.timer, + e2e_registry: self.e2e_registry, + interface, + spawner: self.spawner, + } + } + + /// Replace the `spawner` field with a `Send + Sync` spawner + /// suitable for [`Client::new_with_deps`]. + /// + /// For single-threaded executors that ship `!Send` futures, use + /// [`Self::with_local_spawner`] instead — the eventual + /// [`Client::new_with_deps_local`] expects a `LocalSpawner` and + /// the bound is enforced here at the builder call site rather + /// than deferred to construction. + pub fn with_spawner(self, spawner: Sp2) -> ClientDeps { + ClientDeps { + factory: self.factory, + timer: self.timer, + e2e_registry: self.e2e_registry, + interface: self.interface, + spawner, + } + } + + /// Replace the `spawner` field with a [`LocalSpawner`] for use + /// with [`Client::new_with_deps_local`] (single-threaded + /// executors such as `tokio::task::LocalSet`, + /// `embassy-executor`, or hand-rolled poll loops). + /// + /// [`LocalSpawner`]: crate::transport::LocalSpawner + pub fn with_local_spawner( + self, + spawner: Sp2, + ) -> ClientDeps { + ClientDeps { + factory: self.factory, + timer: self.timer, + e2e_registry: self.e2e_registry, + interface: self.interface, + spawner, + } + } } /// A SOME/IP client that handles service discovery and message exchange. @@ -248,6 +469,19 @@ where /// (`Arc>` and `Arc>`) are used by the /// standard constructors `Self::new` / `Self::new_with_loopback` / /// `Self::new_with_spawner_and_loopback` (all under `client-tokio`). +/// +/// # Note on generic-parameter alignment with [`crate::ServerDeps`] +/// +/// [`ClientDeps`] and [`crate::ServerDeps`] share their first three +/// generic positions (`F`, `Tm`, `R`) to read symmetrically, but the +/// `Client` struct itself carries only `` +/// — `F`, `Tm`, and `Sp` (Spawner) live on the run-loop future +/// produced by [`Self::new_with_deps`], not on the handle. The +/// asymmetry between `Client<…>` and `Server` is +/// structural, not an oversight: a `Client` value retains no reference +/// to the transport / timer / spawner once construction is done, +/// whereas a `Server` value does (factory + timer fields are stored +/// for the announcement loop and any rebind operations). #[derive(Clone)] pub struct Client< MessageDefinitions: PayloadWireFormat + Send + 'static, @@ -381,7 +615,7 @@ where /// /// # Bounds /// - /// `S: Spawner + Send + Sync + 'static` — the spawner is stored in + /// `Sp: Spawner + Send + Sync + 'static` — the spawner is stored in /// the run-loop future, which is `Send + 'static`, so the spawner /// must match those bounds. `Sync` is required because `&self.spawner` /// is held across `.await` points inside @@ -389,25 +623,25 @@ where /// `bind_discovery_seeded_with_transport`, both of which execute on /// the driven run-loop task (not on the user's call site). #[must_use = "the returned run-loop future must be spawned (e.g. via the Spawner) for the client to make progress"] - pub fn new_with_spawner_and_loopback( + pub fn new_with_spawner_and_loopback( interface: Ipv4Addr, multicast_loopback: bool, - spawner: S, + spawner: Sp, ) -> ( Self, ClientUpdates, impl core::future::Future + Send + 'static, ) where - S: Spawner + Send + Sync + 'static, + Sp: Spawner + Send + Sync + 'static, { Self::new_with_deps( ClientDeps { factory: crate::tokio_transport::TokioTransport, - spawner, timer: TokioTimer, e2e_registry: Arc::new(Mutex::new(E2ERegistry::new())), interface: Arc::new(RwLock::new(interface)), + spawner, }, multicast_loopback, ) @@ -457,8 +691,8 @@ where /// `LocalSet`-style spawner shim. #[allow(clippy::type_complexity)] #[must_use = "the returned run-loop future must be spawned (e.g. via the Spawner) for the client to make progress"] - pub fn new_with_deps( - deps: ClientDeps, + pub fn new_with_deps( + deps: ClientDeps, multicast_loopback: bool, ) -> ( Self, @@ -471,21 +705,21 @@ where for<'a> F::BindFuture<'a>: Send, for<'a> ::SendFuture<'a>: Send, for<'a> ::RecvFuture<'a>: Send, - S: Spawner + Send + Sync + 'static, + Sp: Spawner + Send + Sync + 'static, Tm: Timer + Send + Sync + 'static, for<'a> Tm::SleepFuture<'a>: Send, { let ClientDeps { factory, - spawner, timer, e2e_registry, interface, + spawner, } = deps; let initial_addr = interface.get(); let dispatch = bind_dispatch::SpawnerDispatch { factory, spawner }; let (control_sender, update_receiver, run_future) = - Inner::>::build( + Inner::>::build( initial_addr, e2e_registry.clone(), multicast_loopback, @@ -518,8 +752,8 @@ where /// [`Spawner`]: crate::transport::Spawner #[allow(clippy::type_complexity)] #[must_use = "the returned run-loop future must be spawned (e.g. via the LocalSpawner) for the client to make progress"] - pub fn new_with_deps_local( - deps: ClientDeps, + pub fn new_with_deps_local( + deps: ClientDeps, multicast_loopback: bool, ) -> ( Self, @@ -529,15 +763,15 @@ where where F: TransportFactory + 'static, F::Socket: 'static, - S: crate::transport::LocalSpawner + 'static, + Sp: crate::transport::LocalSpawner + 'static, Tm: Timer + 'static, { let ClientDeps { factory, - spawner, timer, e2e_registry, interface, + spawner, } = deps; let initial_addr = interface.get(); let dispatch = bind_dispatch::LocalSpawnerDispatch { factory, spawner }; @@ -546,7 +780,7 @@ where Tm, R, C, - bind_dispatch::LocalSpawnerDispatch, + bind_dispatch::LocalSpawnerDispatch, >::build( initial_addr, e2e_registry.clone(), @@ -1065,26 +1299,26 @@ where let (flag_rx, flag_msg) = ControlMessage::::query_reboot_flag(); let Some(sender) = weak_sender.upgrade() else { - tracing::info!("Client shut down, stopping SD announcements"); + crate::log::info!("Client shut down, stopping SD announcements"); break; }; let enqueue_ok = sender.send(flag_msg).await.is_ok(); drop(sender); if !enqueue_ok { - tracing::warn!("SD announcement channel closed, stopping"); + crate::log::warn!("SD announcement channel closed, stopping"); break; } let reboot = match flag_rx.recv().await { Ok(Ok(flag)) => flag, Ok(Err(e)) => { - tracing::warn!( + crate::log::warn!( "SD announcement reboot-flag query returned error ({:?}), skipping tick", e ); continue; } Err(_) => { - tracing::warn!("SD announcement reboot-flag query dropped, stopping"); + crate::log::warn!("SD announcement reboot-flag query dropped, stopping"); break; } }; @@ -1095,14 +1329,14 @@ where ControlMessage::::send_sd(target, header); let Some(sender) = weak_sender.upgrade() else { - tracing::info!("Client shut down, stopping SD announcements"); + crate::log::info!("Client shut down, stopping SD announcements"); break; }; let send_ok = sender.send(message).await.is_ok(); drop(sender); if !send_ok { - tracing::warn!("SD announcement channel closed, stopping"); + crate::log::warn!("SD announcement channel closed, stopping"); break; } @@ -1110,16 +1344,16 @@ where Ok(Ok(())) => { count += 1; if count == 1 { - tracing::info!("Sent first client SD announcement"); + crate::log::info!("Sent first client SD announcement"); } else { - tracing::trace!("Sent {count} client SD announcements"); + crate::log::trace!("Sent {count} client SD announcements"); } } Ok(Err(e)) => { - tracing::error!("Failed to send SD announcement: {e:?}"); + crate::log::error!("Failed to send SD announcement: {e:?}"); } Err(_) => { - tracing::warn!("SD announcement response dropped, stopping"); + crate::log::warn!("SD announcement response dropped, stopping"); break; } } diff --git a/src/client/session.rs b/src/client/session.rs index 558ad069..4200270f 100644 --- a/src/client/session.rs +++ b/src/client/session.rs @@ -151,7 +151,7 @@ impl SessionTracker { // suppress further warnings so a saturated tracker does not // spam the log at the incoming-packet rate. if !self.saturation_warned { - tracing::warn!( + crate::log::warn!( "SessionTracker at capacity ({}); dropping new sender state for \ sender={} transport={:?} svc=0x{:04X} inst=0x{:04X}. Reboot \ detection disabled for this entry and any further new entries \ @@ -408,7 +408,7 @@ mod tests { #[test] fn capacity_overflow_warns_only_on_first_hit() { - // `saturation_warned` is the latch that guards the tracing::warn! + // `saturation_warned` is the latch that guards the crate::log::warn! // call in `check()`. It must flip false → true on the first // rejected insert and stay true for subsequent hits — otherwise // a saturated tracker spams the log at the packet rate. diff --git a/src/client/socket_manager.rs b/src/client/socket_manager.rs index 6764bcee..ef567c64 100644 --- a/src/client/socket_manager.rs +++ b/src/client/socket_manager.rs @@ -57,14 +57,14 @@ use core::{ task::{Context, Poll}, }; use futures_util::{FutureExt, pin_mut, select_biased}; -use tracing::{debug, error, info, trace, warn}; +use crate::log::{debug, error, info, trace, warn}; /// A received message together with the source address it came from. /// -/// TODO: narrow `source` to `SocketAddrV4` to match the `TransportSocket` -/// trait's IPv4-only contract — today the field is always a -/// `SocketAddr::V4(_)` wrapping, and the V6 variant is unreachable. -/// Deferred because the rename ripples through `DiscoveryMessage` and +/// Tracked in #118: narrow `source` to `SocketAddrV4` to match the +/// `TransportSocket` trait's IPv4-only contract — today the field is +/// always a `SocketAddr::V4(_)` wrapping, and the V6 variant is +/// unreachable. The rename ripples through `DiscoveryMessage` and /// `ClientUpdate::Unicast`. #[derive(Clone, Debug)] pub struct ReceivedMessage

{ diff --git a/src/e2e/crc.rs b/src/e2e/crc.rs index 91e60caa..a72854aa 100644 --- a/src/e2e/crc.rs +++ b/src/e2e/crc.rs @@ -41,7 +41,7 @@ pub fn compute_crc32_p4(length: u16, counter: u16, data_id: u32, payload: &[u8]) /// Note: CRC field itself is not included in the calculation. /// Note: `DataLength` is NOT included in the CRC calculation. pub fn compute_crc16_p5(data_id: u16, counter: u8, payload: &[u8]) -> u16 { - tracing::trace!( + crate::log::trace!( "CRC-16 Profile5: data_id=0x{:04X}, counter={}, payload_len={}, payload={:02X?}", data_id, counter, @@ -62,7 +62,7 @@ pub fn compute_crc16_p5(data_id: u16, counter: u8, payload: &[u8]) -> u16 { digest.update(&data_id_bytes); let crc = digest.finalize(); - tracing::trace!( + crate::log::trace!( "CRC-16 Profile5: computed CRC = 0x{:04X} (bytes: {:02X?})", crc, crc.to_le_bytes() @@ -83,7 +83,7 @@ pub fn compute_crc16_p5_with_header( payload: &[u8], upper_header: [u8; 8], ) -> u16 { - tracing::trace!( + crate::log::trace!( "CRC-16 Profile5 (with header): data_id=0x{:04X}, counter={}, payload_len={}, upper_header={:02X?}, payload={:02X?}", data_id, counter, @@ -99,7 +99,7 @@ pub fn compute_crc16_p5_with_header( digest.update(&data_id.to_le_bytes()); let crc = digest.finalize(); - tracing::trace!( + crate::log::trace!( "CRC-16 Profile5 (with header): computed CRC = 0x{:04X} (bytes: {:02X?})", crc, crc.to_le_bytes() diff --git a/src/e2e/e2e_checker.rs b/src/e2e/e2e_checker.rs index e8b73773..19212731 100644 --- a/src/e2e/e2e_checker.rs +++ b/src/e2e/e2e_checker.rs @@ -92,7 +92,7 @@ pub fn check_profile5<'a>( // Verify data length matches configuration (header + payload = config.data_length) let expected_total_length = PROFILE5_HEADER_SIZE + config.data_length as usize; if protected.len() != expected_total_length { - tracing::warn!( + crate::log::warn!( "E2E Profile 5 length mismatch: expected {} bytes (3 header + {} payload), got {} bytes", expected_total_length, config.data_length, @@ -155,7 +155,7 @@ pub fn check_profile5_with_header<'a>( let expected_total_length = PROFILE5_HEADER_SIZE + config.data_length as usize; if protected.len() != expected_total_length { - tracing::warn!( + crate::log::warn!( "E2E Profile 5 length mismatch: expected {} bytes (3 header + {} payload), got {} bytes", expected_total_length, config.data_length, diff --git a/src/lib.rs b/src/lib.rs index b5ce3193..0cff68d4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,9 +115,8 @@ extern crate std; // - `server` — `EventPublisher` and the `Server` struct hold // `Arc>` / `Arc` for sharing // between the run loop and external publishing tasks. A -// future refactor may switch to `&'static` borrows so the -// server compiles in pure no_std without an allocator; -// tracked in `bare_metal_plan_v3.md` Phase 21+ backlog. +// the `&'static`-borrow refactor tracked in #115 would let +// server compile in pure no_std without an allocator. // // The `static_channels` module (under `bare_metal` alone) does // NOT need alloc — users wanting `client` + `bare_metal` without @@ -161,6 +160,7 @@ pub const UDP_BUFFER_SIZE: usize = 1500; /// SOME/IP client for discovering services and exchanging messages. #[cfg(feature = "client")] pub mod client; +mod log; /// End-to-end (E2E) protection utilities for SOME/IP payloads. pub mod e2e; /// SOME/IP protocol primitives: headers, messages, return codes, and service discovery. @@ -215,9 +215,17 @@ pub use traits::{OfferedEndpoint, PayloadWireFormat, WireFormat}; pub use client::{ Client, ClientDeps, ClientUpdate, ClientUpdates, DiscoveryMessage, PendingResponse, }; +// `ClientChannelTypes`, `ControlMessage`, `SendMessage`, `ReceivedMessage` +// are intentionally NOT re-exported at crate root — they are +// implementation-detail-with-a-public-name (reachable as +// `simple_someip::client::ControlMessage` etc. for the +// `define_static_channels!` macro) rather than first-class crate-API +// types. Elevating them to crate root would lock their shape into +// the public-API contract and tempt generic users into hitting the +// `ClientChannelTypes` elaboration limit at the wrong call site. pub use e2e::{E2ECheckStatus, E2EKey, E2EProfile}; #[cfg(feature = "server")] -pub use server::{Server, ServerDeps, ServerHandles, SubscriptionHandle}; +pub use server::{Server, ServerDeps, ServerHandles, ServerStorage, SubscriptionHandle}; #[cfg(any(feature = "client-tokio", feature = "server-tokio"))] pub use tokio_transport::{TokioChannels, TokioSocket, TokioSpawner, TokioTimer, TokioTransport}; #[cfg(feature = "bare_metal")] diff --git a/src/log.rs b/src/log.rs new file mode 100644 index 00000000..f1d2ec3d --- /dev/null +++ b/src/log.rs @@ -0,0 +1,43 @@ +//! Internal log-macro shim. Crate-private — never re-exported. +//! +//! When the `tracing` feature is on, `crate::log::{debug, error, info, +//! trace, warn}` re-export the corresponding `tracing::*` macros +//! verbatim. When off (`bare_metal`-without-`std` builds, where the +//! `tracing-core` `extern crate alloc` declaration would fail against +//! a `core`-only sysroot), the names resolve to a single token-eating +//! macro that wraps `core::format_args!` in an `if false { … }` block: +//! references in the format string still count as variable uses for +//! the borrow checker (no spurious `unused_variables` lints in callers +//! that only consume a binding inside a log call), and the dead block +//! optimizes out, so no log code reaches the linker. + +// `unused_imports` because rustc only counts a macro re-export as used +// when it appears in a `use crate::log::name` path; bare-macro +// invocations (`crate::log::name!(…)`) are resolved through the macro +// table rather than the item table and don't satisfy the lint. Three +// of these (debug/error/info) happen not to appear in any `use`-list +// today, so they trip an unused-import warning that doesn't reflect +// reality. Suppress here rather than restructure the call sites. +#[cfg(feature = "tracing")] +#[allow(unused_imports)] +pub(crate) use tracing::{debug, error, info, trace, warn}; + +#[cfg(not(feature = "tracing"))] +macro_rules! noop { + ($($arg:tt)+) => { + if false { + let _ = ::core::format_args!($($arg)+); + } + }; +} + +#[cfg(not(feature = "tracing"))] +pub(crate) use noop as debug; +#[cfg(not(feature = "tracing"))] +pub(crate) use noop as error; +#[cfg(not(feature = "tracing"))] +pub(crate) use noop as info; +#[cfg(not(feature = "tracing"))] +pub(crate) use noop as trace; +#[cfg(not(feature = "tracing"))] +pub(crate) use noop as warn; diff --git a/src/server/event_publisher.rs b/src/server/event_publisher.rs index f4a04723..82f6903f 100644 --- a/src/server/event_publisher.rs +++ b/src/server/event_publisher.rs @@ -38,11 +38,11 @@ const _: () = assert!( /// to use `Arc` (which is `Send + Sync` whenever `T` is) without /// any change. /// -/// The explicit `T` parameter is the price of consolidating all -/// three former handle traits (Phase 20e) into a single -/// [`SharedHandle`]: the trait carries `T` as a generic, not -/// as an associated type, so consumers that need to name the -/// socket type spell it out. +/// The explicit `T` parameter is the price of consolidating the +/// three former handle traits (`SocketHandle`, `SdStateHandle`, +/// `EventPublisherHandle`) into a single [`SharedHandle`]: the +/// trait carries `T` as a generic, not as an associated type, so +/// consumers that need to name the socket type spell it out. pub struct EventPublisher where R: E2ERegistryHandle, @@ -127,7 +127,7 @@ where .await; if subscribers.is_empty() { - tracing::trace!( + crate::log::trace!( "No subscribers for service 0x{:04X}, instance {}, event group 0x{:04X}", service_id, instance_id, @@ -142,7 +142,7 @@ where // and the client socket_manager path. let required_size = message.required_size(); if required_size > UDP_BUFFER_SIZE { - tracing::error!( + crate::log::error!( "Message size ({} bytes) exceeds UDP_BUFFER_SIZE ({}); dropping publish", required_size, UDP_BUFFER_SIZE @@ -175,7 +175,7 @@ where match result { Some(Ok(protected_len)) => { if 16 + protected_len > UDP_BUFFER_SIZE { - tracing::error!( + crate::log::error!( "E2E-protected datagram ({} bytes, header + protected payload) \ exceeds UDP_BUFFER_SIZE ({}); dropping publish", 16 + protected_len, @@ -197,7 +197,7 @@ where // receiver's CRC/counter checks. Counter // exhaustion, key-lookup races, and similar // backend errors all funnel here. - tracing::error!("E2E protect error: {:?}; dropping publish", e); + crate::log::error!("E2E protect error: {:?}; dropping publish", e); return Err(Error::E2e(e)); } None => unreachable!("contains_key was true"), @@ -218,20 +218,20 @@ where match self.socket.get().send_to(datagram, *addr).await { Ok(()) => { sent_count += 1; - tracing::trace!( + crate::log::trace!( "Sent event to subscriber {} ({} bytes)", addr, message_length ); } Err(e) => { - tracing::error!("Failed to send event to subscriber {}: {:?}", addr, e); + crate::log::error!("Failed to send event to subscriber {}: {:?}", addr, e); last_err = Some(e); } } } - tracing::debug!( + crate::log::debug!( "Published event to {}/{} subscribers for service 0x{:04X}", sent_count, subscribers.len(), @@ -290,7 +290,7 @@ where // where `Header::SIZE + payload` could overflow `usize`. The // `16` here is the SOME/IP header size in bytes. if payload.len() > UDP_BUFFER_SIZE.saturating_sub(16) { - tracing::error!( + crate::log::error!( "raw event payload ({} bytes) + 16-byte header exceeds UDP_BUFFER_SIZE ({}); dropping publish", payload.len(), UDP_BUFFER_SIZE @@ -313,7 +313,7 @@ where let mut buffer = [0u8; UDP_BUFFER_SIZE]; let header_len = header.encode_to_slice(&mut buffer)?; let Some(total_len) = header_len.checked_add(payload.len()) else { - tracing::error!( + crate::log::error!( "raw event length computation overflowed usize (header_len={}, payload.len()={}); dropping publish", header_len, payload.len() @@ -325,7 +325,7 @@ where // post-encode tail bytes (e.g. another protect profile) would // need this branch. Cheap to keep. if total_len > UDP_BUFFER_SIZE { - tracing::error!( + crate::log::error!( "raw event ({} bytes) exceeds UDP_BUFFER_SIZE ({}); dropping publish", total_len, UDP_BUFFER_SIZE @@ -346,7 +346,7 @@ where sent_count += 1; } Err(e) => { - tracing::error!("Failed to send raw event to {}: {:?}", addr, e); + crate::log::error!("Failed to send raw event to {}: {:?}", addr, e); last_err = Some(e); } } @@ -469,13 +469,12 @@ where } } -// Phase 20e collapsed `EventPublisherHandle` / -// `WrappableEventPublisherHandle` into the unified -// `crate::transport::SharedHandle>` / -// `WrappableSharedHandle>` traits. The -// blanket impls there cover both `&'static EventPublisher<...>` -// and `Arc>`; no dedicated trait survives -// here. +// `EventPublisherHandle` / +// `WrappableEventPublisherHandle` were collapsed into the +// unified `crate::transport::SharedHandle>` +// / `WrappableSharedHandle>` traits. The +// blanket impls there cover both `&'static EventPublisher<...>` and +// `Arc>`; no dedicated trait survives here. #[cfg(all(test, feature = "server-tokio"))] mod tests { diff --git a/src/server/mod.rs b/src/server/mod.rs index 46be4d27..ac6d61d0 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -8,6 +8,7 @@ mod error; mod event_publisher; +mod runtime; mod sd_state; mod service_info; mod subscription_manager; @@ -27,14 +28,15 @@ use core::sync::atomic::{AtomicBool, Ordering}; use crate::Timer; use crate::e2e::{E2EKey, E2EProfile}; -use crate::protocol::sd::{self, Entry, Flags, OptionsCount, ServiceEntry, TransportProtocol}; +use crate::protocol::sd; +#[cfg(test)] +use crate::protocol::sd::{Entry, Flags, ServiceEntry}; use crate::transport::{ E2ERegistryHandle, SharedHandle, SocketOptions, TransportFactory, TransportSocket, WrappableSharedHandle, }; use alloc::sync::Arc; use core::net::{Ipv4Addr, SocketAddrV4}; -use futures_util::{FutureExt, pin_mut, select_biased}; #[cfg(test)] use std::vec::Vec; @@ -69,6 +71,16 @@ pub struct ServerConfig { /// accepted — preserves back-compat for callers that have not /// enumerated their groups; populate to opt into validation. pub event_group_ids: heapless::Vec, + /// Whether the run-future drives the SD `OfferService` announcement + /// loop. Defaults to `true`. + /// + /// Set to `false` (via [`Self::with_announce`]) when an external + /// component drives announcements — for example the + /// `examples/client_server` topology where a co-located `Client`'s + /// `sd_announcements_loop` emits the offers and the server should + /// stay silent on SD. Has no effect on passive servers, which never + /// announce. + pub announce: bool, } impl ServerConfig { @@ -77,27 +89,168 @@ impl ServerConfig { /// subscription manager. pub const EVENT_GROUP_IDS_CAP: usize = 32; - /// Create a new server configuration + /// Create a new server configuration with sane defaults for + /// development. + /// + /// Required arguments are the SOME/IP `service_id` and + /// `instance_id` — the two values that identify the offered + /// service. Other fields use development-friendly defaults that + /// production callers will typically override via the fluent + /// setters: + /// + /// | Field | Default | Override via | + /// |---|---|---| + /// | `interface` | [`Ipv4Addr::UNSPECIFIED`] (`0.0.0.0`) | [`Self::with_interface`] | + /// | `local_port` | `0` (kernel-assigned ephemeral) | [`Self::with_local_port`] | + /// | `major_version` | `1` | [`Self::with_major_version`] | + /// | `minor_version` | `0` | [`Self::with_minor_version`] | + /// | `ttl` | 3 seconds (typical for SOME/IP) | [`Self::with_ttl`] | + /// | `event_group_ids` | empty (any group accepted) | [`Self::with_event_group`] | + /// + /// Production deployments almost always need a specific interface + /// and port — `0.0.0.0` lets the kernel pick a binding that may + /// not match the service's E/E-architecture wiring expectations, + /// and an ephemeral port can't be discovered by peers without a + /// separate side-channel. Treat the defaults as "good enough to + /// stand up a test server in three lines" rather than + /// production-ready. + /// + /// # Example + /// + /// ``` + /// use simple_someip::server::ServerConfig; + /// use std::net::Ipv4Addr; + /// + /// let config = ServerConfig::new(0x5BAA, 1) + /// .with_interface(Ipv4Addr::new(192, 168, 1, 100)) + /// .with_local_port(30500); + /// ``` #[must_use] - pub fn new(interface: Ipv4Addr, local_port: u16, service_id: u16, instance_id: u16) -> Self { + pub fn new(service_id: u16, instance_id: u16) -> Self { Self { - interface, - local_port, + interface: Ipv4Addr::UNSPECIFIED, + local_port: 0, service_id, instance_id, major_version: 1, minor_version: 0, ttl: 3, // 3 seconds is typical for SOME/IP event_group_ids: heapless::Vec::new(), + announce: true, } } + /// Set the local interface IP address. Defaults to + /// [`Ipv4Addr::UNSPECIFIED`] (`0.0.0.0`) from [`Self::new`] — + /// production deployments will almost always override this to + /// match their E/E-architecture wiring. + #[must_use] + pub fn with_interface(mut self, interface: Ipv4Addr) -> Self { + self.interface = interface; + self + } + + /// Set the local UDP port the server listens on for subscription + /// requests and unicast traffic. Defaults to `0` from + /// [`Self::new`] (kernel-assigned ephemeral port), which is fine + /// for tests but cannot be discovered by external peers and + /// should be set explicitly in production. + #[must_use] + pub fn with_local_port(mut self, local_port: u16) -> Self { + self.local_port = local_port; + self + } + /// Returns `true` if `event_group_id` is registered, OR /// [`Self::event_group_ids`] is empty (validation disabled). #[must_use] pub fn accepts_event_group(&self, event_group_id: u16) -> bool { self.event_group_ids.is_empty() || self.event_group_ids.contains(&event_group_id) } + + // ── Fluent builder ─────────────────────────────────────────────── + // + // Each `with_*` setter consumes and returns `self` so callers can + // chain overrides starting from `Self::new(...)`. The struct's + // public fields stay available; the builder is just a less-noisy + // path for the common "constructor + a couple of overrides" shape. + + /// Set the SOME/IP major version. Defaults to `1` from + /// [`Self::new`]. + #[must_use] + pub fn with_major_version(mut self, major_version: u8) -> Self { + self.major_version = major_version; + self + } + + /// Set the SOME/IP minor version. Defaults to `0` from + /// [`Self::new`]. + #[must_use] + pub fn with_minor_version(mut self, minor_version: u32) -> Self { + self.minor_version = minor_version; + self + } + + /// Set the SD announcement TTL. Defaults to 3 seconds from + /// [`Self::new`] (typical for SOME/IP). + /// + /// The SOME/IP-SD wire format encodes TTL as `u32` whole seconds; + /// sub-second precision in the supplied `Duration` is truncated + /// (rounded down). Durations exceeding `u32::MAX` seconds (~136 + /// years) saturate to `u32::MAX`. The reserved special value + /// `0xFFFFFF` ("until next reboot") can be requested by passing + /// `Duration::from_secs(0xFFFFFF)`. + #[must_use] + pub fn with_ttl(mut self, ttl: core::time::Duration) -> Self { + self.ttl = u32::try_from(ttl.as_secs()).unwrap_or(u32::MAX); + self + } + + /// Append an event-group ID to the registered set. Subscriptions + /// for groups not in this set are NACK'd; an empty set (the + /// default after [`Self::new`]) accepts any group. + /// + /// # Panics + /// + /// Panics if more than [`Self::EVENT_GROUP_IDS_CAP`] groups have + /// been registered. Use [`Self::try_with_event_group`] for the + /// fallible variant. + #[must_use] + pub fn with_event_group(mut self, event_group_id: u16) -> Self { + self.event_group_ids + .push(event_group_id) + .expect("event_group_ids capacity exceeded"); + self + } + + /// Fallible counterpart to [`Self::with_event_group`]. + /// + /// # Errors + /// + /// Returns the unmodified config (in `Err`) if registering would + /// exceed [`Self::EVENT_GROUP_IDS_CAP`]. + #[must_use = "the returned `Result` carries the (possibly-modified) config — drop is silent"] + pub fn try_with_event_group(mut self, event_group_id: u16) -> Result { + if self.event_group_ids.push(event_group_id).is_ok() { + Ok(self) + } else { + Err(self) + } + } + + /// Set whether the run-future drives the SD `OfferService` + /// announcement loop. Defaults to `true` from [`Self::new`]. + /// + /// Pass `false` for the dispatcher topology where a co-located + /// `Client` drives SD via its own `sd_announcements_loop` and the + /// server should stay silent on the SD socket. Passive servers + /// (constructed via `Server::new_passive*`) ignore this setting — + /// they never announce regardless. + #[must_use] + pub fn with_announce(mut self, announce: bool) -> Self { + self.announce = announce; + self + } } /// Bundle of pluggable infrastructure passed to [`Server::new_with_deps`]. @@ -108,12 +261,12 @@ impl ServerConfig { /// /// All four fields are public so callers can construct the struct /// inline. -pub struct ServerDeps +pub struct ServerDeps where F: TransportFactory, Tm: Timer, R: E2ERegistryHandle, - S: SubscriptionHandle, + Sub: SubscriptionHandle, { /// Transport factory used to bind the unicast and SD sockets. pub factory: F, @@ -125,7 +278,153 @@ where /// `Server::new` (under `server-tokio`) builds an /// `Arc>` for this; bare-metal callers /// supply their own [`SubscriptionHandle`] impl. - pub subscriptions: S, + pub subscriptions: Sub, +} + +/// Tokio-defaulted constructor. +/// +/// Available under the `server-tokio` feature. Returns a `ServerDeps` +/// pre-populated with `TokioTransport` / `TokioTimer` and a fresh +/// `Arc>` / `Arc>`. +/// Combine with the [`ServerDeps::with_factory`] / +/// [`ServerDeps::with_timer`] / [`ServerDeps::with_e2e_registry`] / +/// [`ServerDeps::with_subscriptions`] builders to override individual +/// fields without spelling out the rest by hand. +/// +/// ```no_run +/// # #[cfg(feature = "server-tokio")] +/// # async fn demo() -> Result<(), simple_someip::server::Error> { +/// use simple_someip::{Server, ServerDeps}; +/// use simple_someip::server::ServerConfig; +/// use std::net::Ipv4Addr; +/// let deps = ServerDeps::tokio(); +/// let config = ServerConfig::new(0x1234, 1).with_interface(Ipv4Addr::LOCALHOST).with_local_port(0); +/// // The binding-site type fixes Server's `H`/`Hsd`/`Hep` to their +/// // `Arc<…>` defaults so type inference doesn't have to chase them. +/// let (_server, _handles, _run): (Server<_, _, _, _>, _, _) = +/// Server::new_with_deps(deps, config, false).await?; +/// # Ok(()) +/// # } +/// ``` +#[cfg(feature = "server-tokio")] +impl + ServerDeps< + crate::tokio_transport::TokioTransport, + crate::tokio_transport::TokioTimer, + Arc>, + Arc>, + > +{ + /// Build a `ServerDeps` with the tokio defaults. + #[must_use] + pub fn tokio() -> Self { + Self { + factory: crate::tokio_transport::TokioTransport, + timer: crate::tokio_transport::TokioTimer, + e2e_registry: Arc::new(Mutex::new(E2ERegistry::new())), + subscriptions: Arc::new(RwLock::new(SubscriptionManager::new())), + } + } +} + +/// Field-by-field fluent builder. Each `with_*` returns a new +/// `ServerDeps` with that single field replaced (and its corresponding +/// generic parameter updated). Lets callers start from +/// [`ServerDeps::tokio`] and override individual fields without +/// spelling out the full struct literal. +impl ServerDeps +where + F: TransportFactory, + Tm: Timer, + R: E2ERegistryHandle, + Sub: SubscriptionHandle, +{ + /// Replace the `factory` field, returning a `ServerDeps` over the + /// new factory type. + pub fn with_factory(self, factory: F2) -> ServerDeps { + ServerDeps { + factory, + timer: self.timer, + e2e_registry: self.e2e_registry, + subscriptions: self.subscriptions, + } + } + + /// Replace the `timer` field, returning a `ServerDeps` over the new + /// timer type. + pub fn with_timer(self, timer: Tm2) -> ServerDeps { + ServerDeps { + factory: self.factory, + timer, + e2e_registry: self.e2e_registry, + subscriptions: self.subscriptions, + } + } + + /// Replace the `e2e_registry` field, returning a `ServerDeps` over + /// the new registry-handle type. + pub fn with_e2e_registry( + self, + e2e_registry: R2, + ) -> ServerDeps { + ServerDeps { + factory: self.factory, + timer: self.timer, + e2e_registry, + subscriptions: self.subscriptions, + } + } + + /// Replace the `subscriptions` field, returning a `ServerDeps` over + /// the new subscription-handle type. + pub fn with_subscriptions( + self, + subscriptions: Sub2, + ) -> ServerDeps { + ServerDeps { + factory: self.factory, + timer: self.timer, + e2e_registry: self.e2e_registry, + subscriptions, + } + } +} + +/// Post-construction accessor bundle returned from `Server::new` (and +/// the other constructor variants) alongside the [`Server`] handle and +/// the combined run-future. +/// +/// Mirrors `crate::ClientUpdates`'s role on the [`Client`](crate::Client) +/// side: a place to hang things the caller will reach for once +/// construction completes (today: just the +/// [`EventPublisher`](crate::server::EventPublisher) handle; future +/// fields are reserved for forward-compat). Existing +/// `Server::publisher()` accessor is unchanged — the field on this +/// struct is the more discoverable path now that `Server::new` returns +/// it up front. +/// +/// The single field is public so callers can destructure inline: +/// ```no_run +/// # #[cfg(feature = "server-tokio")] +/// # async fn demo() -> Result<(), simple_someip::server::Error> { +/// use simple_someip::Server; +/// use simple_someip::server::ServerConfig; +/// use std::net::Ipv4Addr; +/// let config = ServerConfig::new(0x1234, 1) +/// .with_interface(Ipv4Addr::LOCALHOST) +/// .with_local_port(0); +/// let (_server, handles, run) = Server::new(config).await?; +/// let _publisher = handles.publisher; +/// tokio::spawn(run); +/// # Ok(()) +/// # } +/// ``` +pub struct ServerHandles { + /// `EventPublisher` handle for emitting events from the server side + /// (clone of the field on [`Server`]; included here so the common + /// destructuring pattern doesn't have to call `.publisher()` + /// separately). + pub publisher: Hep, } /// Bundle of pre-built dependencies + storage handles for @@ -146,15 +445,15 @@ where /// /// All eight fields are public so the struct can be assembled /// inline. -pub struct ServerHandles +pub struct ServerStorage where F: TransportFactory + 'static, Tm: Timer, R: E2ERegistryHandle, - S: SubscriptionHandle, + Sub: SubscriptionHandle, H: SharedHandle, Hsd: SharedHandle, - Hep: SharedHandle>, + Hep: SharedHandle>, { /// Transport factory. Retained on the `Server` for any /// post-construction state the backend needs to keep alive @@ -167,7 +466,7 @@ where /// Shared E2E registry handle for runtime E2E configuration. pub e2e_registry: R, /// Shared subscription manager handle. - pub subscriptions: S, + pub subscriptions: Sub, /// Pre-built unicast socket handle. Caller has already bound /// the underlying socket to the desired interface + port. pub unicast_socket: H, @@ -190,34 +489,38 @@ where /// /// Generic over the four pluggable infrastructure types bundled in /// [`ServerDeps`]: -/// - `R: E2ERegistryHandle` — runtime E2E configuration registry -/// - `S: SubscriptionHandle` — event-group subscription state /// - `F: TransportFactory` — socket primitive (carried as a stored /// unit-struct in the tokio path; bare-metal impls may carry state) /// - `Tm: Timer` — async sleep used by the announcement loop +/// - `R: E2ERegistryHandle` — runtime E2E configuration registry +/// - `Sub: SubscriptionHandle` — event-group subscription state +/// +/// The generic order mirrors [`ServerDeps`] (and, for the shared +/// infrastructure parameters `F`, `Tm`, `R`, the order is also shared +/// with [`crate::ClientDeps`]). /// /// The convenience constructors `Self::new` / `Self::new_with_loopback` /// / `Self::new_passive` (under the `server-tokio` feature) instantiate -/// these as `Arc>` / `Arc>` -/// / `TokioTransport` / `TokioTimer`. Bare-metal callers use +/// these as `TokioTransport` / `TokioTimer` / `Arc>` +/// / `Arc>`. Bare-metal callers use /// [`Self::new_with_deps`] (under `server`) and supply their own. pub struct Server< - R, - S, F, Tm, + R, + Sub, H = Arc<::Socket>, Hsd = Arc, - Hep = Arc::Socket>>, + Hep = Arc::Socket>>, > where - R: E2ERegistryHandle, - S: SubscriptionHandle, F: TransportFactory + 'static, F::Socket: 'static, Tm: Timer + Clone + 'static, + R: E2ERegistryHandle, + Sub: SubscriptionHandle, H: SharedHandle, Hsd: SharedHandle, - Hep: SharedHandle>, + Hep: SharedHandle>, { config: ServerConfig, /// Socket for receiving subscription requests, behind whatever @@ -228,10 +531,10 @@ pub struct Server< /// `unicast_socket`; both are produced by the same factory). sd_socket: H, /// Subscription manager - subscriptions: S, + subscriptions: Sub, /// Event publisher, behind whatever shared-storage `Hep` chose - /// (`Arc>` on std, - /// `&'static EventPublisher` on bare-metal-no-alloc). + /// (`Arc>` on std, + /// `&'static EventPublisher` on bare-metal-no-alloc). publisher: Hep, /// SD session-ID counter and announcement emitter, behind whatever /// shared-storage `Hsd` chose (`Arc` on std, @@ -251,33 +554,84 @@ pub struct Server< timer: Tm, /// `true` if this server was constructed via `Server::new_passive`. /// Passive servers have no real SD socket bound to port 30490; their - /// SD handling is managed externally. Calling [`Self::announcement_loop`] - /// or [`Self::run`] on a passive server is a programming error and - /// returns an [`Error::Io`] with [`std::io::ErrorKind::InvalidInput`]. + /// SD handling is managed externally. Calling [`Self::run`] on a + /// passive server is a programming error and returns + /// [`Error::InvalidUsage`]. is_passive: bool, - /// Set the first time [`Self::announcement_loop`] is called. A - /// second call returns `Err(Error::Io(InvalidInput))` so two - /// independent futures cannot race on the same SD socket and - /// session counter. - announcement_loop_started: AtomicBool, + /// Latch flipped on the first poll of any run-future built from + /// this `Server`. Subsequent run-futures (whether from the + /// constructor's tuple, [`Self::run`], or [`Self::run_with_buffers`]) + /// short-circuit with `Err(Error::InvalidUsage("server_already_running"))` + /// rather than racing on the same SD/unicast sockets and session + /// counter. `Arc` because the run-future captures a + /// clone independent of `&self`'s lifetime. + started: Arc, } +/// `Hep` resolved against the `server-tokio` convenience constructors' +/// concrete defaults — the `EventPublisher` shape with all four +/// publisher type parameters bound to their tokio impls. Lets the +/// tokio constructors' `(Self, ServerHandles<…>, run-future)` return +/// type spell out cleanly rather than dragging the four-deep `Arc<…>` +/// chain through every signature. #[cfg(feature = "server-tokio")] -impl - Server< +type DefaultTokioServerHep = Arc< + EventPublisher< Arc>, Arc>, + Arc, + crate::tokio_transport::TokioSocket, + >, +>; + +#[cfg(feature = "server-tokio")] +impl + Server< crate::tokio_transport::TokioTransport, crate::tokio_transport::TokioTimer, + Arc>, + Arc>, > { - /// Create a new SOME/IP server + /// Create a new SOME/IP server. + /// + /// Returns the `Server` handle for runtime mutation + /// (`register_e2e`, `publisher`, etc.), a [`ServerHandles`] bundle + /// destructuring the [`EventPublisher`] up front, and a single + /// combined run-future the caller spawns to drive both the + /// receive loop and (unless suppressed via + /// [`ServerConfig::with_announce`]) the SD announcement loop. + /// + /// ```no_run + /// # #[cfg(feature = "server-tokio")] + /// # async fn demo() -> Result<(), simple_someip::server::Error> { + /// use simple_someip::Server; + /// use simple_someip::server::ServerConfig; + /// use std::net::Ipv4Addr; + /// let config = ServerConfig::new(0x1234, 1) + /// .with_interface(Ipv4Addr::LOCALHOST) + /// .with_local_port(0); + /// let (_server, handles, run) = Server::new(config).await?; + /// let _publisher = handles.publisher; + /// tokio::spawn(run); + /// # Ok(()) + /// # } + /// ``` /// /// # Errors /// /// Returns an error if binding the unicast or SD socket fails, or if joining the /// SD multicast group fails. - pub async fn new(config: ServerConfig) -> Result { + pub async fn new( + config: ServerConfig, + ) -> Result< + ( + Self, + ServerHandles, + impl core::future::Future> + 'static, + ), + Error, + > { Self::new_with_loopback(config, false).await } @@ -308,7 +662,14 @@ impl pub async fn new_with_loopback( config: ServerConfig, multicast_loopback: bool, - ) -> Result { + ) -> Result< + ( + Self, + ServerHandles, + impl core::future::Future> + 'static, + ), + Error, + > { let deps = ServerDeps { factory: crate::tokio_transport::TokioTransport, timer: crate::tokio_transport::TokioTimer, @@ -339,7 +700,16 @@ impl /// # Errors /// /// Returns an error if binding either socket fails. - pub async fn new_passive(config: ServerConfig) -> Result { + pub async fn new_passive( + config: ServerConfig, + ) -> Result< + ( + Self, + ServerHandles, + impl core::future::Future> + 'static, + ), + Error, + > { let deps = ServerDeps { factory: crate::tokio_transport::TokioTransport, timer: crate::tokio_transport::TokioTimer, @@ -350,16 +720,16 @@ impl } } -impl Server +impl Server where - R: E2ERegistryHandle, - S: SubscriptionHandle, F: TransportFactory + 'static, F::Socket: 'static, Tm: Timer + Clone + 'static, + R: E2ERegistryHandle, + Sub: SubscriptionHandle, H: WrappableSharedHandle, Hsd: WrappableSharedHandle, - Hep: WrappableSharedHandle>, + Hep: WrappableSharedHandle>, { /// Bare-metal-friendly constructor that takes every dependency /// explicitly via a [`ServerDeps`] bundle. The `server-tokio` @@ -381,10 +751,17 @@ where /// [`TransportFactory::bind`] fails, or if joining the SD multicast /// group fails. pub async fn new_with_deps( - deps: ServerDeps, + deps: ServerDeps, mut config: ServerConfig, multicast_loopback: bool, - ) -> Result { + ) -> Result< + ( + Self, + ServerHandles, + impl core::future::Future> + 'static, + ), + Error, + > { let ServerDeps { factory, timer, @@ -404,7 +781,7 @@ where // ephemeral port. Back-fill the config so SD offers and event // publishers advertise the actual bound port instead of 0. config.local_port = bound_port; - tracing::info!( + crate::log::info!( "Server bound to {}:{} for service 0x{:04X}", config.interface, bound_port, @@ -421,7 +798,7 @@ where let sd_raw = factory.bind(sd_addr, &sd_opts).await?; sd_raw.join_multicast_v4(sd::MULTICAST_IP, config.interface)?; let sd_socket: H = H::wrap(sd_raw); - tracing::info!( + crate::log::info!( "Server SD socket bound to {} (expected port {}), joined multicast {}", sd_addr, sd::MULTICAST_PORT, @@ -434,7 +811,7 @@ where e2e_registry.clone(), )); - Ok(Self { + let server = Self { config, unicast_socket, sd_socket, @@ -445,8 +822,13 @@ where factory, timer, is_passive: false, - announcement_loop_started: AtomicBool::new(false), - }) + started: Arc::new(AtomicBool::new(false)), + }; + let handles = ServerHandles { + publisher: server.publisher(), + }; + let run = server.run_inner(); + Ok((server, handles, run)) } /// Bare-metal-friendly passive-server constructor. @@ -461,9 +843,16 @@ where /// /// Returns an error if binding either socket fails. pub async fn new_passive_with_deps( - deps: ServerDeps, + deps: ServerDeps, mut config: ServerConfig, - ) -> Result { + ) -> Result< + ( + Self, + ServerHandles, + impl core::future::Future> + 'static, + ), + Error, + > { let ServerDeps { factory, timer, @@ -478,7 +867,7 @@ where let unicast_socket: H = H::wrap(unicast_raw); // Back-fill the actual bound port if the caller passed 0. config.local_port = bound_port; - tracing::info!( + crate::log::info!( "Passive server bound to {}:{} for service 0x{:04X}", config.interface, bound_port, @@ -493,7 +882,7 @@ where .bind(sd_placeholder_addr, &SocketOptions::new()) .await?, ); - tracing::info!( + crate::log::info!( "Passive server SD placeholder socket bound near {} (not in SD reuseport group)", sd_placeholder_addr ); @@ -504,7 +893,7 @@ where e2e_registry.clone(), )); - Ok(Self { + let server = Self { config, unicast_socket, sd_socket, @@ -515,21 +904,26 @@ where factory, timer, is_passive: true, - announcement_loop_started: AtomicBool::new(false), - }) + started: Arc::new(AtomicBool::new(false)), + }; + let handles = ServerHandles { + publisher: server.publisher(), + }; + let run = server.run_inner(); + Ok((server, handles, run)) } } -impl Server +impl Server where - R: E2ERegistryHandle, - S: SubscriptionHandle, F: TransportFactory + 'static, F::Socket: 'static, Tm: Timer + Clone + 'static, + R: E2ERegistryHandle, + Sub: SubscriptionHandle, H: SharedHandle, Hsd: SharedHandle, - Hep: SharedHandle>, + Hep: SharedHandle>, { /// Construct a `Server` from pre-built dependencies + storage /// handles. The bare-metal-no-alloc counterpart to @@ -560,14 +954,14 @@ where /// [`Error::InvalidUsage`] if `config.local_port` is non-zero /// and does not equal the unicast socket's bound port. pub fn new_with_handles( - deps: ServerHandles, + deps: ServerStorage, mut config: ServerConfig, ) -> Result { let bound_port = deps.unicast_socket.get().local_addr()?.port(); if config.local_port == 0 { config.local_port = bound_port; } else if config.local_port != bound_port { - tracing::error!( + crate::log::error!( "ServerConfig.local_port ({}) does not match unicast socket's \ bound port ({}); SD offers would lie. Pass local_port = 0 to \ auto-fill from the bound port instead.", @@ -576,7 +970,7 @@ where ); return Err(Error::InvalidUsage("new_with_handles_local_port_mismatch")); } - tracing::info!( + crate::log::info!( "Server (handles) bound to {}:{} for service 0x{:04X}", config.interface, bound_port, @@ -594,7 +988,7 @@ where factory: deps.factory, timer: deps.timer, is_passive: false, - announcement_loop_started: AtomicBool::new(false), + started: Arc::new(AtomicBool::new(false)), }) } @@ -623,14 +1017,14 @@ where /// back-fill-only-on-zero discipline as /// [`Self::new_with_handles`]). pub fn new_passive_with_handles( - deps: ServerHandles, + deps: ServerStorage, mut config: ServerConfig, ) -> Result { let bound_port = deps.unicast_socket.get().local_addr()?.port(); if config.local_port == 0 { config.local_port = bound_port; } else if config.local_port != bound_port { - tracing::error!( + crate::log::error!( "ServerConfig.local_port ({}) does not match unicast socket's \ bound port ({}); event publishers would advertise a port \ nothing is listening on. Pass local_port = 0 to auto-fill.", @@ -641,7 +1035,7 @@ where "new_passive_with_handles_local_port_mismatch", )); } - tracing::info!( + crate::log::info!( "Passive server (handles) bound to {}:{} for service 0x{:04X}", config.interface, bound_port, @@ -659,254 +1053,17 @@ where factory: deps.factory, timer: deps.timer, is_passive: true, - announcement_loop_started: AtomicBool::new(false), + started: Arc::new(AtomicBool::new(false)), }) } - /// Build the periodic-SD-announcement future. - /// - /// Returns a future that sends an `OfferService` message to the SD - /// multicast group every second. The caller must drive the future - /// (typically via `tokio::spawn`) for announcements to fire; this - /// function does no work on its own. - /// - /// ```no_run - /// # #[cfg(feature = "server-tokio")] { - /// # use simple_someip::server::{Server, ServerConfig}; - /// # use std::net::Ipv4Addr; - /// # async fn demo() -> Result<(), simple_someip::server::Error> { - /// # let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30490, 0, 0); - /// # let server = Server::new(config).await?; - /// let announce_fut = server.announcement_loop()?; - /// tokio::spawn(announce_fut); - /// # Ok(()) - /// # } - /// # } - /// ``` - /// - /// # Errors - /// - /// Returns [`Error::InvalidUsage`] (with the tag - /// `"passive_server_announcement_loop"` or - /// `"announcement_loop_already_started"`) if: - /// - called on a server constructed via `Server::new_passive` — passive - /// servers have no real SD socket bound to port 30490, so any - /// announcements would go out with an incorrect source port; or - /// - called twice on the same server. Two announcement futures - /// driving the same SD socket and session counter would double the - /// announcement rate and race on the wrap-flag latch. Drop the - /// first future to disable announcements before requesting a new - /// one (which currently still requires a fresh `Server`). - #[must_use = "the returned announcement-loop future must be spawned (e.g. tokio::spawn) or awaited for the server to emit SD announcements; dropping it silently disables announcements"] - pub fn announcement_loop( - &self, - ) -> Result + Send + 'static, Error> - where - F: Send + Sync, - F::Socket: Send + Sync, - for<'a> ::SendFuture<'a>: Send, - H: Send + Sync, - Hsd: Send + Sync, - Tm: Send + Sync, - for<'a> Tm::SleepFuture<'a>: Send, - { - if self.is_passive { - tracing::warn!( - "announcement_loop called on passive Server for service 0x{:04X}; \ - announcements must be driven externally (e.g. via \ - `simple_someip::Client::sd_announcements_loop`)", - self.config.service_id - ); - return Err(Error::InvalidUsage("passive_server_announcement_loop")); - } - if self - .announcement_loop_started - .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) - .is_err() - { - tracing::warn!( - "announcement_loop already started for service 0x{:04X}; \ - two announcement futures cannot share the same SD socket \ - and session counter", - self.config.service_id - ); - return Err(Error::InvalidUsage("announcement_loop_already_started")); - } - let config = self.config.clone(); - let sd_socket = self.sd_socket.clone(); - let sd_state = self.sd_state.clone(); - let timer = self.timer.clone(); - - Ok(async move { - let mut announcement_count = 0u32; - loop { - match sd_state - .get() - .send_offer_service(&config, sd_socket.get()) - .await - { - Ok(()) => { - announcement_count += 1; - if announcement_count == 1 { - tracing::info!( - "Sent first SD announcement for service 0x{:04X}", - config.service_id - ); - } else { - tracing::debug!( - "Sent {} SD announcements for service 0x{:04X}", - announcement_count, - config.service_id - ); - } - } - Err(e) => { - tracing::error!("Failed to send OfferService: {:?}", e); - } - } - - // Send announcements every 1 second. Sleep goes through - // the `Timer` trait so bare-metal consumers can swap in - // a different timer impl; today it resolves to - // `TokioTimer` under the `server-tokio` feature. - timer.sleep(core::time::Duration::from_secs(1)).await; - } - }) - } - - /// `!Send` counterpart to [`Self::announcement_loop`]. - /// - /// Returns the same announcement-loop future without the `+ Send` - /// bound on the return type, so it can be driven by single-threaded - /// executors (`tokio::task::LocalSet`, embassy with `task-arena = 0`, - /// etc.) over a `!Sync` transport such as `embassy-net`. Use this on - /// bare-metal targets where `H::Socket` is `!Sync`; use the - /// Send-bounded `announcement_loop` on multi-threaded targets. - /// - /// # Errors - /// - /// Same as [`Self::announcement_loop`]. - #[must_use = "the returned announcement-loop future must be driven (e.g. tokio::task::spawn_local) for the server to emit SD announcements; dropping it silently disables announcements"] - pub fn announcement_loop_local( - &self, - ) -> Result + 'static, Error> { - if self.is_passive { - tracing::warn!( - "announcement_loop_local called on passive Server for service 0x{:04X}; \ - announcements must be driven externally (e.g. via \ - `simple_someip::Client::sd_announcements_loop`)", - self.config.service_id - ); - return Err(Error::InvalidUsage("passive_server_announcement_loop")); - } - if self - .announcement_loop_started - .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) - .is_err() - { - tracing::warn!( - "announcement_loop already started for service 0x{:04X}; \ - two announcement futures cannot share the same SD socket \ - and session counter", - self.config.service_id - ); - return Err(Error::InvalidUsage("announcement_loop_already_started")); - } - let config = self.config.clone(); - let sd_socket = self.sd_socket.clone(); - let sd_state = self.sd_state.clone(); - let timer = self.timer.clone(); - - Ok(async move { - let mut announcement_count = 0u32; - loop { - match sd_state - .get() - .send_offer_service(&config, sd_socket.get()) - .await - { - Ok(()) => { - announcement_count += 1; - if announcement_count == 1 { - tracing::info!( - "Sent first SD announcement for service 0x{:04X}", - config.service_id - ); - } else { - tracing::debug!( - "Sent {} SD announcements for service 0x{:04X}", - announcement_count, - config.service_id - ); - } - } - Err(e) => { - tracing::error!("Failed to send OfferService: {:?}", e); - } - } - timer.sleep(core::time::Duration::from_secs(1)).await; - } - }) - } - - /// Send a unicast `OfferService` to a specific address (in response to `FindService`) - async fn send_unicast_offer(&self, target: core::net::SocketAddr) -> Result<(), Error> { - use crate::protocol::Header as SomeIpHeader; - use crate::traits::WireFormat; - - let entry = Entry::OfferService(ServiceEntry { - index_first_options_run: 0, - index_second_options_run: 0, - options_count: OptionsCount::new(1, 0), - service_id: self.config.service_id, - instance_id: self.config.instance_id, - major_version: self.config.major_version, - ttl: self.config.ttl, - minor_version: self.config.minor_version, - }); - - let option = sd::Options::IpV4Endpoint { - ip: self.config.interface, - port: self.config.local_port, - protocol: TransportProtocol::Udp, - }; - - let entries = [entry]; - let options = [option]; - // Atomic (sid, reboot_flag) pair so concurrent emissions cannot - // race around the wrap boundary — see - // `SdStateManager::next_session_id_with_reboot_flag` docs. - let (sid, reboot_flag) = self.sd_state.get().next_session_id_with_reboot_flag(); - let sd_payload = sd::Header::new(Flags::new_sd(reboot_flag), &entries, &options); - - let mut buffer = [0u8; crate::UDP_BUFFER_SIZE]; - let sd_data_len = sd_payload.encode_to_slice(&mut buffer[16..])?; - let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); - someip_header.encode_to_slice(&mut buffer[..16])?; - let total_len = 16 + sd_data_len; - - let target_v4 = socket_addr_v4(target)?; - self.sd_socket - .get() - .send_to(&buffer[..total_len], target_v4) - .await?; - tracing::debug!( - "Sent unicast OfferService to {} for service 0x{:04X}", - target, - self.config.service_id - ); - - Ok(()) - } - /// Get a clone of the event-publisher handle for sending events. /// /// Returns the `Hep` type parameter — typically - /// `Arc>` for std users (the default - /// `Hep`), `&'static EventPublisher` for + /// `Arc>` for std users (the default + /// `Hep`), `&'static EventPublisher` for /// bare-metal-no-alloc. (`EventPublisherHandle` was a former - /// trait alias collapsed into [`crate::transport::SharedHandle`] - /// in phase 19f / 20e.) + /// trait alias collapsed into [`crate::transport::SharedHandle`].) #[must_use] pub fn publisher(&self) -> Hep { self.publisher.clone() @@ -924,11 +1081,6 @@ where } } - /// Update the configured local port (useful after binding to ephemeral port 0). - pub fn set_local_port(&mut self, port: u16) { - self.config.local_port = port; - } - /// Register an E2E profile for the given key. /// /// Once registered, outgoing events published via [`EventPublisher::publish_event`] @@ -954,613 +1106,191 @@ where /// Run the server event loop with caller-provided receive buffers. /// - /// Handles incoming subscription requests and manages event groups. - /// Listens on both the unicast socket (for direct requests) and the - /// SD multicast socket (for `FindService` and `SubscribeEventGroup`). + /// Drives the receive loop (handling incoming `Subscribe` / + /// `FindService` SD messages on the SD multicast socket and + /// unicast traffic on the unicast socket) concurrently with the + /// 1-Hz `OfferService` announcement loop. The two are combined + /// into a single future so callers cannot forget to spawn the + /// announcement side; passing + /// [`ServerConfig::with_announce(false)`] suppresses the + /// announcement arm for dispatcher topologies where a co-located + /// `Client` drives SD on the server's behalf. /// /// `unicast_buf` and `sd_buf` are caller-supplied scratch buffers /// for incoming datagrams. Each must be at least one MTU /// (~1500 bytes) and ideally up to the IP datagram limit - /// (64 KiB - 1) — peer SD messages are bounded by the link MTU, - /// but a SOME/IP server should not silently cap at 1500 because - /// it is a sink for any peer datagram landing on its SD or - /// unicast port. The `ReceivedDatagram::truncated` flag - /// returned by [`crate::transport::TransportSocket::recv_from`] - /// is currently NOT inspected by this run loop: backends that - /// surface truncation will have it observable on the value, but - /// a follow-up pass is needed to emit the corresponding - /// `tracing::warn!`. Tracking issue: bare-metal plan v3 phase - /// 21+ backlog. + /// (64 KiB - 1). On bare-metal targets, callers typically place + /// these in `static` storage; on std (or any alloc-using + /// target), [`Self::run`] is the convenience shim that + /// heap-allocates 64 KiB buffers and delegates here. /// - /// On bare-metal, callers typically place the buffers in - /// `static` storage. `static mut` would require unsafe and is - /// a hard error in Rust 2024 when used through `&mut`; the - /// recommended pattern is a `static` cell wrapped in interior - /// mutability: - /// ```ignore - /// use core::cell::UnsafeCell; - /// // One owner per buffer: only the task driving - /// // `run_with_buffers` ever obtains a `&mut` from these cells, - /// // and the borrow lives only for the run-loop's lifetime. - /// struct Buf(UnsafeCell<[u8; 65535]>); - /// // SAFETY: hand-shaken — only the `Server::run_with_buffers` - /// // task touches the inner storage, so the `Sync` claim is - /// // sound for that single-owner discipline. - /// unsafe impl Sync for Buf {} - /// static UNICAST_BUF: Buf = Buf(UnsafeCell::new([0; 65535])); - /// static SD_BUF: Buf = Buf(UnsafeCell::new([0; 65535])); - /// // SAFETY: only one task drives `run_with_buffers` for a given Server. - /// let unicast = unsafe { &mut *UNICAST_BUF.0.get() }; - /// let sd = unsafe { &mut *SD_BUF.0.get() }; - /// server.run_with_buffers(unicast, sd).await?; - /// ``` - /// - /// On std (or any alloc-using target), [`Self::run`] is the - /// convenience shim that heap-allocates 64 KiB buffers and - /// delegates here. + /// The returned future is independent of `&self` — the cheap + /// shared-handle clones it captures own everything it needs to + /// drive both loops, so the caller can keep using `Server` to + /// register E2E profiles, query `unicast_local_addr`, etc. while + /// the future runs. /// /// # Errors /// /// Returns [`Error::InvalidUsage`] (tag `"passive_server_run"`) if - /// called on a server constructed via `Server::new_passive` — passive - /// servers have no real SD socket to read from, so the run loop would - /// block forever on the ephemeral placeholder socket. + /// the server was constructed via `Server::new_passive*` — passive + /// servers have no real SD socket to read from, so the run loop + /// would block forever on the ephemeral placeholder socket. /// - /// Otherwise returns an error if receiving from a socket fails or + /// Otherwise resolves to `Err` if receiving from a socket fails or /// handling an SD message fails. - pub async fn run_with_buffers( - &mut self, - unicast_buf: &mut [u8], - sd_buf: &mut [u8], - ) -> Result<(), Error> { - use crate::protocol::MessageView; - - if self.is_passive { - tracing::warn!( - "run called on passive Server for service 0x{:04X}; \ - SD receive must be driven externally (e.g. via the \ - Client's discovery socket, routing Subscribes to \ - `EventPublisher::register_subscriber`)", - self.config.service_id - ); - return Err(Error::InvalidUsage("passive_server_run")); - } - - // Iteration counter used to flip `select_biased!` arm priority - // each turn. We can't use the pseudo-random `select!` (it needs - // `std`), so flipping arm order each iteration approximates the - // fairness it would give without pulling std — a sustained - // one-sided load (only-unicast or only-sd) cannot starve the - // other arm. - let mut prefer_sd_first = false; - loop { - // SAFETY: both arms call `TransportSocket::recv_from`. The - // `TokioSocket` backend is cancel-safe per tokio docs — a - // non-selected arm can be dropped without losing in-flight - // kernel state. Custom transport backends MUST provide the - // same guarantee. A future contributor adding a - // non-cancel-safe `FusedFuture` arm here would silently lose - // state when the arm is dropped on a select win. Both futures - // must therefore stay `Send + FusedFuture + Unpin` *and* - // cancel-safe. - // - // Fresh futures are constructed each iteration so the borrows - // of `unicast_buf` / `sd_buf` / the sockets end when the - // select macro returns, freeing the buffer we index into - // below. - // Each arm returns just `(datagram, from_unicast)`; the - // `(len, addr, source)` derivation lives once below the - // select so the arm-flip pattern doesn't duplicate it. - let (datagram, from_unicast) = { - // Reborrow `&mut *foo` rather than `&mut foo` because - // `unicast_buf` / `sd_buf` are `&mut [u8]` parameters - // here (caller-owned), not owned `Vec` locals — - // direct `&mut foo` would produce `&mut &mut [u8]`. - let unicast_fut = self - .unicast_socket - .get() - .recv_from(&mut *unicast_buf) - .fuse(); - let sd_fut = self.sd_socket.get().recv_from(&mut *sd_buf).fuse(); - pin_mut!(unicast_fut, sd_fut); - if prefer_sd_first { - select_biased! { - result = sd_fut => (result?, false), - result = unicast_fut => (result?, true), - } - } else { - select_biased! { - result = unicast_fut => (result?, true), - result = sd_fut => (result?, false), - } - } - }; - prefer_sd_first = !prefer_sd_first; - let len = datagram.bytes_received; - let addr = core::net::SocketAddr::V4(datagram.source); - let source = if from_unicast { - "unicast" - } else { - "sd-multicast" - }; - let data = if from_unicast { - &unicast_buf[..len] - } else { - &sd_buf[..len] - }; - - // By default IP_MULTICAST_LOOP=false suppresses own multicast - // messages on the SD socket, so no source-IP filtering is needed. - // When the server was constructed via `Server::new_with_loopback` - // with `multicast_loopback = true` (e.g. for same-host testing), - // the kernel delivers our own SD multicasts back to this loop. - // That is tolerated here: `handle_sd_message` only acts on - // `Subscribe` / `SubscribeAck` / `FindService` entries, so the - // `OfferService` entries we send ourselves are effectively - // ignored. A self-sent `FindService` for our own service ID - // would trigger a unicast `OfferService` reply back to - // ourselves, which is the same behavior an external peer's - // `FindService` would produce and is therefore safe. - - tracing::trace!("Received {} bytes from {} on {} socket", len, addr, source); - tracing::trace!("Raw data: {:02X?}", &data[..len.min(64_usize)]); - - // Try to parse as SOME/IP message using zero-copy view - match MessageView::parse(data) { - Ok(view) => { - tracing::trace!( - "SOME/IP Header: service=0x{:04X}, method=0x{:04X}, type={:?}", - view.header().message_id().service_id(), - view.header().message_id().method_id(), - view.header().message_type().message_type() - ); - - // Check if this is a Service Discovery message (0xFFFF8100) - if view.is_sd() { - tracing::trace!("This is an SD message"); - // Parse SD payload - match view.sd_header() { - Ok(sd_view) => { - tracing::trace!("SD message has {} entries", sd_view.entry_count(),); - self.handle_sd_message(&sd_view, addr).await?; - } - Err(e) => { - tracing::warn!("Failed to parse SD message: {:?}", e); - } - } - } else { - tracing::trace!("Non-SD SOME/IP message, ignoring"); - } - } - Err(e) => { - tracing::warn!("Failed to parse SOME/IP header from {}: {:?}", addr, e); - tracing::trace!("Data: {:02X?}", &data[..len.min(32)]); - } + pub fn run_with_buffers<'a>( + &self, + unicast_buf: &'a mut [u8], + sd_buf: &'a mut [u8], + ) -> impl core::future::Future> + + 'a + + use<'a, F, Tm, R, Sub, H, Hsd, Hep> + where + Tm: 'a, + Sub: 'a, + H: 'a, + Hsd: 'a, + { + let config = self.config.clone(); + let unicast_socket = self.unicast_socket.clone(); + let sd_socket = self.sd_socket.clone(); + let subscriptions = self.subscriptions.clone(); + let sd_state = self.sd_state.clone(); + let timer = self.timer.clone(); + let is_passive = self.is_passive; + let started = self.started.clone(); + + async move { + // See `run_inner` for the rationale on the first-poll + // latch — same race, same fix. + if started + .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) + .is_err() + { + crate::log::warn!( + "Server::run_with_buffers already started for service 0x{:04X}; \ + a second run-future cannot share the same sockets \ + and session counter", + config.service_id + ); + return Err(Error::InvalidUsage("server_already_running")); } + + runtime::run_combined::( + config, + unicast_socket, + sd_socket, + subscriptions, + sd_state, + timer, + is_passive, + unicast_buf, + sd_buf, + ) + .await } } - /// Run the server event loop with heap-allocated 64 KiB recv buffers. + /// Run the server event loop with heap-allocated 64 KiB receive + /// buffers — the convenience entry point for std and alloc-using + /// bare-metal builds. Drives both the receive loop and (unless + /// suppressed via [`ServerConfig::with_announce`]) the + /// announcement loop in a single future. /// - /// Convenience wrapper over [`Self::run_with_buffers`] for callers - /// who have an allocator available — this is the simplest entry - /// point for std and bare-metal-with-alloc consumers. Bare-metal - /// callers without an allocator must use - /// [`Self::run_with_buffers`] directly with caller-supplied - /// buffers (e.g. `static`-declared `[u8; N]` arrays). + /// The returned future is `Send + 'static` under the where-clause + /// bounds spelled below, so it is suitable for `tokio::spawn`. + /// Single-threaded executors that need a `!Send` future (e.g. + /// `tokio::task::spawn_local` over a `!Sync` transport) should + /// call [`Self::run_with_buffers`] directly, which has no `Send` + /// requirement. /// - /// The 64 KiB sizing matches the IP datagram limit so the server - /// surfaces (or cleanly truncates at the OS level) any peer - /// datagram that exceeds the link MTU. See - /// [`Self::run_with_buffers`] for the full sizing rationale. + /// Bare-metal callers without an allocator must use + /// [`Self::run_with_buffers`] with caller-supplied buffers + /// (e.g. `static`-declared `[u8; N]` arrays). /// /// # Errors /// /// Same as [`Self::run_with_buffers`]. - pub async fn run(&mut self) -> Result<(), Error> { - let mut unicast_buf = alloc::vec![0u8; 65535]; - let mut sd_buf = alloc::vec![0u8; 65535]; - self.run_with_buffers(&mut unicast_buf, &mut sd_buf).await - } - - /// Handle a Service Discovery message - #[allow(clippy::too_many_lines)] - async fn handle_sd_message( - &mut self, - sd_view: &sd::SdHeaderView<'_>, - sender: core::net::SocketAddr, - ) -> Result<(), Error> { - tracing::trace!("Handling SD message from {}", sender); - - for entry_view in sd_view.entries() { - let entry_type = entry_view.entry_type()?; - match entry_type { - sd::EntryType::Subscribe => { - tracing::debug!( - "Received Subscribe from {}: service=0x{:04X}, instance={}, eventgroup=0x{:04X}", - sender, - entry_view.service_id(), - entry_view.instance_id(), - entry_view.event_group_id() - ); - - // Check if this is for our service. - if entry_view.service_id() != self.config.service_id { - tracing::warn!( - "Subscribe for wrong service: expected 0x{:04X}, got 0x{:04X}", - self.config.service_id, - entry_view.service_id() - ); - self.send_subscribe_nack_from_view(&entry_view, sender, "wrong_service_id") - .await?; - } else if entry_view.instance_id() != self.config.instance_id { - tracing::warn!( - "Subscribe for wrong instance: expected {}, got {}", - self.config.instance_id, - entry_view.instance_id() - ); - self.send_subscribe_nack_from_view( - &entry_view, - sender, - "wrong_instance_id", - ) - .await?; - } else if entry_view.major_version() != self.config.major_version { - // Per AUTOSAR SOME/IP-SD: a Subscribe whose - // major_version disagrees with the server's - // configured major must be NACKed (TTL=0). Without - // this arm a client probing for a v2 service - // against a v1 server would get an Ack and start - // sending traffic that the application stack - // would silently mis-decode. - tracing::warn!( - "Subscribe for wrong major_version: expected {}, got {}", - self.config.major_version, - entry_view.major_version() - ); - if let Err(e) = self - .send_subscribe_nack_from_view( - &entry_view, - sender, - "wrong_major_version", - ) - .await - { - tracing::warn!(error = %e, "SubscribeNack send failed"); - } - } else if !self.config.accepts_event_group(entry_view.event_group_id()) { - // Per AUTOSAR SOME/IP-SD, the event group must - // be known to the server before subscription - // can be granted. If `event_group_ids` is - // populated and the request is for an - // unrecognised group, NACK so the client - // doesn't believe it's subscribed. - tracing::warn!( - "Subscribe for unknown event_group_id 0x{:04X} (service 0x{:04X})", - entry_view.event_group_id(), - entry_view.service_id() - ); - if let Err(e) = self - .send_subscribe_nack_from_view( - &entry_view, - sender, - "unknown_event_group", - ) - .await - { - tracing::warn!(error = %e, "SubscribeNack send failed"); - } - } else { - // Extract the subscriber endpoint from the entry's - // own options run. Each SD entry describes two runs - // of options via `(index_first_options_run, - // first_options_count)` and the symmetric second - // pair; we walk both runs, collect every - // `IpV4Endpoint` option in them, and take the first. - let first_index = entry_view.index_first_options_run() as usize; - let first_count = entry_view.options_count().first_options_count as usize; - let second_index = entry_view.index_second_options_run() as usize; - let second_count = entry_view.options_count().second_options_count as usize; - if let Some(endpoint_addr) = extract_subscriber_endpoint( - &sd_view.options(), - first_index, - first_count, - second_index, - second_count, - ) { - let subscribe_result = self - .subscriptions - .subscribe( - entry_view.service_id(), - entry_view.instance_id(), - entry_view.event_group_id(), - endpoint_addr, - ) - .await; - - match subscribe_result { - Ok(()) => { - // ACK the just-committed subscription. If the - // ACK send fails (transient transport error), - // roll back the subscription so we don't leak - // a committed-but-unacked entry — and log - // rather than propagate, so a single SD-socket - // hiccup doesn't tear down `run()`. - if let Err(e) = - self.send_subscribe_ack_from_view(&entry_view, sender).await - { - tracing::warn!( - error = %e, - service_id = entry_view.service_id(), - instance_id = entry_view.instance_id(), - event_group_id = entry_view.event_group_id(), - "SubscribeAck send failed; rolling back subscription" - ); - self.subscriptions - .unsubscribe( - entry_view.service_id(), - entry_view.instance_id(), - entry_view.event_group_id(), - endpoint_addr, - ) - .await; - } - } - Err(e) => { - // Capacity-rejected subscription: NACK so - // the client doesn't believe it's - // subscribed. - let reason: &'static str = match e { - SubscribeError::SubscribersPerGroupFull => { - "subscribers_per_group_full" - } - SubscribeError::EventGroupsFull => "event_groups_full", - }; - tracing::debug!("Subscription rejected: {reason}"); - if let Err(e) = self - .send_subscribe_nack_from_view(&entry_view, sender, reason) - .await - { - tracing::warn!(error = %e, "SubscribeNack send failed"); - } - } - } - } else { - tracing::warn!("No endpoint found in Subscribe message options"); - if let Err(e) = self - .send_subscribe_nack_from_view( - &entry_view, - sender, - "no_endpoint_in_options", - ) - .await - { - tracing::warn!(error = %e, "SubscribeNack send failed"); - } - } - } - } - sd::EntryType::FindService => { - let find_service_id = entry_view.service_id(); - // Check if this FindService is for our service (or wildcard 0xFFFF) - if find_service_id == self.config.service_id || find_service_id == 0xFFFF { - tracing::debug!( - "Received FindService from {} for service 0x{:04X} (ours: 0x{:04X}), sending unicast offer", - sender, - find_service_id, - self.config.service_id - ); - if let Err(e) = self.send_unicast_offer(sender).await { - tracing::warn!(error = %e, "Unicast OfferService send failed"); - } - } else { - tracing::trace!( - "Ignoring FindService for service 0x{:04X} (not ours)", - find_service_id - ); - } - } - _ => { - tracing::trace!("Ignoring SD entry type: {:?}", entry_type); - } - } - } - - Ok(()) - } -} - -/// Convert a [`core::net::SocketAddr`] into a [`SocketAddrV4`] for the -/// transport layer. SOME/IP-SD is IPv4-only at this layer; if a V6 -/// address ever surfaces here it indicates a misconfiguration upstream -/// (a V6 socket binding the SD port, or a V6 source address surfaced -/// by a transport that should not produce one). Returns -/// [`TransportError::Unsupported`](crate::transport::TransportError::Unsupported) -/// in that case so the caller can log and drop the message instead of panicking. -fn socket_addr_v4(addr: core::net::SocketAddr) -> Result { - match addr { - core::net::SocketAddr::V4(v4) => Ok(v4), - core::net::SocketAddr::V6(_) => Err(Error::Transport( - crate::transport::TransportError::Unsupported, - )), - } -} - -/// Extract a single subscriber endpoint from the options runs associated with -/// an SD entry. Walks both option runs, returns the first `IpV4Endpoint` -/// found, and logs a `warn!` if more than one is present. -fn extract_subscriber_endpoint( - options: &sd::OptionIter<'_>, - first_index: usize, - first_count: usize, - second_index: usize, - second_count: usize, -) -> Option { - let mut first_endpoint: Option = None; - let mut endpoint_count: usize = 0; - let mut ignored_other: usize = 0; - - let mut walk_run = |index: usize, count: usize| { - if count == 0 { - return; - } - for option_view in options.clone().skip(index).take(count) { - match option_view.option_type() { - Ok(sd::OptionType::IpV4Endpoint) => { - if let Ok((ip, _, port)) = option_view.as_ipv4() { - endpoint_count += 1; - if first_endpoint.is_none() { - first_endpoint = Some(SocketAddrV4::new(ip, port)); - } - } - } - Ok(_) | Err(_) => ignored_other += 1, - } - } - }; - - walk_run(first_index, first_count); - walk_run(second_index, second_count); - - match endpoint_count { - 0 => { - tracing::warn!( - "No IPv4 endpoint in options runs \ - (first: idx={first_index}, count={first_count}; \ - second: idx={second_index}, count={second_count}; \ - ignored={ignored_other})" - ); - None - } - 1 => { - let ep = first_endpoint.expect("endpoint_count=1 implies first_endpoint is Some"); - tracing::trace!("Found IPv4 endpoint {}", ep); - Some(ep) - } - n => { - let ep = first_endpoint.expect("endpoint_count>=1 implies first_endpoint is Some"); - tracing::warn!( - "{} IPv4 endpoints found in subscribe options runs; \ - using first ({}) and ignoring {} additional. \ - Multi-endpoint (e.g. TCP+UDP) subscribers are not yet supported.", - n, - ep, - n - 1 - ); - Some(ep) - } - } -} - -impl Server -where - R: E2ERegistryHandle, - S: SubscriptionHandle, - F: TransportFactory + 'static, - F::Socket: 'static, - Tm: Timer + Clone + 'static, - H: SharedHandle, - Hsd: SharedHandle, - Hep: SharedHandle>, -{ - /// Send `SubscribeAck` from an entry view - async fn send_subscribe_ack_from_view( + pub fn run( &self, - entry_view: &sd::EntryView<'_>, - subscriber: core::net::SocketAddr, - ) -> Result<(), Error> { - use crate::protocol::Header as SomeIpHeader; - use crate::traits::WireFormat; - - let ack_entry = Entry::SubscribeAckEventGroup(sd::EventGroupEntry { - index_first_options_run: 0, - index_second_options_run: 0, - options_count: OptionsCount::new(0, 0), - service_id: entry_view.service_id(), - instance_id: entry_view.instance_id(), - major_version: entry_view.major_version(), - ttl: self.config.ttl, - counter: entry_view.counter(), - event_group_id: entry_view.event_group_id(), - }); - - let entries = [ack_entry]; - // Atomic (sid, reboot_flag) pair — see - // `SdStateManager::next_session_id_with_reboot_flag`. - let (sid, reboot_flag) = self.sd_state.get().next_session_id_with_reboot_flag(); - let sd_payload = sd::Header::new(Flags::new_sd(reboot_flag), &entries, &[]); - - let mut buffer = [0u8; crate::UDP_BUFFER_SIZE]; - let sd_data_len = sd_payload.encode_to_slice(&mut buffer[16..])?; - let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); - someip_header.encode_to_slice(&mut buffer[..16])?; - let total_len = 16 + sd_data_len; - - let subscriber_v4 = socket_addr_v4(subscriber)?; - self.sd_socket - .get() - .send_to(&buffer[..total_len], subscriber_v4) - .await?; - - tracing::debug!( - "Sent SubscribeAck to {} for service 0x{:04X}, eventgroup 0x{:04X}", - subscriber, - entry_view.service_id(), - entry_view.event_group_id() - ); - - Ok(()) + ) -> impl core::future::Future> + + Send + + 'static + + use + where + F: Send + Sync, + F::Socket: Send + Sync, + for<'a> ::SendFuture<'a>: Send, + for<'a> ::RecvFuture<'a>: Send, + H: Send + Sync, + Sub: Send + Sync, + for<'a> Sub::SubscribeFuture<'a>: Send, + for<'a> Sub::UnsubscribeFuture<'a>: Send, + R: Send + Sync, + Tm: Send + Sync, + for<'a> Tm::SleepFuture<'a>: Send, + Hsd: Send + Sync, + Hep: Send + Sync, + { + self.run_inner() } - /// Send `SubscribeNack` from an entry view - async fn send_subscribe_nack_from_view( + /// Auto-trait-inferred run-future used by the constructors and by + /// the `Send`-requiring [`Self::run`] convenience above. Private + /// because it exposes `Send`-or-not as an inference rather than a + /// declared bound — callers should prefer `run` (Send-checked at + /// the API boundary) or `run_with_buffers` (explicitly no `Send` + /// requirement). + fn run_inner( &self, - entry_view: &sd::EntryView<'_>, - subscriber: core::net::SocketAddr, - reason: &str, - ) -> Result<(), Error> { - use crate::protocol::Header as SomeIpHeader; - use crate::traits::WireFormat; - - let nack_entry = Entry::SubscribeAckEventGroup(sd::EventGroupEntry { - index_first_options_run: 0, - index_second_options_run: 0, - options_count: OptionsCount::new(0, 0), - service_id: entry_view.service_id(), - instance_id: entry_view.instance_id(), - major_version: entry_view.major_version(), - ttl: 0, // TTL=0 indicates NACK - counter: entry_view.counter(), - event_group_id: entry_view.event_group_id(), - }); - - let entries = [nack_entry]; - // Atomic (sid, reboot_flag) pair — see - // `SdStateManager::next_session_id_with_reboot_flag`. - let (sid, reboot_flag) = self.sd_state.get().next_session_id_with_reboot_flag(); - let sd_payload = sd::Header::new(Flags::new_sd(reboot_flag), &entries, &[]); - - let mut buffer = [0u8; crate::UDP_BUFFER_SIZE]; - let sd_data_len = sd_payload.encode_to_slice(&mut buffer[16..])?; - let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); - someip_header.encode_to_slice(&mut buffer[..16])?; - let total_len = 16 + sd_data_len; - - let subscriber_v4 = socket_addr_v4(subscriber)?; - self.sd_socket - .get() - .send_to(&buffer[..total_len], subscriber_v4) - .await?; - - tracing::warn!( - "Sent SubscribeNack to {} for service 0x{:04X}, eventgroup 0x{:04X} (reason: {})", - subscriber, - entry_view.service_id(), - entry_view.event_group_id(), - reason - ); + ) -> impl core::future::Future> + + 'static + + use { + let config = self.config.clone(); + let unicast_socket = self.unicast_socket.clone(); + let sd_socket = self.sd_socket.clone(); + let subscriptions = self.subscriptions.clone(); + let sd_state = self.sd_state.clone(); + let timer = self.timer.clone(); + let is_passive = self.is_passive; + let started = self.started.clone(); + + async move { + // First-poll latch — guards against a caller spawning + // both the constructor's run-future *and* a fresh + // `server.run()` / `server.run_with_buffers()`. Two + // concurrent receive loops would race on the same SD / + // unicast sockets and the SD session counter; reject the + // second one rather than silently corrupt wire output. + if started + .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) + .is_err() + { + crate::log::warn!( + "Server::run already started for service 0x{:04X}; \ + a second run-future cannot share the same sockets \ + and session counter", + config.service_id + ); + return Err(Error::InvalidUsage("server_already_running")); + } - Ok(()) + let mut unicast_buf = alloc::vec![0u8; 65535]; + let mut sd_buf = alloc::vec![0u8; 65535]; + runtime::run_combined::( + config, + unicast_socket, + sd_socket, + subscriptions, + sd_state, + timer, + is_passive, + &mut unicast_buf, + &mut sd_buf, + ) + .await + } } } @@ -1582,18 +1312,94 @@ mod tests { /// chasing the four-type-parameter signature on every call site. /// Mirrors the `TestClient` pattern from `tests/client_server.rs`. type TestServer = Server< - Arc>, - Arc>, TokioTransport, TokioTimer, + Arc>, + Arc>, >; #[tokio::test] async fn test_server_creation() { - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30682, 0x5B, 1); + let config = ServerConfig::new(0x5B, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30682); - let server: Result = TestServer::new(config).await; - assert!(server.is_ok()); + let result = TestServer::new(config).await; + assert!(result.is_ok()); + } + + #[test] + fn server_config_builder_chain_overrides_each_field() { + let cfg = ServerConfig::new(0x5B, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30683) + .with_major_version(2) + .with_minor_version(7) + .with_ttl(core::time::Duration::from_secs(10)) + .with_event_group(0x42) + .with_event_group(0x43); + assert_eq!(cfg.interface, Ipv4Addr::LOCALHOST); + assert_eq!(cfg.local_port, 30683); + assert_eq!(cfg.major_version, 2); + assert_eq!(cfg.minor_version, 7); + assert_eq!(cfg.ttl, 10); + assert!(cfg.accepts_event_group(0x42)); + assert!(cfg.accepts_event_group(0x43)); + assert!(!cfg.accepts_event_group(0x44)); + } + + #[test] + fn server_config_with_ttl_truncates_subsecond_precision() { + let cfg = ServerConfig::new(0x5B, 1).with_ttl(core::time::Duration::from_millis(2_999)); + assert_eq!(cfg.ttl, 2, "sub-second is truncated, not rounded"); + } + + /// `announce` defaults to `true` from `ServerConfig::new`, and + /// `with_announce(false)` flips it. The dispatcher topology in + /// `examples/client_server` depends on this default-vs-override + /// being load-bearing — see + /// `with_announce_false_suppresses_offer_service` for the + /// behavioral counterpart that proves the run-future actually + /// honours the flag. + #[test] + fn server_config_with_announce_toggles_field() { + let default_cfg = ServerConfig::new(0x5B, 1); + assert!( + default_cfg.announce, + "announce must default to true so a fresh `ServerConfig` emits SD offers" + ); + + let suppressed = default_cfg.clone().with_announce(false); + assert!(!suppressed.announce, "with_announce(false) must clear the field"); + + let restored = suppressed.with_announce(true); + assert!( + restored.announce, + "with_announce(true) must re-enable after a previous suppression" + ); + } + + #[test] + fn server_config_with_ttl_saturates_overflow() { + let cfg = ServerConfig::new(0x5B, 1) + .with_ttl(core::time::Duration::from_secs(u64::from(u32::MAX) + 1)); + assert_eq!(cfg.ttl, u32::MAX); + } + + #[test] + fn server_config_try_with_event_group_rejects_at_capacity() { + let mut cfg = ServerConfig::new(0x5B, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30684); + for i in 0..u16::try_from(ServerConfig::EVENT_GROUP_IDS_CAP).unwrap() { + cfg = cfg.try_with_event_group(i).expect("under cap"); + } + // One more should be rejected and return the unmodified config. + let cap = ServerConfig::EVENT_GROUP_IDS_CAP; + let result = cfg.try_with_event_group(0xFFFF); + let returned = result.expect_err("at-cap insert must fail"); + assert_eq!(returned.event_group_ids.len(), cap); + assert!(!returned.accepts_event_group(0xFFFF)); } // ── new_with_handles / new_passive_with_handles tests ────────────── @@ -1601,18 +1407,18 @@ mod tests { // These constructors take pre-built socket handles instead of // calling `factory.bind()` themselves, and validate that the // caller-supplied `config.local_port` matches the actual bound - // port (back-fill-only-on-zero, MED-22 in phase 20 cleanup). - // The validation logic only exercises through these tests; the - // production code paths use `new` / `new_with_deps`. + // port (back-fill-only-on-zero). The validation logic only + // exercises through these tests; the production code paths use + // `new` / `new_with_deps`. - /// Build a `ServerHandles<…>` whose unicast socket is bound to + /// Build a `ServerStorage<…>` whose unicast socket is bound to /// the given port (port `0` for ephemeral) and whose other /// fields are the std defaults a tokio consumer would assemble. /// Used by the `new_with_handles` tests below. async fn build_test_handles( unicast_port: u16, ) -> ( - ServerHandles< + ServerStorage< TokioTransport, TokioTimer, Arc>, @@ -1655,7 +1461,7 @@ mod tests { unicast_socket.clone(), e2e_registry.clone(), )); - let handles = ServerHandles { + let handles = ServerStorage { factory, timer: TokioTimer, e2e_registry, @@ -1676,7 +1482,9 @@ mod tests { "test precondition: kernel must assign a real ephemeral port", ); // Port 0 → caller asks for back-fill from the bound port. - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, 0xFE10, 1); + let config = ServerConfig::new(0xFE10, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); let server = TestServer::new_with_handles(handles, config) .expect("new_with_handles must accept local_port = 0"); assert_eq!( @@ -1689,7 +1497,9 @@ mod tests { async fn new_with_handles_accepts_matching_local_port() { let (handles, bound_port) = build_test_handles(0).await; // Caller supplies the matching port explicitly. - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, bound_port, 0xFE11, 1); + let config = ServerConfig::new(0xFE11, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(bound_port); let server = TestServer::new_with_handles(handles, config) .expect("matching local_port must be accepted"); assert_eq!(server.config.local_port, bound_port); @@ -1705,7 +1515,9 @@ mod tests { // distinct from `bound_port`. let bogus_port = bound_port.wrapping_add(1); assert_ne!(bogus_port, bound_port); - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, bogus_port, 0xFE12, 1); + let config = ServerConfig::new(0xFE12, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(bogus_port); let result = TestServer::new_with_handles(handles, config); match result { Err(Error::InvalidUsage(tag)) => { @@ -1723,7 +1535,9 @@ mod tests { #[tokio::test] async fn new_passive_with_handles_back_fills_local_port_on_zero() { let (handles, bound_port) = build_test_handles(0).await; - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, 0xFE13, 1); + let config = ServerConfig::new(0xFE13, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); let server = TestServer::new_passive_with_handles(handles, config) .expect("new_passive_with_handles must accept local_port = 0"); assert_eq!(server.config.local_port, bound_port); @@ -1735,7 +1549,9 @@ mod tests { let (handles, bound_port) = build_test_handles(0).await; let bogus_port = bound_port.wrapping_add(1); assert_ne!(bogus_port, bound_port); - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, bogus_port, 0xFE14, 1); + let config = ServerConfig::new(0xFE14, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(bogus_port); let result = TestServer::new_passive_with_handles(handles, config); match result { Err(Error::InvalidUsage(tag)) => { @@ -1752,8 +1568,10 @@ mod tests { #[tokio::test] async fn passive_server_run_with_buffers_returns_invalid_usage() { let (handles, _) = build_test_handles(0).await; - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, 0xFE15, 1); - let mut server = + let config = ServerConfig::new(0xFE15, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); + let server = TestServer::new_passive_with_handles(handles, config).expect("passive ctor"); let mut unicast_buf = vec![0u8; 1500]; let mut sd_buf = vec![0u8; 1500]; @@ -1766,23 +1584,11 @@ mod tests { } } - /// Same short-circuit on the announcement-loop side. - #[tokio::test] - async fn passive_server_announcement_loop_returns_invalid_usage() { - let (handles, _) = build_test_handles(0).await; - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, 0xFE16, 1); - let server = TestServer::new_passive_with_handles(handles, config).expect("passive ctor"); - // The success arm returns an opaque `impl Future` that - // doesn't impl Debug, so we can't pattern-match on a - // `Result` directly with `{:?}`. Discriminate explicitly. - match server.announcement_loop() { - Err(Error::InvalidUsage(tag)) => { - assert_eq!(tag, "passive_server_announcement_loop"); - } - Err(other) => panic!("expected InvalidUsage, got {other:?}"), - Ok(_) => panic!("passive server's announcement_loop must error"), - } - } + // No standalone `passive_server_announcement_loop` test: the + // announcement loop is folded into the combined [`Server::run`] + // future, so the only entry point that can short-circuit on a + // passive server is `run_with_buffers` (covered by + // `passive_server_run_with_buffers_returns_invalid_usage` above). /// Regression for H5: `ServerConfig::accepts_event_group` must /// accept any group when `event_group_ids` is empty (back-compat: @@ -1790,7 +1596,9 @@ mod tests { /// working) and validate strictly when populated. #[test] fn server_config_accepts_event_group_empty_means_any() { - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30490, 0x5B, 1); + let config = ServerConfig::new(0x5B, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30490); assert!(config.event_group_ids.is_empty()); // Empty list: every group accepted. assert!(config.accepts_event_group(0x0001)); @@ -1800,7 +1608,9 @@ mod tests { #[test] fn server_config_accepts_event_group_populated_validates() { - let mut config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30490, 0x5B, 1); + let mut config = ServerConfig::new(0x5B, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30490); config.event_group_ids.push(0x0001).unwrap(); config.event_group_ids.push(0x0042).unwrap(); assert!(config.accepts_event_group(0x0001)); @@ -1893,13 +1703,18 @@ mod tests { e2e_registry: Arc::new(Mutex::new(E2ERegistry::new())), subscriptions: subscriptions.clone(), }; - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, 0x5B, 1); + let config = ServerConfig::new(0x5B, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); // Explicit `Arc` H so the compiler doesn't have // to invent it across the deps-bundle indirection. - let mut server: Server<_, _, _, _, Arc> = - Server::new_with_deps(deps, config, false) - .await - .expect("create failing-socket server"); + let (server, _handles, _run): ( + Server<_, _, _, _, Arc>, + _, + _, + ) = Server::new_with_deps(deps, config, false) + .await + .expect("create failing-socket server"); // Build a valid Subscribe; our service id/instance/major // match the config's defaults, so the only failure point @@ -1920,7 +1735,7 @@ mod tests { // The H3 fix: handle_sd_message must NOT bubble the ACK send // failure as Err — it logs and continues. - let result = server.handle_sd_message(&sd_view, sender).await; + let result = runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, sender).await; assert!( result.is_ok(), "handle_sd_message must not propagate transient SD-socket I/O errors; got {result:?}" @@ -1936,30 +1751,13 @@ mod tests { ); } - /// Regression for H4: `announcement_loop` must be idempotent. - /// Calling it a second time returns `Err(Error::Io(InvalidInput))` - /// so two announcement futures cannot race on the same SD socket - /// and session counter. - #[tokio::test] - async fn announcement_loop_second_call_returns_invalid_input() { - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30683, 0x5BB4, 1); - let server = TestServer::new(config).await.expect("create server"); - let _first = server - .announcement_loop() - .expect("first announcement_loop call must succeed"); - let second = server.announcement_loop(); - match second { - Err(Error::InvalidUsage(tag)) => { - assert_eq!(tag, "announcement_loop_already_started"); - } - Ok(_) => panic!("second announcement_loop must error, got Ok"), - Err(other) => { - panic!( - "expected Error::InvalidUsage(\"announcement_loop_already_started\"), got {other:?}" - ) - } - } - } + // No standalone `announcement_loop` method: the announcement + // loop is folded into the single combined run-future, so there + // is only one entry point. (The previous + // `announcement_loop_started: AtomicBool` latch existed because + // two independently-spawned announcement futures would race on + // the SD socket / session counter; that failure mode is now + // structurally impossible.) #[tokio::test] async fn test_server_creation_with_loopback_enabled() { @@ -1967,9 +1765,11 @@ mod tests { // when the test binary runs tests in parallel. The SD socket binds // the SD multicast port (30490) and relies on SO_REUSEPORT, the same // as `test_server_creation`. - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30683, 0x5C, 1); + let config = ServerConfig::new(0x5C, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30683); - let server = TestServer::new_with_loopback(config, true) + let (server, _handles, _run) = TestServer::new_with_loopback(config, true) .await .expect("new_with_loopback(true) should succeed on localhost"); @@ -2015,16 +1815,19 @@ mod tests { /// Helper: create a server on an ephemeral port and return (Server, port) async fn create_test_server(service_id: u16, instance_id: u16) -> (TestServer, u16) { // Use port 0 to get an ephemeral port - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, service_id, instance_id); - let mut server = TestServer::new(config) + let config = ServerConfig::new(service_id, instance_id) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); + let (server, _handles, _run) = TestServer::new(config) .await .expect("Failed to create server"); + // Constructor already back-filled `config.local_port` from the + // kernel-assigned bound port; just read it back via + // `unicast_local_addr` for the test return. let port = match server.unicast_local_addr().unwrap() { core::net::SocketAddr::V4(addr) => addr.port(), core::net::SocketAddr::V6(_) => panic!("expected IPv4 address"), }; - // Update config to reflect actual bound port - server.set_local_port(port); (server, port) } @@ -2063,7 +1866,7 @@ mod tests { #[tokio::test] async fn test_subscribe_ack_success() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; // Create a client socket to send subscription and receive response let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); @@ -2094,7 +1897,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); // Check subscription was added let subs = server.subscriptions.read().await; @@ -2121,7 +1924,7 @@ mod tests { #[tokio::test] async fn test_subscribe_nack_wrong_service() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let message = make_subscription_header( @@ -2148,7 +1951,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); // No subscription should have been added let subs = server.subscriptions.read().await; @@ -2173,7 +1976,7 @@ mod tests { #[tokio::test] async fn test_subscribe_nack_wrong_instance() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let message = make_subscription_header( @@ -2199,7 +2002,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); let subs = server.subscriptions.read().await; assert_eq!(subs.subscription_count(), 0); @@ -2222,7 +2025,7 @@ mod tests { #[tokio::test] async fn test_find_service_sends_unicast_offer() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); // Send a FindService for 0x5B @@ -2248,7 +2051,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); }); // Receive the unicast OfferService response @@ -2275,7 +2078,7 @@ mod tests { #[tokio::test] async fn test_find_service_wildcard() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); // Send wildcard FindService (0xFFFF) @@ -2300,7 +2103,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); }); let mut resp_buf = vec![0u8; 65535]; @@ -2324,7 +2127,7 @@ mod tests { #[tokio::test] async fn test_find_service_wrong_service_ignored() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); // Send FindService for 0x99 (not our service) @@ -2349,7 +2152,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); }); // Should NOT receive any response (short timeout) @@ -2369,7 +2172,7 @@ mod tests { #[tokio::test] async fn test_subscribe_nack_no_endpoint() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); // Build a SubscribeEventGroup with NO endpoint option @@ -2391,7 +2194,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); // No subscription should have been added let subs = server.subscriptions.read().await; @@ -2422,10 +2225,14 @@ mod tests { let recv_addr = receiver.local_addr().unwrap(); let (server, _) = create_test_server(0x5B, 1).await; - server - .send_unicast_offer(recv_addr) - .await - .expect("send_unicast_offer failed"); + runtime::send_unicast_offer( + &server.config, + server.sd_socket.get(), + server.sd_state.get(), + recv_addr, + ) + .await + .expect("send_unicast_offer failed"); // Receive and parse the offer let mut buf = vec![0u8; 65535]; @@ -2446,24 +2253,19 @@ mod tests { assert_eq!(entry.service_id(), 0x5B); assert_eq!(entry.instance_id(), 1); - // Also test that announcement_loop builds a future without error. + // Announcements are folded into `Server::run`. Verify a + // fresh server can build its combined run-future without + // error; intentionally do not poll or spawn it (would loop + // indefinitely emitting multicast). drop(server); let (server2, _) = create_test_server(0x5B, 1).await; - let fut = server2 - .announcement_loop() - .expect("announcement_loop on a regular server must build"); - // Intentionally do not poll or spawn the future: we only care - // that constructing it returned Ok. If this future were - // spawned, the announcer would loop indefinitely and emit - // multicast until explicitly aborted or the Tokio runtime - // shut down at end-of-test, which could interfere with - // parallel tests using the same multicast group. + let fut = server2.run(); drop(fut); } #[tokio::test] async fn test_run_non_sd_message() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let client_port = match client_socket.local_addr().unwrap() { core::net::SocketAddr::V4(a) => a.port(), @@ -2532,7 +2334,7 @@ mod tests { #[tokio::test] async fn test_run_malformed_data() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let client_port = match client_socket.local_addr().unwrap() { core::net::SocketAddr::V4(a) => a.port(), @@ -2589,7 +2391,7 @@ mod tests { #[tokio::test] async fn test_handle_sd_other_entry_type() { - let (mut server, _) = create_test_server(0x5B, 1).await; + let (server, _) = create_test_server(0x5B, 1).await; // Build SD message with a StopOfferService entry (not handled by server) let entry = sd::Entry::StopOfferService(sd::ServiceEntry { @@ -2611,15 +2413,21 @@ mod tests { let sd_view = sd::SdHeaderView::parse(&buf[..n]).unwrap(); // Should not panic or error - let result = server - .handle_sd_message(&sd_view, "127.0.0.1:12345".parse().unwrap()) - .await; + let result = runtime::handle_sd_message( + &server.config, + server.sd_socket.get(), + server.sd_state.get(), + &server.subscriptions, + &sd_view, + "127.0.0.1:12345".parse().unwrap(), + ) + .await; assert!(result.is_ok()); } #[tokio::test] async fn test_subscribe_ack_different_endpoint_port() { - let (mut server, server_port) = create_test_server(0x5B, 1).await; + let (server, server_port) = create_test_server(0x5B, 1).await; let client_socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let message = make_subscription_header( @@ -2645,7 +2453,7 @@ mod tests { let data = &buf[..len]; let view = MessageView::parse(data).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, addr).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, addr).await.unwrap(); // Subscription should have been added let subs = server.subscriptions.read().await; @@ -2718,7 +2526,7 @@ mod tests { let total = fill_ipv4_endpoints(&mut buf, 1, 30000); let iter = sd::OptionIter::new(&buf[..total]); - let got = extract_subscriber_endpoint(&iter, 0, 1, 0, 0); + let got = runtime::extract_subscriber_endpoint(&iter, 0, 1, 0, 0); assert_eq!( got, Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 30000)) @@ -2728,7 +2536,7 @@ mod tests { #[test] fn extract_endpoint_zero_options_in_both_runs_returns_none() { let iter = sd::OptionIter::new(&[]); - assert_eq!(extract_subscriber_endpoint(&iter, 0, 0, 0, 0), None); + assert_eq!(runtime::extract_subscriber_endpoint(&iter, 0, 0, 0, 0), None); } #[test] @@ -2740,7 +2548,7 @@ mod tests { let total = fill_ipv4_endpoints(&mut buf, 2, 30100); let iter = sd::OptionIter::new(&buf[..total]); - assert_eq!(extract_subscriber_endpoint(&iter, 1, 0, 0, 0), None); + assert_eq!(runtime::extract_subscriber_endpoint(&iter, 1, 0, 0, 0), None); } #[test] @@ -2752,7 +2560,7 @@ mod tests { let total = fill_ipv4_endpoints(&mut buf, 2, 30200); let iter = sd::OptionIter::new(&buf[..total]); - let got = extract_subscriber_endpoint(&iter, 0, 2, 0, 0); + let got = runtime::extract_subscriber_endpoint(&iter, 0, 2, 0, 0); assert_eq!( got, Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 30200)) @@ -2771,7 +2579,7 @@ mod tests { let total = fill_ipv4_endpoints(&mut buf, 3, 30300); let iter = sd::OptionIter::new(&buf[..total]); - let got = extract_subscriber_endpoint(&iter, 0, 1, 2, 1); + let got = runtime::extract_subscriber_endpoint(&iter, 0, 1, 2, 1); assert_eq!( got, Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 30300)) @@ -2786,7 +2594,7 @@ mod tests { let total = fill_ipv4_endpoints(&mut buf, 4, 30400); let iter = sd::OptionIter::new(&buf[..total]); - let got = extract_subscriber_endpoint(&iter, 2, 1, 0, 0); + let got = runtime::extract_subscriber_endpoint(&iter, 2, 1, 0, 0); assert_eq!( got, Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 30402)) @@ -2802,7 +2610,7 @@ mod tests { let iter = sd::OptionIter::new(&buf[..total]); // Take only 1 option starting at index 1 -> port 30501. - let got = extract_subscriber_endpoint(&iter, 1, 1, 0, 0); + let got = runtime::extract_subscriber_endpoint(&iter, 1, 1, 0, 0); assert_eq!( got, Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 30501)) @@ -2826,7 +2634,7 @@ mod tests { offset += write_load_balancing_option(&mut buf[offset..], 3, 4); let iter = sd::OptionIter::new(&buf[..offset]); - let got = extract_subscriber_endpoint(&iter, 0, 3, 0, 0); + let got = runtime::extract_subscriber_endpoint(&iter, 0, 3, 0, 0); assert_eq!( got, Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 30600)) @@ -2841,7 +2649,7 @@ mod tests { offset += write_load_balancing_option(&mut buf[offset..], 3, 4); let iter = sd::OptionIter::new(&buf[..offset]); - assert_eq!(extract_subscriber_endpoint(&iter, 0, 2, 0, 0), None); + assert_eq!(runtime::extract_subscriber_endpoint(&iter, 0, 2, 0, 0), None); } #[test] @@ -2852,7 +2660,7 @@ mod tests { let total = fill_ipv4_endpoints(&mut buf, 2, 30700); let iter = sd::OptionIter::new(&buf[..total]); - let got = extract_subscriber_endpoint(&iter, 0, 0, 1, 1); + let got = runtime::extract_subscriber_endpoint(&iter, 0, 0, 1, 1); assert_eq!( got, Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 30701)) @@ -2872,7 +2680,7 @@ mod tests { /// wrong endpoint. #[tokio::test] async fn combined_sd_subscribe_uses_its_own_options_run() { - let (mut server, _port) = create_test_server(0x5B, 1).await; + let (server, _port) = create_test_server(0x5B, 1).await; let offer_endpoint_port: u16 = 40_111; let subscribe_endpoint_port: u16 = 40_222; @@ -2942,7 +2750,7 @@ mod tests { let sender = core::net::SocketAddr::V4(datagram.source); let view = MessageView::parse(&buf[..len]).unwrap(); let sd_view = view.sd_header().unwrap(); - server.handle_sd_message(&sd_view, sender).await.unwrap(); + runtime::handle_sd_message(&server.config, server.sd_socket.get(), server.sd_state.get(), &server.subscriptions, &sd_view, sender).await.unwrap(); // The server must have registered exactly one subscriber, and // its endpoint must be the SubscribeEventGroup entry's options[1] @@ -2978,10 +2786,13 @@ mod tests { /// Construct a passive server on loopback with an ephemeral unicast /// port. Tests use this as a standard fixture. async fn make_passive_server(service_id: u16, instance_id: u16) -> TestServer { - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, service_id, instance_id); - TestServer::new_passive(config) + let config = ServerConfig::new(service_id, instance_id) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); + let (server, _handles, _run) = TestServer::new_passive(config) .await - .expect("new_passive should succeed") + .expect("new_passive should succeed"); + server } #[tokio::test] @@ -3041,26 +2852,14 @@ mod tests { assert!(!publisher.has_subscribers(0x005C, 0x0001, 0x0001).await); } - #[tokio::test] - async fn announcement_loop_on_passive_returns_invalid_input() { - let server = make_passive_server(0x005C, 0x0001).await; - let err = server - .announcement_loop() - .err() - .expect("announcement_loop on a passive server must fail"); - match err { - Error::InvalidUsage(tag) => { - assert_eq!(tag, "passive_server_announcement_loop"); - } - other => panic!( - "expected Error::InvalidUsage(\"passive_server_announcement_loop\"), got {other:?}" - ), - } - } + // The announcement loop is folded into the combined + // `Server::run` future, so the `is_passive` check happens on + // `run` itself — exercised by + // `run_on_passive_returns_invalid_input` below. #[tokio::test] async fn run_on_passive_returns_invalid_input() { - let mut server = make_passive_server(0x005C, 0x0001).await; + let server = make_passive_server(0x005C, 0x0001).await; let err = server .run() .await @@ -3074,23 +2873,64 @@ mod tests { } #[tokio::test] - async fn announcement_loop_on_regular_server_still_succeeds() { - // Regression guard: the is_passive check must not break the - // standard non-passive path. + async fn run_on_regular_server_builds_future_ok() { + // Regression guard: the combined run-future must build + // without error on a non-passive server. We don't poll or + // spawn — doing so would leave the run-loop emitting + // multicast for the rest of the test binary's lifetime and + // interfere with parallel tests that share the SD multicast + // group. let (server, _port) = create_test_server(0x005C, 0x0001).await; - let fut = server - .announcement_loop() - .expect("announcement_loop on a regular server must build"); - // The announcer loops forever; the test succeeds as soon as - // construction returns Ok. - // Do not poll or spawn the future: doing so would leave the - // announcer running and emitting multicast for the rest of the - // test binary's lifetime, interfering with parallel tests that - // bind the same multicast group. We only care that construction - // returned Ok, so drop the future without polling it. + let fut = server.run(); drop(fut); } + /// Two run-futures from the same `Server` would race on the SD + /// and unicast sockets and the SD session counter; the second to + /// be polled must short-circuit with + /// `Err(Error::InvalidUsage("server_already_running"))` rather + /// than silently corrupt wire output. Tests both ordering and + /// the buffer-supplied variant. + #[tokio::test] + async fn second_run_future_returns_already_running() { + let (server, _port) = create_test_server(0x005D, 0x0001).await; + + // First run-future: spawn it so its async-move body actually + // runs and flips the latch on first poll. Yield once so tokio + // schedules the spawned task; the task itself blocks + // indefinitely in `recv_from`, which is fine — abort below. + let first = tokio::spawn(server.run()); + tokio::task::yield_now().await; + tokio::task::yield_now().await; + + // Second run-future from the same server must reject. + let second = server.run().await; + match second { + Err(Error::InvalidUsage(tag)) => { + assert_eq!(tag, "server_already_running"); + } + other => panic!( + "second run-future must return InvalidUsage(\"server_already_running\"), got {other:?}" + ), + } + + // Same gate on `run_with_buffers`. + let mut unicast_buf = vec![0u8; 1500]; + let mut sd_buf = vec![0u8; 1500]; + let third = server.run_with_buffers(&mut unicast_buf, &mut sd_buf).await; + match third { + Err(Error::InvalidUsage(tag)) => { + assert_eq!(tag, "server_already_running"); + } + other => panic!( + "second run_with_buffers must return InvalidUsage(\"server_already_running\"), got {other:?}" + ), + } + + first.abort(); + let _ = first.await; + } + /// Direct test that `announcement_loop` actually emits an SD /// announcement when driven. Explicit coverage for the primary entry /// point (avoids regressions where only the deleted shim was exercised). @@ -3130,10 +2970,17 @@ mod tests { rs }; - let config = ServerConfig::new(iface, 30501, SID, IID); - let server = TestServer::new_with_loopback(config, true).await.unwrap(); - let fut = server.announcement_loop().expect("build loop"); - let handle = tokio::spawn(fut); + let config = ServerConfig::new(SID, IID) + .with_interface(iface) + .with_local_port(30501); + let (_server, _handles, run) = TestServer::new_with_loopback(config, true).await.unwrap(); + // `Server::run` is the combined receive+announce future. The + // receive arm here just waits for traffic that never arrives + // in this test; the announce arm is what we capture on `recv` + // below. + let handle = tokio::spawn(async move { + let _ = run.await; + }); // Filter out any stray SD traffic from other parallel tests // until we see one whose OfferService entry carries OUR sid/iid. @@ -3184,6 +3031,97 @@ mod tests { handle.abort(); } + /// `ServerConfig::with_announce(false)` is the contract the + /// dispatcher topology relies on (`examples/client_server`). It + /// MUST suppress the announce arm of the combined run-future, + /// even though the receive arm keeps running. This is the + /// negative counterpart to + /// `announcement_loop_sends_offer_service_when_driven` above — + /// same SD-multicast capture machinery, but we assert the listen + /// window expires *without* seeing one of our OfferServices. + #[tokio::test] + async fn with_announce_false_suppresses_offer_service() { + use crate::protocol::MessageId; + + // Distinct (sid, iid) so parallel tests on the same SD multicast + // group don't bleed into our negative assertion. These IDs must + // not appear in any other in-tree test or example. + const SID: u16 = 0xAA02; + const IID: u16 = 0xFF02; + + let iface = std::net::Ipv4Addr::LOCALHOST; + let recv = { + let s = socket2::Socket::new( + socket2::Domain::IPV4, + socket2::Type::DGRAM, + Some(socket2::Protocol::UDP), + ) + .unwrap(); + s.set_reuse_address(true).unwrap(); + #[cfg(unix)] + s.set_reuse_port(true).unwrap(); + s.bind(&core::net::SocketAddr::new(IpAddr::V4(iface), sd::MULTICAST_PORT).into()) + .unwrap(); + s.set_nonblocking(true).unwrap(); + let std_s: std::net::UdpSocket = s.into(); + let rs = tokio::net::UdpSocket::from_std(std_s).unwrap(); + rs.join_multicast_v4(sd::MULTICAST_IP, iface).unwrap(); + rs + }; + + let config = ServerConfig::new(SID, IID) + .with_interface(iface) + .with_local_port(30502) + .with_announce(false); + let (_server, _handles, run) = TestServer::new_with_loopback(config, true).await.unwrap(); + let handle = tokio::spawn(async move { + let _ = run.await; + }); + + // Listen for ~2 seconds — comfortably more than the 1-second + // announcement period the run-future would emit at if announce + // were on. If we see an OfferService for OUR (SID, IID) in this + // window, the suppression is broken. Stray traffic for *other* + // service IDs is ignored (parallel tests share the SD group). + let saw_our_offer = tokio::time::timeout(std::time::Duration::from_millis(2_500), async { + let mut buf = [0u8; 1500]; + loop { + let (n, _src) = recv.recv_from(&mut buf).await.expect("recv failed"); + let Ok(view) = crate::protocol::MessageView::parse(&buf[..n]) else { + continue; + }; + if view.header().message_id() != MessageId::SD { + continue; + } + let Ok(sd_view) = view.sd_header() else { + continue; + }; + let Some(entry) = sd_view.entries().next() else { + continue; + }; + if !matches!(entry.entry_type(), Ok(sd::EntryType::OfferService)) { + continue; + } + if entry.service_id() == SID && entry.instance_id() == IID { + break true; + } + } + }) + .await + .unwrap_or(false); + + handle.abort(); + let _ = handle.await; + + assert!( + !saw_our_offer, + "with_announce(false) must suppress OfferService emission for the configured \ + service; observed an OfferService for (sid={SID:#06x}, iid={IID:#06x}) within \ + the listen window. The dispatcher topology in examples/client_server depends \ + on this suppression." + ); + } + #[tokio::test] async fn new_passive_two_instances_do_not_fight_over_sd_port() { // Two passive servers on the same interface must both construct @@ -3215,7 +3153,9 @@ mod tests { core::net::SocketAddr::V6(_) => panic!("expected IPv4"), }; - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, blocker_port, 0x005C, 0x0001); + let config = ServerConfig::new(0x005C, 0x0001) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(blocker_port); let result = TestServer::new_passive(config).await; let Err(err) = result else { panic!("new_passive must fail when the unicast port is taken"); @@ -3244,8 +3184,8 @@ mod tests { #[tokio::test] async fn new_passive_with_tracing_subscriber_evaluates_format_args() { - // Coverage helper: with no global tracing subscriber, `tracing::info!` - // and `tracing::debug!` short-circuit before evaluating their + // Coverage helper: with no global tracing subscriber, `crate::log::info!` + // and `crate::log::debug!` short-circuit before evaluating their // formatted arguments, leaving the format-arg lines in `new_passive` // marked as uncovered. This test installs a max-level subscriber so // the macros take their full format path and the arg-evaluation @@ -3298,14 +3238,14 @@ mod tests { with_default(subscriber, || { // 0 endpoints → warn! "No IPv4 endpoint" branch. let iter_empty = sd::OptionIter::new(&[]); - assert_eq!(extract_subscriber_endpoint(&iter_empty, 0, 0, 0, 0), None); + assert_eq!(runtime::extract_subscriber_endpoint(&iter_empty, 0, 0, 0, 0), None); // 1 endpoint → trace! "Found IPv4 endpoint" branch. let mut buf_one = [0u8; 32]; let len_one = fill_ipv4_endpoints(&mut buf_one, 1, 31000); let iter_one = sd::OptionIter::new(&buf_one[..len_one]); assert_eq!( - extract_subscriber_endpoint(&iter_one, 0, 1, 0, 0), + runtime::extract_subscriber_endpoint(&iter_one, 0, 1, 0, 0), Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 31000)) ); @@ -3314,7 +3254,7 @@ mod tests { let len_many = fill_ipv4_endpoints(&mut buf_many, 3, 31100); let iter_many = sd::OptionIter::new(&buf_many[..len_many]); assert_eq!( - extract_subscriber_endpoint(&iter_many, 0, 3, 0, 0), + runtime::extract_subscriber_endpoint(&iter_many, 0, 3, 0, 0), Some(SocketAddrV4::new(Ipv4Addr::new(10, 0, 0, 1), 31100)) ); }); @@ -3339,7 +3279,9 @@ mod tests { // Pick a service_id and unicast port that do not collide with // the other loopback-enabled server test in this file. let service_id = 0xFE02; - let config = ServerConfig::new(interface, 30684, service_id, 0x43); + let config = ServerConfig::new(service_id, 0x43) + .with_interface(interface) + .with_local_port(30684); // Receiver joined to the SD multicast group on loopback. let raw_rx = socket2::Socket::new( @@ -3359,13 +3301,13 @@ mod tests { let rx: UdpSocket = UdpSocket::from_std(raw_rx.into()).unwrap(); rx.join_multicast_v4(sd::MULTICAST_IP, interface).unwrap(); - let server = TestServer::new_with_loopback(config, true) + let (_server, _handles, run_fut) = TestServer::new_with_loopback(config, true) .await .expect("server must bind with loopback enabled"); - let announce_fut = server - .announcement_loop() - .expect("announcement_loop should build on a non-passive server"); - let announce_handle = tokio::spawn(announce_fut); + // Announcement is folded into the combined run-future. + let announce_handle = tokio::spawn(async move { + let _ = run_fut.await; + }); // Scan the multicast group for our OfferService. The first tick // happens immediately; 2s is ample headroom for scheduler jitter. diff --git a/src/server/runtime.rs b/src/server/runtime.rs new file mode 100644 index 00000000..4a022ed4 --- /dev/null +++ b/src/server/runtime.rs @@ -0,0 +1,680 @@ +//! Server runtime helpers — free async functions that drive the +//! receive loop, the SD announcement loop, and SD-message handling. +//! +//! These live as free functions (rather than `&self` methods on +//! [`Server`]) so the run-future returned from `Server::new` can be +//! `'static` — built by cloning the cheap shared-handles into an +//! `async move` instead of borrowing whatever `Server` value the +//! caller holds. +//! +//! All functions here take their state by reference; ownership lives +//! in the caller's async-move scope, which is itself constructed by +//! [`Server::run`](super::Server::run) / +//! [`Server::run_with_buffers`](super::Server::run_with_buffers). + +use core::net::SocketAddrV4; + +use futures_util::{FutureExt, future::Either, pin_mut, select_biased}; + +use crate::Timer; +use crate::protocol::sd::{self, Entry, Flags, OptionsCount, ServiceEntry, TransportProtocol}; +use crate::transport::{SharedHandle, TransportSocket}; + +use super::sd_state::SdStateManager; +use super::subscription_manager::{SubscribeError, SubscriptionHandle}; +use super::{Error, ServerConfig}; + +/// Send a unicast `OfferService` to a specific address (typically in +/// response to a `FindService`). +pub(super) async fn send_unicast_offer( + config: &ServerConfig, + sd_socket: &T, + sd_state: &SdStateManager, + target: core::net::SocketAddr, +) -> Result<(), Error> +where + T: TransportSocket, +{ + use crate::protocol::Header as SomeIpHeader; + use crate::traits::WireFormat; + + let entry = Entry::OfferService(ServiceEntry { + index_first_options_run: 0, + index_second_options_run: 0, + options_count: OptionsCount::new(1, 0), + service_id: config.service_id, + instance_id: config.instance_id, + major_version: config.major_version, + ttl: config.ttl, + minor_version: config.minor_version, + }); + + let option = sd::Options::IpV4Endpoint { + ip: config.interface, + port: config.local_port, + protocol: TransportProtocol::Udp, + }; + + let entries = [entry]; + let options = [option]; + let (sid, reboot_flag) = sd_state.next_session_id_with_reboot_flag(); + let sd_payload = sd::Header::new(Flags::new_sd(reboot_flag), &entries, &options); + + let mut buffer = [0u8; crate::UDP_BUFFER_SIZE]; + let sd_data_len = sd_payload.encode_to_slice(&mut buffer[16..])?; + let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); + someip_header.encode_to_slice(&mut buffer[..16])?; + let total_len = 16 + sd_data_len; + + let target_v4 = socket_addr_v4(target)?; + sd_socket.send_to(&buffer[..total_len], target_v4).await?; + crate::log::debug!( + "Sent unicast OfferService to {} for service 0x{:04X}", + target, + config.service_id + ); + + Ok(()) +} + +/// Send `SubscribeAck` derived from a peer's `Subscribe` entry view. +pub(super) async fn send_subscribe_ack_from_view( + config: &ServerConfig, + sd_socket: &T, + sd_state: &SdStateManager, + entry_view: &sd::EntryView<'_>, + subscriber: core::net::SocketAddr, +) -> Result<(), Error> +where + T: TransportSocket, +{ + use crate::protocol::Header as SomeIpHeader; + use crate::traits::WireFormat; + + let ack_entry = Entry::SubscribeAckEventGroup(sd::EventGroupEntry { + index_first_options_run: 0, + index_second_options_run: 0, + options_count: OptionsCount::new(0, 0), + service_id: entry_view.service_id(), + instance_id: entry_view.instance_id(), + major_version: entry_view.major_version(), + ttl: config.ttl, + counter: entry_view.counter(), + event_group_id: entry_view.event_group_id(), + }); + + let entries = [ack_entry]; + let (sid, reboot_flag) = sd_state.next_session_id_with_reboot_flag(); + let sd_payload = sd::Header::new(Flags::new_sd(reboot_flag), &entries, &[]); + + let mut buffer = [0u8; crate::UDP_BUFFER_SIZE]; + let sd_data_len = sd_payload.encode_to_slice(&mut buffer[16..])?; + let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); + someip_header.encode_to_slice(&mut buffer[..16])?; + let total_len = 16 + sd_data_len; + + let subscriber_v4 = socket_addr_v4(subscriber)?; + sd_socket + .send_to(&buffer[..total_len], subscriber_v4) + .await?; + + crate::log::debug!( + "Sent SubscribeAck to {} for service 0x{:04X}, eventgroup 0x{:04X}", + subscriber, + entry_view.service_id(), + entry_view.event_group_id() + ); + + Ok(()) +} + +/// Send `SubscribeNack` (`SubscribeAckEventGroup` with `ttl = 0`). +pub(super) async fn send_subscribe_nack_from_view( + _config: &ServerConfig, + sd_socket: &T, + sd_state: &SdStateManager, + entry_view: &sd::EntryView<'_>, + subscriber: core::net::SocketAddr, + reason: &str, +) -> Result<(), Error> +where + T: TransportSocket, +{ + use crate::protocol::Header as SomeIpHeader; + use crate::traits::WireFormat; + + let nack_entry = Entry::SubscribeAckEventGroup(sd::EventGroupEntry { + index_first_options_run: 0, + index_second_options_run: 0, + options_count: OptionsCount::new(0, 0), + service_id: entry_view.service_id(), + instance_id: entry_view.instance_id(), + major_version: entry_view.major_version(), + ttl: 0, + counter: entry_view.counter(), + event_group_id: entry_view.event_group_id(), + }); + + let entries = [nack_entry]; + let (sid, reboot_flag) = sd_state.next_session_id_with_reboot_flag(); + let sd_payload = sd::Header::new(Flags::new_sd(reboot_flag), &entries, &[]); + + let mut buffer = [0u8; crate::UDP_BUFFER_SIZE]; + let sd_data_len = sd_payload.encode_to_slice(&mut buffer[16..])?; + let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); + someip_header.encode_to_slice(&mut buffer[..16])?; + let total_len = 16 + sd_data_len; + + let subscriber_v4 = socket_addr_v4(subscriber)?; + sd_socket + .send_to(&buffer[..total_len], subscriber_v4) + .await?; + + crate::log::warn!( + "Sent SubscribeNack to {} for service 0x{:04X}, eventgroup 0x{:04X} (reason: {})", + subscriber, + entry_view.service_id(), + entry_view.event_group_id(), + reason + ); + + Ok(()) +} + +/// Handle a Service Discovery message (Subscribe / FindService etc.). +#[allow(clippy::too_many_lines)] +pub(super) async fn handle_sd_message( + config: &ServerConfig, + sd_socket: &T, + sd_state: &SdStateManager, + subscriptions: &Sub, + sd_view: &sd::SdHeaderView<'_>, + sender: core::net::SocketAddr, +) -> Result<(), Error> +where + T: TransportSocket, + Sub: SubscriptionHandle, +{ + crate::log::trace!("Handling SD message from {}", sender); + + for entry_view in sd_view.entries() { + let entry_type = entry_view.entry_type()?; + match entry_type { + sd::EntryType::Subscribe => { + crate::log::debug!( + "Received Subscribe from {}: service=0x{:04X}, instance={}, eventgroup=0x{:04X}", + sender, + entry_view.service_id(), + entry_view.instance_id(), + entry_view.event_group_id() + ); + + if entry_view.service_id() != config.service_id { + crate::log::warn!( + "Subscribe for wrong service: expected 0x{:04X}, got 0x{:04X}", + config.service_id, + entry_view.service_id() + ); + send_subscribe_nack_from_view( + config, + sd_socket, + sd_state, + &entry_view, + sender, + "wrong_service_id", + ) + .await?; + } else if entry_view.instance_id() != config.instance_id { + crate::log::warn!( + "Subscribe for wrong instance: expected {}, got {}", + config.instance_id, + entry_view.instance_id() + ); + send_subscribe_nack_from_view( + config, + sd_socket, + sd_state, + &entry_view, + sender, + "wrong_instance_id", + ) + .await?; + } else if entry_view.major_version() != config.major_version { + crate::log::warn!( + "Subscribe for wrong major_version: expected {}, got {}", + config.major_version, + entry_view.major_version() + ); + if let Err(e) = send_subscribe_nack_from_view( + config, + sd_socket, + sd_state, + &entry_view, + sender, + "wrong_major_version", + ) + .await + { + crate::log::warn!("SubscribeNack send failed: {e}"); + } + } else if !config.accepts_event_group(entry_view.event_group_id()) { + crate::log::warn!( + "Subscribe for unknown event_group_id 0x{:04X} (service 0x{:04X})", + entry_view.event_group_id(), + entry_view.service_id() + ); + if let Err(e) = send_subscribe_nack_from_view( + config, + sd_socket, + sd_state, + &entry_view, + sender, + "unknown_event_group", + ) + .await + { + crate::log::warn!("SubscribeNack send failed: {e}"); + } + } else { + let first_index = entry_view.index_first_options_run() as usize; + let first_count = entry_view.options_count().first_options_count as usize; + let second_index = entry_view.index_second_options_run() as usize; + let second_count = entry_view.options_count().second_options_count as usize; + if let Some(endpoint_addr) = extract_subscriber_endpoint( + &sd_view.options(), + first_index, + first_count, + second_index, + second_count, + ) { + let subscribe_result = subscriptions + .subscribe( + entry_view.service_id(), + entry_view.instance_id(), + entry_view.event_group_id(), + endpoint_addr, + ) + .await; + + match subscribe_result { + Ok(()) => { + if let Err(e) = send_subscribe_ack_from_view( + config, + sd_socket, + sd_state, + &entry_view, + sender, + ) + .await + { + crate::log::warn!( + "SubscribeAck send failed; rolling back subscription \ + (service_id=0x{:04X}, instance_id={}, \ + event_group_id=0x{:04X}, error={e})", + entry_view.service_id(), + entry_view.instance_id(), + entry_view.event_group_id(), + ); + subscriptions + .unsubscribe( + entry_view.service_id(), + entry_view.instance_id(), + entry_view.event_group_id(), + endpoint_addr, + ) + .await; + } + } + Err(e) => { + let reason: &'static str = match e { + SubscribeError::SubscribersPerGroupFull => { + "subscribers_per_group_full" + } + SubscribeError::EventGroupsFull => "event_groups_full", + }; + crate::log::debug!("Subscription rejected: {reason}"); + if let Err(e) = send_subscribe_nack_from_view( + config, + sd_socket, + sd_state, + &entry_view, + sender, + reason, + ) + .await + { + crate::log::warn!("SubscribeNack send failed: {e}"); + } + } + } + } else { + crate::log::warn!("No endpoint found in Subscribe message options"); + if let Err(e) = send_subscribe_nack_from_view( + config, + sd_socket, + sd_state, + &entry_view, + sender, + "no_endpoint_in_options", + ) + .await + { + crate::log::warn!("SubscribeNack send failed: {e}"); + } + } + } + } + sd::EntryType::FindService => { + let find_service_id = entry_view.service_id(); + if find_service_id == config.service_id || find_service_id == 0xFFFF { + crate::log::debug!( + "Received FindService from {} for service 0x{:04X} (ours: 0x{:04X}), sending unicast offer", + sender, + find_service_id, + config.service_id + ); + if let Err(e) = + send_unicast_offer(config, sd_socket, sd_state, sender).await + { + crate::log::warn!("Unicast OfferService send failed: {e}"); + } + } else { + crate::log::trace!( + "Ignoring FindService for service 0x{:04X} (not ours)", + find_service_id + ); + } + } + _ => { + crate::log::trace!("Ignoring SD entry type: {:?}", entry_type); + } + } + } + + Ok(()) +} + +/// Periodic SD `OfferService` announcement loop. Runs forever; intended +/// to be combined with the receive loop via [`run_combined`]. +pub(super) async fn announce_loop( + config: &ServerConfig, + sd_socket: &T, + sd_state: &SdStateManager, + timer: &Tm, +) where + T: TransportSocket, + Tm: Timer, +{ + let mut announcement_count = 0u32; + loop { + match sd_state.send_offer_service(config, sd_socket).await { + Ok(()) => { + announcement_count += 1; + if announcement_count == 1 { + crate::log::info!( + "Sent first SD announcement for service 0x{:04X}", + config.service_id + ); + } else { + crate::log::debug!( + "Sent {} SD announcements for service 0x{:04X}", + announcement_count, + config.service_id + ); + } + } + Err(e) => { + crate::log::error!("Failed to send OfferService: {:?}", e); + } + } + timer.sleep(core::time::Duration::from_secs(1)).await; + } +} + +/// Receive loop body — drives `recv_from` on both the unicast and SD +/// sockets, dispatches SD messages to [`handle_sd_message`]. +async fn recv_loop( + config: &ServerConfig, + unicast_socket: &T, + sd_socket: &T, + sd_state: &SdStateManager, + subscriptions: &Sub, + unicast_buf: &mut [u8], + sd_buf: &mut [u8], +) -> Result<(), Error> +where + T: TransportSocket, + Sub: SubscriptionHandle, +{ + use crate::protocol::MessageView; + + // Iteration counter used to flip `select_biased!` arm priority + // each turn. We can't use the pseudo-random `select!` (it needs + // `std`), so flipping arm order each iteration approximates the + // fairness it would give without pulling std — a sustained + // one-sided load (only-unicast or only-sd) cannot starve the + // other arm. + let mut prefer_sd_first = false; + loop { + // Both arms call `TransportSocket::recv_from`, whose contract + // (see the trait docs) requires the returned future be + // cancel-safe — dropping a non-selected arm must not lose + // in-flight kernel state. The `TokioSocket` backend satisfies + // this; custom backends must too. A future contributor adding + // a non-cancel-safe arm here would silently lose datagrams + // when the arm is dropped on a select win. + // + // Fresh futures are constructed each iteration so the borrows + // of `unicast_buf` / `sd_buf` / the sockets end when the + // select macro returns, freeing the buffer we index into + // below. Each arm returns just `(datagram, from_unicast)`; + // the `(len, addr, source)` derivation lives once below the + // select so the arm-flip pattern doesn't duplicate it. + let (datagram, from_unicast) = { + let unicast_fut = unicast_socket.recv_from(&mut *unicast_buf).fuse(); + let sd_fut = sd_socket.recv_from(&mut *sd_buf).fuse(); + pin_mut!(unicast_fut, sd_fut); + if prefer_sd_first { + select_biased! { + result = sd_fut => (result?, false), + result = unicast_fut => (result?, true), + } + } else { + select_biased! { + result = unicast_fut => (result?, true), + result = sd_fut => (result?, false), + } + } + }; + prefer_sd_first = !prefer_sd_first; + let len = datagram.bytes_received; + let addr = core::net::SocketAddr::V4(datagram.source); + let source = if from_unicast { + "unicast" + } else { + "sd-multicast" + }; + // The `datagram.truncated` flag is currently not surfaced via + // `crate::log::warn!` — backends that report truncation honestly + // (embassy-net today, tokio after #119) won't be observable + // from the server side until #120 lands. + let data = if from_unicast { + &unicast_buf[..len] + } else { + &sd_buf[..len] + }; + + crate::log::trace!("Received {} bytes from {} on {} socket", len, addr, source); + crate::log::trace!("Raw data: {:02X?}", &data[..len.min(64_usize)]); + + match MessageView::parse(data) { + Ok(view) => { + crate::log::trace!( + "SOME/IP Header: service=0x{:04X}, method=0x{:04X}, type={:?}", + view.header().message_id().service_id(), + view.header().message_id().method_id(), + view.header().message_type().message_type() + ); + + if view.is_sd() { + crate::log::trace!("This is an SD message"); + match view.sd_header() { + Ok(sd_view) => { + crate::log::trace!("SD message has {} entries", sd_view.entry_count()); + handle_sd_message( + config, + sd_socket, + sd_state, + subscriptions, + &sd_view, + addr, + ) + .await?; + } + Err(e) => { + crate::log::warn!("Failed to parse SD message: {:?}", e); + } + } + } else { + crate::log::trace!("Non-SD SOME/IP message, ignoring"); + } + } + Err(e) => { + crate::log::warn!("Failed to parse SOME/IP header from {}: {:?}", addr, e); + crate::log::trace!("Data: {:02X?}", &data[..len.min(32)]); + } + } + } +} + +/// Combined receive + announce loop. The single future returned from +/// `Server::new` (and friends) drives this; it is also what +/// [`Server::run_with_buffers`] resolves to once buffers are +/// supplied. +/// +/// Returns `Err(Error::InvalidUsage("passive_server_run"))` if invoked +/// on a passive server (passive servers have no SD socket bound to +/// 30490 and rely on an external SD dispatcher). +/// +/// When `config.announce` is `false`, the announcement arm is skipped +/// and only the receive loop drives — used by the dispatcher topology +/// where a co-located `Client` emits `OfferService` on the server's +/// behalf. +pub(super) async fn run_combined( + config: ServerConfig, + unicast_socket: H, + sd_socket: H, + subscriptions: Sub, + sd_state: Hsd, + timer: Tm, + is_passive: bool, + unicast_buf: &mut [u8], + sd_buf: &mut [u8], +) -> Result<(), Error> +where + H: SharedHandle, + T: TransportSocket + 'static, + Sub: SubscriptionHandle, + Hsd: SharedHandle, + Tm: Timer, +{ + if is_passive { + crate::log::warn!( + "run called on passive Server for service 0x{:04X}; \ + SD receive must be driven externally (e.g. via the \ + Client's discovery socket, routing Subscribes to \ + `EventPublisher::register_subscriber`)", + config.service_id + ); + return Err(Error::InvalidUsage("passive_server_run")); + } + + let unicast = unicast_socket.get(); + let sd = sd_socket.get(); + let sd_state_ref = sd_state.get(); + + let recv_fut = recv_loop(&config, unicast, sd, sd_state_ref, &subscriptions, unicast_buf, sd_buf); + + if config.announce { + let announce_fut = announce_loop(&config, sd, sd_state_ref, &timer); + pin_mut!(recv_fut, announce_fut); + match futures_util::future::select(recv_fut, announce_fut).await { + Either::Left((recv_result, _)) => recv_result, + Either::Right(((), recv_pending)) => recv_pending.await, + } + } else { + recv_fut.await + } +} + +fn socket_addr_v4(addr: core::net::SocketAddr) -> Result { + match addr { + core::net::SocketAddr::V4(v4) => Ok(v4), + core::net::SocketAddr::V6(_) => Err(Error::Transport( + crate::transport::TransportError::Unsupported, + )), + } +} + +pub(super) fn extract_subscriber_endpoint( + options: &sd::OptionIter<'_>, + first_index: usize, + first_count: usize, + second_index: usize, + second_count: usize, +) -> Option { + let mut first_endpoint: Option = None; + let mut endpoint_count: usize = 0; + let mut ignored_other: usize = 0; + + let mut walk_run = |index: usize, count: usize| { + if count == 0 { + return; + } + for option_view in options.clone().skip(index).take(count) { + match option_view.option_type() { + Ok(sd::OptionType::IpV4Endpoint) => { + if let Ok((ip, _, port)) = option_view.as_ipv4() { + endpoint_count += 1; + if first_endpoint.is_none() { + first_endpoint = Some(SocketAddrV4::new(ip, port)); + } + } + } + Ok(_) | Err(_) => ignored_other += 1, + } + } + }; + + walk_run(first_index, first_count); + walk_run(second_index, second_count); + + match endpoint_count { + 0 => { + crate::log::warn!( + "No IPv4 endpoint in options runs \ + (first: idx={first_index}, count={first_count}; \ + second: idx={second_index}, count={second_count}; \ + ignored={ignored_other})" + ); + None + } + 1 => { + let ep = first_endpoint.expect("endpoint_count=1 implies first_endpoint is Some"); + crate::log::trace!("Found IPv4 endpoint {}", ep); + Some(ep) + } + n => { + let ep = first_endpoint.expect("endpoint_count>=1 implies first_endpoint is Some"); + crate::log::warn!( + "{} IPv4 endpoints found in subscribe options runs; \ + using first ({}) and ignoring {} additional. \ + Multi-endpoint (e.g. TCP+UDP) subscribers are not yet supported.", + n, + ep, + n - 1 + ); + Some(ep) + } + } +} diff --git a/src/server/sd_state.rs b/src/server/sd_state.rs index e4d6ae67..3b321f6d 100644 --- a/src/server/sd_state.rs +++ b/src/server/sd_state.rs @@ -227,26 +227,26 @@ impl SdStateManager { let multicast_addr = SocketAddrV4::new(sd::MULTICAST_IP, sd::MULTICAST_PORT); - tracing::trace!( + crate::log::trace!( "Sending OfferService: service=0x{:04X}, instance={}, port={}, size={} bytes", config.service_id, config.instance_id, config.local_port, total_len ); - tracing::trace!("OfferService data: {:02X?}", &buffer[..total_len.min(64)]); + crate::log::trace!("OfferService data: {:02X?}", &buffer[..total_len.min(64)]); socket.send_to(&buffer[..total_len], multicast_addr).await?; - tracing::trace!("Sent to {}", multicast_addr); + crate::log::trace!("Sent to {}", multicast_addr); Ok(()) } } -// Phase 20e collapsed `SdStateHandle` / `WrappableSdStateHandle` -// into the unified `crate::transport::SharedHandle` -// / `WrappableSharedHandle` traits. The blanket -// impls there cover both `&'static SdStateManager` and +// `SdStateHandle` / `WrappableSdStateHandle` were collapsed into the +// unified `crate::transport::SharedHandle` / +// `WrappableSharedHandle` traits. The blanket impls +// there cover both `&'static SdStateManager` and // `Arc`; no dedicated trait survives here. #[cfg(all(test, feature = "server-tokio"))] @@ -575,12 +575,9 @@ mod tests { #[tokio::test] async fn send_offer_service_through_mock_emits_full_someip_sd_envelope() { - let config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); let sd_state = SdStateManager::with_initial(0x1233); let sock = CapturingSocket::new(); @@ -605,12 +602,9 @@ mod tests { #[tokio::test] async fn send_offer_service_through_mock_advances_session_id_across_calls() { - let config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); let sd_state = SdStateManager::with_initial(0x1233); let sock = CapturingSocket::new(); @@ -627,12 +621,9 @@ mod tests { #[tokio::test] async fn send_offer_service_through_mock_reboot_flag_flips_on_wrap() { - let config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); // Seed so the FIRST send takes 0xFFFE → 0xFFFF (still // RecentlyRebooted) and the SECOND sees the wrap to 0x0001 // (Continuous). @@ -665,12 +656,9 @@ mod tests { #[tokio::test] async fn send_offer_service_through_mock_preserves_zero_ttl() { - let mut config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let mut config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); config.ttl = 0; let sd_state = SdStateManager::with_initial(0x1233); let sock = CapturingSocket::new(); @@ -684,12 +672,9 @@ mod tests { #[tokio::test] async fn send_offer_service_through_mock_propagates_socket_errors() { - let config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); let sd_state = SdStateManager::with_initial(0x1233); let sock = FailingSocket; let result = sd_state.send_offer_service(&config, &sock).await; @@ -900,12 +885,9 @@ mod tests { loopback multicast is available."] #[tokio::test] async fn send_offer_service_emits_parseable_offer_to_multicast() { - let config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); let (rx, tx) = mcast_rx_tx().await; // Seed with a recognisable value so on-wire session_id is exact. @@ -930,12 +912,9 @@ mod tests { // Back-to-back sends must consume distinct, incrementing session // IDs — catches a regression where `send_offer_service` reads the // counter without advancing it, or reuses a cached value. - let config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); let (rx, tx) = mcast_rx_tx().await; let sd_state = SdStateManager::with_initial(0x1233); @@ -956,12 +935,9 @@ mod tests { // Session counter wrap must be visible on the wire: 0xFFFE -> 0xFFFF // -> 0x0001 (skipping the reserved 0). Exercises the wrap branch // *through* the send path, not only the unit test of next_session_id. - let config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); let (rx, tx) = mcast_rx_tx().await; let sd_state = SdStateManager::with_initial(0xFFFE); @@ -1000,12 +976,9 @@ mod tests { // TTL=0 is a legitimate SOME/IP-SD value meaning "stop offering"; // `send_offer_service` must preserve it end-to-end rather than, // say, defaulting it back to the ServerConfig::new value of 3. - let mut config = ServerConfig::new( - Ipv4Addr::LOCALHOST, - TEST_ADVERTISED_PORT, - TEST_SERVICE_ID, - TEST_INSTANCE_ID, - ); + let mut config = ServerConfig::new(TEST_SERVICE_ID, TEST_INSTANCE_ID) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(TEST_ADVERTISED_PORT); config.ttl = 0; let (rx, tx) = mcast_rx_tx().await; diff --git a/src/server/subscription_manager.rs b/src/server/subscription_manager.rs index bb59f068..ade5b213 100644 --- a/src/server/subscription_manager.rs +++ b/src/server/subscription_manager.rs @@ -129,7 +129,7 @@ impl SubscriptionManager { // bump on re-subscribe) are wanted later, update the per- // subscriber record here and rename the log accordingly. if subscribers.iter().any(|s| s.address == subscriber_addr) { - tracing::debug!( + crate::log::debug!( "Subscriber {} already subscribed for service 0x{:04X}, instance {}, \ event group 0x{:04X}; skipping duplicate", subscriber_addr, @@ -143,7 +143,7 @@ impl SubscriptionManager { let subscriber = Subscriber::new(subscriber_addr, service_id, instance_id, event_group_id); if subscribers.push(subscriber).is_err() { - tracing::warn!( + crate::log::warn!( "Subscribers-per-group at capacity ({}); dropping new subscriber {} \ for service 0x{:04X}, instance {}, event group 0x{:04X}", SUBSCRIBERS_PER_GROUP, @@ -155,7 +155,7 @@ impl SubscriptionManager { return Err(SubscribeError::SubscribersPerGroupFull); } - tracing::info!( + crate::log::info!( "Subscriber {} added for service 0x{:04X}, instance {}, event group 0x{:04X}", subscriber_addr, service_id, @@ -184,7 +184,7 @@ impl SubscriptionManager { ); if self.subscriptions.insert(key, list).is_err() { - tracing::warn!( + crate::log::warn!( "Event-group map at capacity ({}); dropping subscriber {} for new group \ service 0x{:04X}, instance {}, event group 0x{:04X}", EVENT_GROUPS_CAP, @@ -196,7 +196,7 @@ impl SubscriptionManager { return Err(SubscribeError::EventGroupsFull); } - tracing::info!( + crate::log::info!( "Subscriber {} added for service 0x{:04X}, instance {}, event group 0x{:04X}", subscriber_addr, service_id, @@ -223,7 +223,7 @@ impl SubscriptionManager { self.subscriptions.remove(&key); } - tracing::info!( + crate::log::info!( "Removed subscriber {} from service 0x{:04X}, instance {}, event group 0x{:04X}", subscriber_addr, service_id, @@ -281,11 +281,42 @@ impl Default for SubscriptionManager { /// critical-section-backed equivalents on bare metal. The futures /// returned by the methods are not required to be `Send`, allowing /// single-threaded executors (embassy-style) to satisfy the trait -/// without an `Arc`-style shared state. +/// without an `Arc`-style shared state. Implementations on +/// multi-threaded executors are free to make their `SubscribeFuture` +/// / `UnsubscribeFuture` `Send`, which lets `Server::run` (the +/// `Send`-bounded entry point used by `tokio::spawn`) accept them via +/// the `for<'a> Sub::SubscribeFuture<'a>: Send` bound. +/// +/// `subscribe` and `unsubscribe` use named [GATs] (rather than +/// return-position `impl Trait`) so `Server::run`'s where clause can +/// spell their `Send`-ness explicitly. `for_each_subscriber` stays +/// as RPIT — it is called by +/// [`EventPublisher::publish_event`](crate::server::EventPublisher), +/// not by the SD run-future, so no `Send` bound on it is currently +/// load-bearing. +/// +/// [GATs]: https://blog.rust-lang.org/2022/10/28/gats-stabilization.html /// /// Both `Server` and `EventPublisher` clone the same handle at construction /// time; the underlying subscription state is shared between them. pub trait SubscriptionHandle: Clone + 'static { + /// Future returned by [`Self::subscribe`]. + /// + /// Implementations choose the concrete type and decide whether to + /// implement `Send` / `Sync` on it. Tokio-backed implementations + /// (`Arc>`) box a `Send` future so + /// `Server::run`'s `Send` where clause is satisfiable; bare-metal + /// implementations are free to leave it `!Send`. + type SubscribeFuture<'a>: Future> + 'a + where + Self: 'a; + + /// Future returned by [`Self::unsubscribe`]. Same `Send`-or-not + /// freedom as [`Self::SubscribeFuture`]. + type UnsubscribeFuture<'a>: Future + 'a + where + Self: 'a; + /// Add a subscriber to an event group. /// /// Idempotent: if the subscriber is already present, this is a no-op @@ -297,7 +328,7 @@ pub trait SubscriptionHandle: Clone + 'static { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future> + '_; + ) -> Self::SubscribeFuture<'_>; /// Remove a subscriber from an event group. fn unsubscribe( @@ -306,7 +337,7 @@ pub trait SubscriptionHandle: Clone + 'static { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future + '_; + ) -> Self::UnsubscribeFuture<'_>; /// Visit each subscriber for the given event group with `f`. /// @@ -333,19 +364,28 @@ pub trait SubscriptionHandle: Clone + 'static { #[cfg(feature = "server-tokio")] impl SubscriptionHandle for Arc> { + /// Boxed `Send` future so `Server::run`'s `Send` bound is + /// satisfiable. The `Box::pin` allocation happens at SD-rate + /// (~1 Hz subscribes during steady state), small cost relative to + /// the wire-side activity it gates. + type SubscribeFuture<'a> = + core::pin::Pin> + Send + 'a>>; + type UnsubscribeFuture<'a> = + core::pin::Pin + Send + 'a>>; + fn subscribe( &self, service_id: u16, instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future> + '_ { + ) -> Self::SubscribeFuture<'_> { let this = self.clone(); - async move { + alloc::boxed::Box::pin(async move { this.write() .await .subscribe(service_id, instance_id, event_group_id, subscriber_addr) - } + }) } fn unsubscribe( @@ -354,16 +394,16 @@ impl SubscriptionHandle for Arc> { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future + '_ { + ) -> Self::UnsubscribeFuture<'_> { let this = self.clone(); - async move { + alloc::boxed::Box::pin(async move { this.write().await.unsubscribe( service_id, instance_id, event_group_id, subscriber_addr, ); - } + }) } fn for_each_subscriber<'a, F>( @@ -452,15 +492,31 @@ pub mod bare_metal_subscription_impl { } impl SubscriptionHandle for StaticSubscriptionHandle { + // Futures are `Send` even though `SubscriptionManager` itself + // isn't `Sync` — the `embassy-sync` + // `CriticalSectionRawMutex` wrapping it IS `Sync`, and the + // future bodies have no `.await` points inside the lock + // closure (they capture only the `&'static` storage handle + // and the by-value args, all `Send`). Boxing with `+ Send` + // lets `Server::run`'s `Send` bound be satisfied. The + // `server` feature (required for `SubscriptionHandle` to be + // in scope) implies `_alloc`, so `Box::pin` is always + // available here. + type SubscribeFuture<'a> = core::pin::Pin< + alloc::boxed::Box> + Send + 'a>, + >; + type UnsubscribeFuture<'a> = + core::pin::Pin + Send + 'a>>; + fn subscribe( &self, service_id: u16, instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future> + '_ { + ) -> Self::SubscribeFuture<'_> { let storage = self.0; - async move { + alloc::boxed::Box::pin(async move { storage.lock(|cell| { cell.borrow_mut().subscribe( service_id, @@ -469,7 +525,7 @@ pub mod bare_metal_subscription_impl { subscriber_addr, ) }) - } + }) } fn unsubscribe( @@ -478,9 +534,9 @@ pub mod bare_metal_subscription_impl { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future + '_ { + ) -> Self::UnsubscribeFuture<'_> { let storage = self.0; - async move { + alloc::boxed::Box::pin(async move { storage.lock(|cell| { cell.borrow_mut().unsubscribe( service_id, @@ -489,7 +545,7 @@ pub mod bare_metal_subscription_impl { subscriber_addr, ); }); - } + }) } fn for_each_subscriber<'a, F>( diff --git a/src/static_channels/mod.rs b/src/static_channels/mod.rs index d945da65..9bdc16f2 100644 --- a/src/static_channels/mod.rs +++ b/src/static_channels/mod.rs @@ -884,6 +884,13 @@ pub const UNBOUNDED_DEFAULT_CAP: usize = 128; /// has no entries. The bounded entry shape is /// `((Type, slot_cap), pool_size)` to disambiguate the slot cap /// from the pool size in the macro grammar. +/// +/// # Required entries for `Client` +/// +/// To use the generated factory with [`crate::Client`], the macro +/// invocation must declare the seven channel types enumerated by +/// [`crate::client::ClientChannelTypes`]. See its rustdoc for the +/// exhaustive list and a worked example. #[macro_export] macro_rules! define_static_channels { // Entry point: explicit visibility. diff --git a/src/tokio_transport.rs b/src/tokio_transport.rs index 37f08ccb..2ee097da 100644 --- a/src/tokio_transport.rs +++ b/src/tokio_transport.rs @@ -189,11 +189,11 @@ impl Future for RecvFrom<'_> { // truncates when the caller's `buf` is smaller than the // datagram and returns only the bytes that fit — it does // NOT expose a truncation flag. Surfacing a reliable - // `truncated: bool` here would require a platform-specific - // `recvmsg`/MSG_TRUNC path (libc + unsafe), which is - // deferred for now. Until then, this field is always - // `false` for the Tokio backend; callers must not rely on - // it for truncation detection. This is documented on + // `truncated: bool` here requires a platform-specific + // `recvmsg`/MSG_TRUNC path (libc + unsafe) — tracked in + // #119. Until then, this field is always `false` for the + // Tokio backend; callers must not rely on it for + // truncation detection. Also documented on // `ReceivedDatagram::truncated`'s field doc. Poll::Ready(Ok(ReceivedDatagram { bytes_received: n, @@ -274,7 +274,7 @@ impl Timer for TokioTimer { } /// Wraps a `Future` so that any panic during `poll` is logged via -/// `tracing::error!` and the future then resolves cleanly. Lets +/// `crate::log::error!` and the future then resolves cleanly. Lets /// `TokioSpawner::spawn` use exactly **one** tokio task per call /// instead of pairing each work future with a `JoinHandle`-watcher /// task — the prior watcher-pair pattern doubled task count and @@ -300,7 +300,7 @@ impl> Future for PanicLoggingFut { Ok(poll) => poll, Err(payload) => { let msg = panic_payload_str(&payload); - tracing::error!( + crate::log::error!( panic_message = msg, "spawned task panicked; channels will close", ); @@ -415,14 +415,14 @@ fn map_io_error(e: &std::io::Error) -> TransportError { // so we don't drown out actionable warnings under load. match kind { K::TimedOut | K::Interrupted | K::ConnectionRefused => { - tracing::debug!( + crate::log::debug!( "tokio transport io error: {e} (raw_os={:?}, kind={:?}) mapped to {mapped}", e.raw_os_error(), kind, ); } _ => { - tracing::warn!( + crate::log::warn!( "tokio transport io error: {e} (raw_os={:?}, kind={:?}) mapped to {mapped}", e.raw_os_error(), kind, diff --git a/src/transport.rs b/src/transport.rs index 073d2456..b81eb24f 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -485,6 +485,20 @@ pub trait TransportSocket { /// socket, or the concurrent send branch of a `select!` cannot /// compile. /// + /// # Cancel safety + /// + /// The returned [`Self::RecvFuture`] **must be cancel-safe**: + /// dropping it before completion (the typical outcome inside a + /// `select!` / `select_biased!` where another arm wins) must not + /// lose any datagram that the kernel had already delivered to the + /// socket. The server run-loop and the client socket-manager both + /// race this future against other arms and rely on the + /// drop-and-retry pattern; a backend whose recv-future commits + /// kernel state before yielding (and loses it on drop) would + /// silently drop datagrams. The default `TokioSocket` impl + /// satisfies this via tokio's documented cancel-safety on + /// `UdpSocket::recv_from`. + /// /// # Errors /// /// Returns: @@ -1052,20 +1066,19 @@ pub mod bare_metal_handle_impls { self.0.store(u32::from(addr), Ordering::Release); } } - // Phase 20e collapsed `StaticSocketHandle(&'static T)` into a + // `StaticSocketHandle(&'static T)` was collapsed into a // direct `impl SharedHandle for &'static T` blanket — the // wrapper type's only role was carrying the `'static` lifetime, // which the blanket impl achieves without a wrapper. Consumers - // that previously constructed `StaticSocketHandle::new(&SOCKET)` - // now pass `&SOCKET` directly into Server's no-wrap constructors. + // pass `&SOCKET` directly into Server's no-wrap constructors. } /// `StaticE2EHandle` — no-alloc `E2ERegistryHandle` backed by a /// `&'static` critical-section mutex. /// /// Available in pure `no_std` builds: [`crate::e2e::E2ERegistry`] is -/// backed by [`heapless::index_map::FnvIndexMap`] (since phase 18a), -/// so no allocator is required. +/// backed by [`heapless::index_map::FnvIndexMap`], so no allocator is +/// required. #[cfg(feature = "bare_metal")] pub mod bare_metal_e2e_impl { use super::E2ERegistryHandle; diff --git a/tests/bare_metal_e2e.rs b/tests/bare_metal_e2e.rs index a90a253c..7127eaa9 100644 --- a/tests/bare_metal_e2e.rs +++ b/tests/bare_metal_e2e.rs @@ -266,22 +266,26 @@ type SubKey = (u16, u16, u16, SocketAddrV4); struct MockSubscriptions(Arc>>); impl SubscriptionHandle for MockSubscriptions { + type SubscribeFuture<'a> = + core::pin::Pin> + Send + 'a>>; + type UnsubscribeFuture<'a> = core::pin::Pin + Send + 'a>>; + fn subscribe( &self, service_id: u16, instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future> + '_ { + ) -> Self::SubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut guard = this.lock().unwrap(); let key = (service_id, instance_id, event_group_id, subscriber_addr); if !guard.contains(&key) { guard.push(key); } Ok(()) - } + }) } fn unsubscribe( @@ -290,12 +294,12 @@ impl SubscriptionHandle for MockSubscriptions { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future + '_ { + ) -> Self::UnsubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut guard = this.lock().unwrap(); guard.retain(|e| *e != (service_id, instance_id, event_group_id, subscriber_addr)); - } + }) } fn for_each_subscriber<'a, F>( @@ -350,7 +354,9 @@ async fn client_receives_server_sd_announcement() { // Create server let server_e2e: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); let server_subs = MockSubscriptions::default(); - let server_config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30500, 0x1234, 1); + let server_config = ServerConfig::new(0x1234, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30500); let server_deps = ServerDeps { factory: server_factory, @@ -359,14 +365,16 @@ async fn client_receives_server_sd_announcement() { subscriptions: server_subs, }; - let server: Server>, MockSubscriptions, MockFactory, MockTimer> = - Server::new_with_deps(server_deps, server_config, false) - .await - .expect("server creation"); + let (_server, _handles, run): ( + Server>, MockSubscriptions>, + _, + _, + ) = Server::new_with_deps(server_deps, server_config, false) + .await + .expect("server creation"); - // Start server announcement loop - let announce_fut = server.announcement_loop().expect("announcement_loop"); - let announce_handle = tokio::spawn(announce_fut); + // Combined run-future drives both announcement + receive. + let announce_handle = tokio::spawn(run); // Create client let client_e2e: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); @@ -442,8 +450,9 @@ async fn client_send_request_server_runloop_stable() { let service_id = 0x5678_u16; let instance_id = 1_u16; let server_port = 30600_u16; - let server_config = - ServerConfig::new(Ipv4Addr::LOCALHOST, server_port, service_id, instance_id); + let server_config = ServerConfig::new(service_id, instance_id) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(server_port); let server_deps = ServerDeps { factory: server_factory, @@ -452,14 +461,17 @@ async fn client_send_request_server_runloop_stable() { subscriptions: server_subs, }; - let mut server: Server>, MockSubscriptions, MockFactory, MockTimer> = - Server::new_passive_with_deps(server_deps, server_config) - .await - .expect("passive server creation"); + let (_server, _handles, run): ( + Server>, MockSubscriptions>, + _, + _, + ) = Server::new_passive_with_deps(server_deps, server_config) + .await + .expect("passive server creation"); - // Start server run loop + // Start server run loop (passive — receive only, no announcements). let run_handle = tokio::spawn(async move { - let _ = server.run().await; + let _ = run.await; }); // Create client diff --git a/tests/bare_metal_server.rs b/tests/bare_metal_server.rs index 986c202f..b487a992 100644 --- a/tests/bare_metal_server.rs +++ b/tests/bare_metal_server.rs @@ -204,22 +204,26 @@ type SubKey = (u16, u16, u16, SocketAddrV4); struct MockSubscriptions(Arc>>); impl SubscriptionHandle for MockSubscriptions { + type SubscribeFuture<'a> = + core::pin::Pin> + Send + 'a>>; + type UnsubscribeFuture<'a> = core::pin::Pin + Send + 'a>>; + fn subscribe( &self, service_id: u16, instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future> + '_ { + ) -> Self::SubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut guard = this.lock().unwrap(); let key = (service_id, instance_id, event_group_id, subscriber_addr); if !guard.contains(&key) { guard.push(key); } Ok(()) - } + }) } fn unsubscribe( @@ -228,12 +232,12 @@ impl SubscriptionHandle for MockSubscriptions { instance_id: u16, event_group_id: u16, subscriber_addr: SocketAddrV4, - ) -> impl Future + '_ { + ) -> Self::UnsubscribeFuture<'_> { let this = self.0.clone(); - async move { + Box::pin(async move { let mut guard = this.lock().unwrap(); guard.retain(|e| *e != (service_id, instance_id, event_group_id, subscriber_addr)); - } + }) } fn for_each_subscriber<'a, F>( @@ -275,7 +279,9 @@ async fn server_constructible_without_server_tokio_feature() { let e2e_handle: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); let subs = MockSubscriptions::default(); - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 30490, 0x5B, 1); + let config = ServerConfig::new(0x5B, 1) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(30490); let deps: ServerDeps>, MockSubscriptions> = ServerDeps { @@ -285,18 +291,19 @@ async fn server_constructible_without_server_tokio_feature() { subscriptions: subs, }; - let server: Server>, MockSubscriptions, MockFactory, MockTimer> = - Server::new_with_deps(deps, config, false) - .await - .expect("Server::new_with_deps must succeed with no-tokio mocks"); + let (_server, _handles, run): ( + Server>, MockSubscriptions>, + _, + _, + ) = Server::new_with_deps(deps, config, false) + .await + .expect("Server::new_with_deps must succeed with no-tokio mocks"); - // Build the announcement-loop future and prove it's `Send + 'static` - // by spawning it on tokio. The witness is purely structural: if this - // line compiles, `Server` is reachable on a no-tokio build. - let announce_fut = server - .announcement_loop() - .expect("announcement_loop must build on a non-passive server"); - let handle = tokio::spawn(announce_fut); + // The combined run-future drives both receive and announce. + // Spawning it on tokio proves it's `'static`. The witness is + // purely structural: if this line compiles, `Server` is reachable + // on a no-tokio build. + let handle = tokio::spawn(run); // Yield once so the spawned future has a chance to poll (its first // tick fires `send_to` immediately, before the timer sleep). @@ -319,7 +326,9 @@ async fn passive_server_constructible_without_server_tokio_feature() { let e2e_handle: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); let subs = MockSubscriptions::default(); - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, 0x5C, 2); + let config = ServerConfig::new(0x5C, 2) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); let deps: ServerDeps>, MockSubscriptions> = ServerDeps { @@ -329,8 +338,11 @@ async fn passive_server_constructible_without_server_tokio_feature() { subscriptions: subs, }; - let _server: Server>, MockSubscriptions, MockFactory, MockTimer> = - Server::new_passive_with_deps(deps, config) - .await - .expect("Server::new_passive_with_deps must succeed with no-tokio mocks"); + let (_server, _handles, _run): ( + Server>, MockSubscriptions>, + _, + _, + ) = Server::new_passive_with_deps(deps, config) + .await + .expect("Server::new_passive_with_deps must succeed with no-tokio mocks"); } diff --git a/tests/client_server.rs b/tests/client_server.rs index 9b72d1fc..7e1703e3 100644 --- a/tests/client_server.rs +++ b/tests/client_server.rs @@ -63,10 +63,10 @@ type TestClient = Client< /// scope so callers can spell `TestServer::new(...)` without chasing the /// four-type-parameter signature on every call site. type TestServer = Server< - std::sync::Arc>, - std::sync::Arc>, simple_someip::TokioTransport, simple_someip::TokioTimer, + std::sync::Arc>, + std::sync::Arc>, >; /// Type alias for the event publisher concrete type used by `TestServer`'s @@ -79,14 +79,24 @@ type TestEventPublisher = simple_someip::server::EventPublisher< >; /// Create a server on an ephemeral unicast port, returning (Server, actual_port). +/// +/// `TestServer::new` returns a `(Server, ServerHandles, run)` tuple. +/// Tests in this module construct the server, query the +/// kernel-assigned port via `unicast_local_addr`, and don't spawn +/// the run future from this helper — the few tests that need it call +/// `server.run()` directly after receiving the `Server` handle. async fn create_server(service_id: u16, instance_id: u16) -> (TestServer, u16) { - let config = ServerConfig::new(Ipv4Addr::LOCALHOST, 0, service_id, instance_id); - let mut server: TestServer = TestServer::new(config).await.expect("Server::new failed"); + let config = ServerConfig::new(service_id, instance_id) + .with_interface(Ipv4Addr::LOCALHOST) + .with_local_port(0); + let (server, _handles, _run): (TestServer, _, _) = + TestServer::new(config).await.expect("Server::new failed"); + // Constructor already back-filled `config.local_port` from the + // kernel-assigned bound port; just read it back for the test return. let port = match server.unicast_local_addr().expect("local_addr failed") { std::net::SocketAddr::V4(a) => a.port(), _ => panic!("expected IPv4"), }; - server.set_local_port(port); (server, port) } @@ -114,7 +124,7 @@ async fn wait_for_subscribers( async fn test_client_server_subscribe_and_receive_event() { // Start server on ephemeral port let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let publisher = server.publisher(); let server_handle = tokio::spawn(async move { server.run().await }); @@ -166,7 +176,7 @@ async fn test_client_server_subscribe_and_receive_event() { async fn test_client_send_sd_auto_binds_discovery() { // Create server so there is something to send to let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let server_handle = tokio::spawn(async move { server.run().await }); // Create client — NO bind_discovery @@ -200,7 +210,7 @@ async fn test_client_send_sd_auto_binds_discovery() { #[tokio::test] async fn test_client_bind_unbind_lifecycle_with_server() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); @@ -235,7 +245,7 @@ async fn test_client_bind_unbind_lifecycle_with_server() { #[tokio::test] async fn test_add_endpoint_and_send_to_service() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let publisher = server.publisher(); let server_handle = tokio::spawn(async move { server.run().await }); @@ -302,7 +312,7 @@ async fn test_add_endpoint_and_send_to_service() { #[tokio::test] async fn test_subscribe_auto_binds_discovery() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let publisher = server.publisher(); let server_handle = tokio::spawn(async move { server.run().await }); @@ -353,7 +363,7 @@ async fn test_subscribe_auto_binds_discovery() { #[tokio::test] async fn test_client_request_resolves_via_unicast_reply() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let publisher = server.publisher(); let server_handle = tokio::spawn(async move { server.run().await }); @@ -413,7 +423,7 @@ async fn test_client_request_resolves_via_unicast_reply() { #[tokio::test] async fn test_e2e_protect_on_publish_and_check_on_receive() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let publisher = server.publisher(); // Register E2E profile on server for the event message ID @@ -495,7 +505,7 @@ async fn test_e2e_protect_on_publish_and_check_on_receive() { #[tokio::test] async fn test_multiple_subscribers_receive_events() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let publisher = server.publisher(); let server_handle = tokio::spawn(async move { server.run().await }); @@ -588,7 +598,7 @@ async fn test_updates_drain_after_shutdown() { #[tokio::test] async fn test_cloned_client_works() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); @@ -616,7 +626,7 @@ async fn test_cloned_client_works() { #[tokio::test] async fn test_subscribe_specific_port_reuse() { let service_id = next_service_id(); - let (mut server, server_port) = create_server(service_id, 1).await; + let (server, server_port) = create_server(service_id, 1).await; let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); diff --git a/tests/data/vsomeip-offerer/Dockerfile b/tests/data/vsomeip-offerer/Dockerfile index 1361c221..45032e8a 100644 --- a/tests/data/vsomeip-offerer/Dockerfile +++ b/tests/data/vsomeip-offerer/Dockerfile @@ -1,6 +1,6 @@ # vsomeip 3.4.10 + a minimal offerer that advertises service 0x1234 -# instance 0x0001 via SD multicast. Used by phase 20f's host-side -# conformance test (`tests/vsomeip_sd_compat.rs`). +# instance 0x0001 via SD multicast. Used by the host-side +# conformance test in `tests/vsomeip_sd_compat.rs`. # # Build: # docker build -t vsomeip-offerer tests/data/vsomeip-offerer/ diff --git a/tests/data/vsomeip-offerer/README.md b/tests/data/vsomeip-offerer/README.md index 2e8c3461..d589a60c 100644 --- a/tests/data/vsomeip-offerer/README.md +++ b/tests/data/vsomeip-offerer/README.md @@ -104,7 +104,7 @@ docker stop vsomeip-offerer For real-NIC testing, set this to the host's interface IP and set `SIMPLE_SOMEIP_TEST_INTERFACE` to match. -## Future (phase 20g+) +## Future - Wire this Dockerfile into CI via TestContainers-rs (or equivalent) so `cargo test ... -- --ignored` runs in a diff --git a/tests/vsomeip_sd_compat.rs b/tests/vsomeip_sd_compat.rs index 7fdc68e6..0893ffc2 100644 --- a/tests/vsomeip_sd_compat.rs +++ b/tests/vsomeip_sd_compat.rs @@ -1,4 +1,4 @@ -//! Phase 20f — Conformance test against the COVESA vsomeip reference +//! Conformance test against the COVESA vsomeip reference //! SOME/IP-SD implementation. //! //! `#[ignore]`'d by default. Run on demand once you have vsomeip @@ -353,7 +353,7 @@ async fn client_sees_vsomeip_offer_service() { } } -// ── Phase 20h: TX direction — simple-someip emits, vsomeip subscribes ─ +// ── TX direction — simple-someip emits, vsomeip subscribes ─────────── /// Container name for the subscriber-role container. Hardcoded so the /// test knows which `docker logs` to scrape; if you run the container @@ -431,27 +431,16 @@ async fn vsomeip_sees_simple_someip_offer_service() { // Build a tokio-flavor Server with multicast loopback enabled // (matches vsomeip's default; lets a same-host subscriber see // our broadcasts even on the actual NIC). - let config = ServerConfig::new(interface, 30500, SERVICE_ID, INSTANCE_ID); - let mut server = Server::new_with_loopback(config, true) + let config = ServerConfig::new(SERVICE_ID, INSTANCE_ID) + .with_interface(interface) + .with_local_port(30500); + let (_server, _handles, run) = Server::new_with_loopback(config, true) .await .expect("Server::new_with_loopback failed (network setup problem?)"); - // `announcement_loop()` returns the `+ Send + 'static` future - // that emits OfferService SD broadcasts every cyclic_offer_delay - // (default 1s in simple-someip). Spawning it on tokio works - // here because TokioSocket is Send + Sync and the std-side - // bounds are met by the convenience constructor's defaults. - let announce_fut = server - .announcement_loop() - .expect("announcement_loop failed; passive server?"); - let announce_handle = tokio::spawn(announce_fut); - - // Drive the server's run loop too — it does multicast-loopback - // SD receive, but for this test we only care that announcements - // go out. The run loop survives without subscribers. - let server_handle = tokio::spawn(async move { - let _ = server.run().await; - }); + // Announcements are folded into `Server::run`'s combined future. + // Spawning it works here because TokioSocket is Send + Sync. + let server_handle = tokio::spawn(run); eprintln!("[test] announcement loop spawned; polling docker logs"); @@ -481,7 +470,6 @@ async fn vsomeip_sees_simple_someip_offer_service() { }) .await; - announce_handle.abort(); server_handle.abort(); match saw_marker { @@ -524,7 +512,7 @@ async fn vsomeip_sees_simple_someip_offer_service() { } } -// ── Phase 20h: TX direction — wire-format self-check (no docker) ────── +// ── TX direction — wire-format self-check (no docker) ──────────────── /// Verifies `Server::announcement_loop` emits SOME/IP-SD bytes that /// match the AUTOSAR SOME/IP-SD spec, by capturing the bytes on a @@ -598,20 +586,14 @@ async fn tx_announcement_loop_emits_wire_format_offer() { // OfferService packets loop back to our receiver on the same // interface. const ADVERTISED_PORT: u16 = 30500; - let config = ServerConfig::new(interface, ADVERTISED_PORT, SERVICE_ID, INSTANCE_ID); - let mut server = Server::new_with_loopback(config, true) + let config = ServerConfig::new(SERVICE_ID, INSTANCE_ID) + .with_interface(interface) + .with_local_port(ADVERTISED_PORT); + let (_server, _handles, run) = Server::new_with_loopback(config, true) .await .expect("Server::new_with_loopback failed"); - let announce_fut = server - .announcement_loop() - .expect("announcement_loop failed; passive server?"); - let announce_handle = tokio::spawn(announce_fut); - // Drive run() too so the Server's own SD socket drains, but we - // assert against bytes we receive on our independent capture - // socket — the run-loop is just to keep the Server healthy. - let server_handle = tokio::spawn(async move { - let _ = server.run().await; - }); + // Combined announce + receive run-future. + let server_handle = tokio::spawn(run); // Owned snapshot of the assertion-relevant fields. Pulled out // inside `recv_loop` because `MessageView` / `SdHeaderView` / @@ -735,13 +717,11 @@ async fn tx_announcement_loop_emits_wire_format_offer() { "Timed out after {}s waiting to capture SECOND OfferService \ on {interface}. Cyclic offer delay is ~1s; if first arrived \ but second didn't, something tore down the announcement loop \ - mid-test (check announce_handle / server_handle for early \ - failure).", + mid-test (check server_handle for early failure).", second_timeout.as_secs(), ) }); - announce_handle.abort(); server_handle.abort(); // ── First announcement: full envelope shape + reboot flag ────────