Phase20 cleanup#113
Open
JustinKovacich wants to merge 34 commits into
Open
Conversation
Drops the std-only HashMap backing in favor of a fixed-capacity
heapless::FnvIndexMap, sized to E2E_REGISTRY_CAP = 32. The registry
is now const-constructible and no_std-compatible; gating drops from
both the registry module itself and the e2e_check / e2e_protect /
E2EState support code (none of which actually used std).
Why this matters: phase 17 / 0.8.0 gated `bare_metal_handle_impls`
behind +std specifically because StaticE2EHandle wraps E2ERegistry
and the registry's HashMap was std-only. With the registry ported,
that gate is no longer load-bearing — phase 18c will drop it. This
sub-phase isolates the storage swap so the gating change has a
clean baseline.
API surface change (breaking, queued for 0.9.0):
- E2ERegistry::register now returns Result<(), E2ERegistryFull>.
Replacing an already-registered key always succeeds (the slot is
reused). Inserting a new key when the registry is at capacity
returns Err(E2ERegistryFull(E2E_REGISTRY_CAP)).
- E2ERegistryHandle trait method `register` lifted to the same
return type, so std (Arc<Mutex<E2ERegistry>>), bare_metal
(StaticE2EHandle), and test (NullE2ERegistry) impls all forward
the typed overflow.
- Client::register_e2e and Server::register_e2e now return
Result<(), E2ERegistryFull> through to the public API. Callers
that previously discarded the unit return must add a
`?` / `.expect("E2E registry has capacity")` / explicit handling.
Two new regression tests:
- register_replacement_succeeds_when_full — re-registering an
existing key at capacity must reuse the slot (locks in the
FnvIndexMap "full + present" branch).
- register_overflow_returns_err_and_does_not_mutate — adding a new
key beyond cap returns Err(E2ERegistryFull(E2E_REGISTRY_CAP)) AND
does not insert.
512 lib tests pass (was 510; +2 new). cargo build clean across all
13 feature combos. cargo clippy --workspace --all-features
-- -D warnings -D clippy::pedantic clean. cargo build
--no-default-features (true no_std without bare_metal) compiles.
This is sub-phase 18a of phase 18 (per bare_metal_plan_v3.md).
Remaining sub-phases:
- 18b: replace std::sync references in SubscriptionManager
- 18c: provide no_std default lock-handle impls (ungate StaticE2EHandle, add StaticSubscriptionHandle)
- 18d: drop std from `client` / `server` Cargo features
- 18e: add the no_std-target CI gate (cross-build for thumbv7em-none-eabihf)
- 18f: docs + 0.9.0 bump
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Narrow scope per bare_metal_plan_v3.md 18b: drop the unconditional
`use std::{net::SocketAddrV4, vec::Vec};` from production code in
`src/server/subscription_manager.rs`, gate `get_subscribers ->
std::vec::Vec<Subscriber>` behind `#[cfg(feature = "std")]`, and
swap the unconditional `std::net::SocketAddrV4` import for
`core::net::SocketAddrV4`. The internal storage (FnvIndexMap +
heapless::Vec) was already heap-free since phase 13.5/13.6; this
sub-phase is what makes it literally compile in pure no_std.
`get_subscribers` is the only Vec-returning method on the manager,
and production code paths migrated to `for_each_subscriber`
(visitor) in phase 17. Std consumers keep the convenience accessor
unchanged; no_std consumers either use `for_each_subscriber` or
collect into their own heapless::Vec.
Out of scope (deferred to 18d's broad sweep):
- ServiceInfo / EventGroupInfo (still use std::vec::Vec for their
pub fields) — 18d will port to heapless::Vec with documented
caps.
- event_publisher.rs / sd_state.rs / mod.rs std::sync references
for the `Arc<Mutex<E2ERegistry>>` / `Arc<RwLock<...>>` lock-handle
defaults.
Verification:
- cargo build --no-default-features clean
- cargo build --no-default-features --features bare_metal clean
- cargo build --all-features clean
- cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic clean
- cargo clippy --no-default-features -- -D warnings -D clippy::pedantic clean
- cargo test --lib --all-features: 512 pass, 0 fail (no regressions)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes that complete the no_std default lock-handle story for
the bare-metal server:
1. Drop the `+ std` gate from `bare_metal_e2e_impl`. Phase 18a
ported `E2ERegistry` to `heapless::FnvIndexMap`, so
`StaticE2EHandle` no longer needs std. It's now reachable in
pure no_std builds via `feature = "bare_metal"` alone. Updated
the lib.rs feature-table line accordingly.
2. New `StaticSubscriptionHandle` (and `StaticSubscriptionStorage`
alias) in `src/server/subscription_manager.rs`. Modeled on
`StaticE2EHandle`: a `&'static BlockingMutex<CriticalSectionRawMutex,
RefCell<SubscriptionManager>>` wrapper that implements the full
`SubscriptionHandle` trait (subscribe / unsubscribe /
for_each_subscriber). Gated on `feature = "bare_metal"`, so
bare-metal Server consumers no longer need to write their own
subscription handle.
Made `SubscriptionManager::new()` `const` so the storage can live in
a plain `static`, no `Box::leak` required:
```rust
static SUBS: StaticSubscriptionStorage =
Mutex::new(RefCell::new(SubscriptionManager::new()));
let handle = StaticSubscriptionHandle::new(&SUBS);
```
Re-exported `StaticSubscriptionHandle` and `StaticSubscriptionStorage`
from `server::*` (gated on `bare_metal`).
Regression test (`static_subscription_handle_full_contract`) walks
subscribe → for_each_subscriber → unsubscribe → for_each_subscriber
through the trait surface to lock in the wiring. Includes a
`block_on_sync` helper that asserts the futures complete
synchronously (no .await inside the critical section), since the
embassy-sync `lock` closure is sync.
After this sub-phase, the three default lock-handles
(`StaticE2EHandle`, `AtomicInterfaceHandle`, `StaticSubscriptionHandle`)
are all available on pure no_std via `feature = "bare_metal"` —
matching the surface that bare-metal Client + Server consumers will
need from 18d onward when `client` / `server` features drop their
std requirement.
Verification:
- cargo build --no-default-features --features bare_metal clean
- cargo build --no-default-features --features server,bare_metal clean
- cargo build --all-features clean
- cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic clean
- cargo clippy --no-default-features -- -D warnings -D clippy::pedantic clean
- cargo test --lib --all-features: 513 pass, 0 fail (+1 new test)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The actual gate-closer for phase 18. After this sub-phase: cargo build --no-default-features --features client,server,bare_metal …compiles in pure no_std (no allocator required for `client`; `server` still pulls `extern crate alloc;` for its `Arc<EventPublisher>` / `Arc<Socket>` plumbing, documented as a known limitation tracked for a future refactor). ## Cargo.toml - `client = ["dep:futures"]` (was `["std", "dep:futures"]`). - `server = ["dep:futures"]` (was `["std", "dep:futures"]`). - `client-tokio = ["client", "std", "dep:tokio", "dep:socket2"]` (added `"std"` so the tokio convenience constructors keep their std backing). - `server-tokio = ["server", "std", "dep:tokio", "dep:socket2"]` (same). - `extern crate alloc;` in lib.rs now activates on `cfg(any(feature = "embassy_channels", feature = "server"))` instead of just `embassy_channels`. Server's Arc usage is the trigger. ## Trait surface change (breaking, queued for 0.9.0) `PayloadWireFormat` was tangled with std-only items (`new_subscription_sd_header` took `std::net::Ipv4Addr`; `offered_endpoints` / `service_instances` returned `Vec<_>`; `set_reboot_flag` was `cfg(feature = "std")`). Restructured: - `OfferedEndpoint` is no longer std-gated; `addr` is now `Option<core::net::SocketAddrV4>`. - `set_reboot_flag` is no longer std-gated. - `new_subscription_sd_header` is no longer std-gated; `client_ip` is now `core::net::Ipv4Addr`. - `offered_endpoints -> Vec<...>` and `service_instances -> Vec<...>` replaced by visitor-pattern `for_each_offered_endpoint(&self, F)` and `for_each_service_instance(&self, F)` (no_std-friendly, alloc-free). - Old `offered_endpoints` / `service_instances` Vec-returning signatures preserved as `cfg(feature = "std")` convenience wrappers that delegate to the new visitors. Std consumers' code keeps compiling unchanged. `Client::run_future` updated to use the new visitor methods directly. `RawPayload`'s impl block updated to override the new visitor signatures (was overriding the old Vec-returning ones). ## server::Error API change - `Error::Io(std::io::Error)` is now gated on `cfg(feature = "std")`. No-std consumers receive transport failures via `Error::Transport(_)` carrying the portable `IoErrorKind` instead. - New `Error::InvalidUsage(&'static str)` variant for misuse paths (`announcement_loop` on a passive server, `announcement_loop` called twice, `run` on a passive server). These previously returned `Error::Io(std::io::Error::new(InvalidInput, ...))` with a formatted message; the new variant carries a `&'static str` tag and the diagnostic moves to `tracing::warn!`. Tags: `"passive_server_announcement_loop"`, `"announcement_loop_already_started"`, `"passive_server_run"`. ## ServiceInfo / EventGroupInfo Both gated on `cfg(feature = "std")` because their pub fields hold `Vec<EventGroupInfo>` / `Vec<u16>`. Bare-metal consumers don't construct these types today; a future port will switch to `heapless::Vec` if a use case emerges. `Subscriber` (no Vec field) stays no_std and exported. ## Other std → core sweeps - `src/client/session.rs`: `std::net::SocketAddr` → `core::net::SocketAddr`. - `src/client/socket_manager.rs`: same. - `src/client/inner.rs`: removed `use std::borrow::ToOwned;`, replaced `sd_header.to_owned()` with `Clone::clone(sd_header)`; replaced `std::future::poll_fn` with `core::future::poll_fn`; replaced `std::fmt::*` with `core::fmt::*`. - `src/server/mod.rs`: `std::net::*` → `core::net::*`, `Arc` from `alloc::sync::Arc`, large `vec![0u8; 65535]` buffers use `alloc::vec![]`. - `src/server/event_publisher.rs`: `Arc` from `alloc::sync::Arc`, `std::net::SocketAddrV4` → `core::net::SocketAddrV4`. - `src/server/sd_state.rs`: `std::net::SocketAddrV4` → `core::net`. - 3 server::tests assertions updated for the new `Error::InvalidUsage` variant (was matching `Error::Io` with InvalidInput kind). ## Verification - cargo build --all-features clean - cargo build --no-default-features clean - cargo build --no-default-features --features client clean - cargo build --no-default-features --features server clean - cargo build --no-default-features --features client,bare_metal clean - cargo build --no-default-features --features server,bare_metal clean - cargo build --no-default-features --features client,server,bare_metal clean - cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic clean - cargo clippy --no-default-features -- -D warnings -D clippy::pedantic clean - cargo clippy --no-default-features --features client,bare_metal -- -D warnings -D clippy::pedantic clean - cargo fmt --all --check clean - cargo test --lib --all-features: 513 pass, 0 fail (test assertions updated for new error variant) The `cargo build --target thumbv7em-none-eabihf` cross-compile gate is the next sub-phase (18e). Locally these cargo build invocations target host x86_64 — they prove the std refs are gone but do NOT prove the bare-metal ABI works end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cross-compiling `client,server,bare_metal` to a true no_std target
surfaced two issues that the host-side x86_64 build hid:
1. **`futures::select!` requires the futures crate's `std` feature**,
which transitively pulls `slab` / `memchr` / `futures-io` —
none of which compile on no_std. Switched dep from the `futures`
umbrella to `futures-util` directly with features
`["async-await", "async-await-macro"]`. `select_biased!` is in
that subset; `select!` is not (it needs std for the random
fairness shuffle). Replaced all four `select!` call sites with
`select_biased!`.
Behavioral consequence: `select_biased!` polls arms top-first
instead of pseudo-randomly. For our three uses
(`socket_loop_future`, `Inner::run_future`, `server::run`) the
bias actually gives slightly better behavior — control messages
and sends get priority over recvs. Genuine starvation requires
the top arm to never go pending, which doesn't happen for any
of these workloads (sends are sporadic, control is sparse, SD
multicast is 1Hz).
2. **`futures::FutureExt::catch_unwind` requires futures-util's
`std` feature.** Replaced the catch-unwind dance with
`JoinHandle::is_panic()` on the `JoinHandle` returned by
`tokio::spawn`. A second tokio task awaits the join and logs
the panic via `tracing::error!` if `is_panic()` is true. Same
observable behavior, no extra dep gating needed.
Verification — both host AND cortex-m4f cross-compile:
cargo build --all-features ✓
cargo build --no-default-features ✓
cargo build --no-default-features --features bare_metal ✓
cargo build --no-default-features --features client,bare_metal ✓
cargo build --no-default-features --features server,bare_metal ✓
cargo build --no-default-features --features client,server,bare_metal ✓
cargo build --target thumbv7em-none-eabihf --no-default-features --features bare_metal ✓
cargo build --target thumbv7em-none-eabihf --no-default-features --features client,bare_metal ✓
cargo build --target thumbv7em-none-eabihf --no-default-features --features server,bare_metal ✓
cargo build --target thumbv7em-none-eabihf --no-default-features --features client,server,bare_metal ✓
cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic ✓
cargo fmt --all --check ✓
cargo test --lib --all-features: 513 pass, 0 fail ✓
Alloc-symbol audit on the cortex-m4f rlib:
client + bare_metal: 0 alloc references (truly alloc-free)
client + server + bare_metal: 14 alloc references (Arc<EventPublisher>
/ Arc<F::Socket> as documented in 18d)
This commit closes phase 18's literal compile gate. The 18e CI step
(adding the cross-build to `.github/workflows/ci.yml`) plus 18f
(0.9.0 docs + bump) remain.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Locks in phase 18's literal compile gate by cross-building the crate for `thumbv7em-none-eabihf` (cortex-m4f, no_std, no allocator) on every PR. Until this job is green, the crate cannot actually be consumed on bare-metal — phases 4–17 shipped the trait surface and no-alloc primitives but the literal cross-build was never verified in CI. Four feature combos exercised, each as a separate `cargo build` so a failure surfaces the specific combo that regressed: - bare_metal alone - server + bare_metal - client + server + bare_metal - client + bare_metal (last, for the alloc-symbol audit below) Plus an alloc-symbol audit step: greps the resulting `libsimple_someip.rlib` for `__rust_alloc` / `__rg_alloc` and fails if any are found. `client + bare_metal` MUST stay alloc-free. The `server` and `client+server` paths reference allocator symbols via `Arc<EventPublisher>` / `Arc<F::Socket>` (documented in `src/lib.rs`) and are not gated by the audit. ## Why thumbv7em-none-eabihf and not tricore The project's actual production target is Infineon AURIX TriCore. Mainline Rust does not have a TriCore target — `rustc --print target-list | grep tricore` returns nothing, and upstream LLVM does not ship a TriCore backend. Compiling Rust for TriCore today requires HighTec's commercial Rust distribution (or a custom LLVM build with their out-of-tree TriCore backend). `thumbv7em-none-eabihf` is the closest no_std proxy mainline Rust supports and runs for free in GitHub Actions: - Same `no_std` posture (no `extern crate std`). - Same alloc-optionality (no implicit allocator). - Same `core::*` / `alloc::*` surface. - Same fixed-width integer / atomic widths as TC1.6. What the proxy does NOT prove for TriCore: - LLVM TriCore-specific codegen edge cases. - Atomic-instruction lowering on the actual chip. - `critical-section` impl behavior under TriCore's split ISR / main-thread context model. A future phase 20 will swap (or layer) this CI step onto a TriCore HighTec runner once that infrastructure is in place. For now, the cortex-m4f proxy is the strongest verification CI can give us without a TriCore toolchain. Verified locally: cargo build --target thumbv7em-none-eabihf --no-default-features --features bare_metal ✓ cargo build --target thumbv7em-none-eabihf --no-default-features --features client,bare_metal ✓ cargo build --target thumbv7em-none-eabihf --no-default-features --features server,bare_metal ✓ cargo build --target thumbv7em-none-eabihf --no-default-features --features client,server,bare_metal ✓ alloc-symbol audit: client+bare_metal = 0 alloc references in rlib ✓ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final sub-phase for the literal no_std compile gate. Folds 18a-18e
into the existing 0.8.0 CHANGELOG entry, updates the lib.rs and
README feature tables, and rewrites the bare-metal examples to use
the new no-alloc lock handles directly (no more `Arc<Mutex<_>>` /
`Arc<RwLock<_>>` placeholders).
## CHANGELOG
Folded into 0.8.0's existing Added / Changed / Notes sections:
Added:
- StaticSubscriptionHandle + StaticSubscriptionStorage
- server::Error::InvalidUsage(&'static str)
- E2ERegistryFull (typed overflow on E2ERegistry::register)
- PayloadWireFormat::for_each_offered_endpoint /
for_each_service_instance visitor methods
Changed (breaking, queued for 0.8.0):
- client / server features no longer imply std (moved to
*-tokio); client compiles in pure no_std, server pulls
extern crate alloc for Arc<EventPublisher> /
Arc<F::Socket>.
- futures dep replaced with futures-util (futures::select! is
std-gated; switched to select_biased!).
- Internal select! → select_biased! (top-arm-first instead of
pseudo-random; observable only under contrived workloads).
- PayloadWireFormat::offered_endpoints / service_instances
Vec-returning forms preserved as cfg(feature = "std")
convenience wrappers; trait now requires the visitor methods.
- PayloadWireFormat::set_reboot_flag and
new_subscription_sd_header no longer std-gated.
- OfferedEndpoint no longer std-gated; addr is
Option<core::net::SocketAddrV4>.
- server::Error::Io now cfg(feature = "std")-gated; misuse paths
return Error::InvalidUsage(tag) instead.
- SubscriptionManager::get_subscribers now
cfg(feature = "std")-only.
- server::ServiceInfo / server::EventGroupInfo now
cfg(feature = "std")-only.
- E2ERegistry: HashMap → heapless::FnvIndexMap (cap = 32);
register returns Result<(), E2ERegistryFull>; new() is const.
- E2ERegistryHandle::register trait method lifts the same
Result through every impl.
Notes:
- Bare-metal compile gate is now literal — cargo build
--target thumbv7em-none-eabihf --no-default-features --features
client,server,bare_metal succeeds in CI; client + bare_metal
is verified alloc-free.
- Known limitation: server pulls extern crate alloc; refactor
to &'static borrows tracked for v3 phase 21+.
## lib.rs feature table
Rewritten to honestly describe each feature:
- std: now described as the gate for the std lock-handle defaults
(Arc<Mutex<E2ERegistry>> etc.) used by tokio backends.
- client: pure no_std-clean, does not pull extern crate alloc.
- server: pulls extern crate alloc.
- client-tokio / server-tokio: imply client/server + std.
- bare_metal: lists all five no-alloc types
(static_channels, AtomicInterfaceHandle, StaticE2EHandle,
StaticSubscriptionHandle).
## README feature table
Mirrors lib.rs. Adds explicit note that the cross-build for
thumbv7em is verified in CI.
## Examples — bare_metal_client / bare_metal_server
Both now use the actual no-alloc handles end-to-end:
- StaticE2EHandle over &'static StaticE2EStorage (was
Arc<Mutex<E2ERegistry>>)
- AtomicInterfaceHandle over &'static AtomicU32 (was
Arc<RwLock<Ipv4Addr>>) -- bare_metal_client only
- StaticSubscriptionHandle over &'static
StaticSubscriptionStorage (was MockSubscriptions, ~75 LoC of
inline trait impl deleted) -- bare_metal_server only
Storage `static`s declared at module scope (clippy::pedantic
dislikes `static` after `let`). `E2ERegistry::new()` and
`SubscriptionManager::new()` are both const, so no Box::leak.
Both example Cargo.toml files now opt into the std feature
explicitly. The examples use RawPayload (std-only) and tokio for
their host-side mock drivers; firmware drops std and provides its
own PayloadWireFormat impl. Documented inline.
The "What is not yet demonstrated" stale section in
bare_metal_client is gone — there is nothing left undemonstrated;
the example covers the actual firmware-target shape end-to-end.
## Verification
cargo fmt --all --check ✓
cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic ✓
cargo clippy --no-default-features -- -D warnings -D clippy::pedantic ✓
cargo test --lib --all-features: 513 pass ✓
cargo run -p bare_metal_client ✓ (runs end-to-end)
cargo run -p bare_metal_server ✓ (announces + asserts SD sent)
cargo build --target thumbv7em-none-eabihf --no-default-features --features client,server,bare_metal ✓
Phase 18 (a through f) is complete. The literal "client + server
compile on cortex-m4f no_std" gate from bare_metal_plan_v3.md is
closed and CI-enforced. Phase 19 (embassy-net reference adapter)
is the next milestone per the v3 plan.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New crate at `simple-someip-embassy-net/` providing the reference no_std backend for `simple-someip`'s transport-trait surface. As of this commit, the crate is scaffolded only: - `Cargo.toml` depends on `simple-someip` (default-features = false, client+server+bare_metal) and `embassy-net = "0.4"` (the last release line that builds against `embassy-sync 0.6`, which is what simple-someip currently uses; bumping both deps in lockstep is its own future phase). - `src/lib.rs` declares `factory` and `socket` modules plus `pub use` re-exports for the eventual `EmbassyNetFactory` / `SocketPool` / `EmbassyNetSocket` surface. - `src/factory.rs` skeleton declares `SocketPool<POOL, RX_BUF, TX_BUF>` and `EmbassyNetFactory<'a, POOL, RX_BUF, TX_BUF>` types with stubbed-out fields (`_todo: ()`); actual buffer storage and the `TransportFactory` impl land in 19b. - `src/socket.rs` skeleton declares `EmbassyNetSocket` placeholder; full `TransportSocket` impl lands in 19c. - `README.md` documents target shape (post-19c) and the surrounding bare-metal-plan-v3 phase 19 framing. Workspace `Cargo.toml` adds the new member. Verification: cargo build -p simple-someip-embassy-net ✓ cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf ✓ cargo build --workspace --all-features ✓ cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic ✓ cargo fmt --all --check ✓ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Real implementation of `TransportFactory` over embassy-net 0.4. The adapter now claims a buffer slot from a caller-declared `&'static SocketPool<POOL, RX_BUF, TX_BUF>` on each `bind()`, constructs an `embassy_net::udp::UdpSocket` borrowing the slot's RX/TX buffers, and reclaims the slot when the returned `EmbassyNetSocket` drops. ## SocketPool - `pub struct SocketPool<const POOL: usize, const RX_BUF: usize, const TX_BUF: usize>` - Holds `[Slot<RX_BUF, TX_BUF>; POOL]` of `UnsafeCell`-wrapped buffers + `[AtomicBool; POOL]` of in-use flags. - `const fn new()` so the pool can live in a plain `static` declaration. - `unsafe impl Sync` justified by the AcqRel CAS handshake serializing per-slot UnsafeCell access. - 4-entry `PacketMetadata` arrays per direction (constant `PACKET_METADATA_LEN = 4`) — sized for SOME/IP-SD's announcement + occasional Subscribe burst pattern. ## EmbassyNetFactory - `pub struct EmbassyNetFactory<'pool, D, POOL, RX_BUF, TX_BUF>` generic over the embassy-net `Driver` and the pool dimensions. - `impl TransportFactory` only for `'pool = 'static` (the trait needs `F::Socket: 'static`); an unsafe lifetime lift in `bind()` carries the pool reference into the socket. SAFETY argument: the lift is identity at the impl-bound `'static`, and the per-slot CAS handshake gives the same exclusion guarantees as a Mutex would. - `bind()` returns `Err(TransportError::AddressInUse)` on pool exhaustion (closest existing variant; a future `TransportError::PoolExhausted` would be a small additive change). ## EmbassyNetSocket - Wraps `UdpSocket<'static>` with the slot index + a `&'static dyn SlotReclaim` for free-list release on `Drop`. - The `SlotReclaim` trait erases the pool's three const generics from the socket type signature, keeping `EmbassyNetSocket` declaration-clean. - `Drop` calls `inner.close()` (releases the smoltcp slot) and then `reclaim.release(slot_index)`. ## TransportSocket impl (stub) The `TransportFactory::Socket: TransportSocket` bound forces a trait impl on `EmbassyNetSocket` for the factory to typecheck. 19b ships a minimum-viable stub: - `send_to` / `recv_from` futures resolve to `Err(TransportError::Unsupported)` (real `poll_send_to` / `poll_recv_from`-driven named futures land in 19c). - `local_addr` returns the bind-time SocketAddrV4. - `join_multicast_v4` / `leave_multicast_v4` return `Ok(())` because embassy-net's multicast-group join lives on `Stack` (async) — the user is expected to call `stack.join_multicast_group(...)` before constructing the factory. Documented prominently on `EmbassyNetFactory`. Until 19c lands, attempting actual I/O through a bound socket fails with `Unsupported`. The 19b commit verifies the pool-claim / pool-release / Drop wiring without requiring the full embassy-net I/O bring-up. Verification: cargo build -p simple-someip-embassy-net ✓ cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf ✓ cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic ✓ cargo fmt --all --check ✓ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the 19b stubs (Ready<Err(Unsupported)>) with named futures that drive embassy-net's poll_send_to / poll_recv_from directly: - EmbassyNetSendFut<'a> / EmbassyNetRecvFut<'a> are hand-rolled Future structs, not async-block wrappers. Each datagram costs zero allocations on the hot path. - Error mapping: SendError::NoRoute -> Io(NetworkUnreachable), SendError::SocketNotBound -> Io(Other) (programming error, bind always precedes return), RecvError::Truncated -> Io(Other), IPv6 source endpoint -> Unsupported. - Address conversions (socket_addr_v4_to_endpoint / endpoint_to_socket_addr_v4) include a defensive wildcard arm so cargo feature-unification pulling in smoltcp's proto-ipv6 doesn't silently break exhaustiveness. - factory.rs: drop the _phantom_io_error_kind_use placeholder; IoErrorKind is now actually used downstream in socket.rs. Open question (per plan v3 phase 19c) resolved: embassy-net 0.4's UdpSocket exposes poll_send_to / poll_recv_from directly, so the named-future shape works without an intermediate shim. Gates green: - cargo fmt --check - cargo clippy -p simple-someip-embassy-net --all-targets -D warnings - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo check --workspace --all-targets What this leaves for 19d: kill heap Vec<u8> from SD-emission paths (Server::send_subscribe_ack_from_view, send_subscribe_nack_from_view, send_unicast_offer, SdStateManager::send_offer_service) in favor of stack [u8; UDP_BUFFER_SIZE] matching EventPublisher::publish_event. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ndtrip) Validates 19a-c against a real embassy_net::Stack: - `LoopbackDriver` pair: two in-memory `Pipe`s (queue + waker) bridge two `embassy_net::Stack` instances. No kernel TUN, no privileges; runs in any CI without setup. `HardwareAddress::Ip` medium skips ARP/Ethernet — pure IP traffic over the loopback. - `adapter_udp_roundtrip`: two stacks on 169.254.1.1 / .1.2, two `EmbassyNetFactory` + `SocketPool` pairs, bind a socket on each, send a UDP datagram A→B, assert byte-equality + source address. Tightest end-to-end exercise of `bind` / `send_to` / `recv_from` / `local_addr` / `SocketPool` slot lifecycle. - Runtime: `#[tokio::test(flavor = "current_thread")]` + a `LocalSet` driving per-stack `spawn_local` runners. `Stack<LoopbackDriver>` is `!Sync` (RefCell internals), so `Stack::run()` is `!Send` — multi-thread `tokio::spawn` does not type-check. The `current_thread` flavor matches the single-task model bare-metal targets actually run under. - Cargo: adds `tokio` (rt-multi-thread, macros, time, sync) and `futures` to dev-deps. What this leaves for follow-on phases: - 19f — `Server::new_with_deps_local` + parallel `impl Server` block with relaxed `Send + Sync` bounds. Required because `embassy_net::udp::UdpSocket<'static>` is `!Sync` (borrows from `Stack`'s `RefCell<Inner<D>>`), and Server's existing three impl blocks (`mod.rs:275/430/1065`) all require `F::Socket: Send + Sync`. Mirrors Client's existing `new_with_deps_local` pattern. - 19g — SOME/IP Client+Server integration test. Lifts the `tests/bare_metal_e2e.rs` harness onto the loopback stack pair using the 19f `_local` API. Mirrors the parent crate's `client_receives_server_sd_announcement` and `client_send_request_server_runloop_stable`, with `EmbassyNetFactory` swapped in for `MockFactory`. Scope split rationale: per phase_13_5_lessons.md lesson #2, "abstract over X" and "drop X" are separate commitments — bundling the Server bound-relaxation under "loopback test" would repeat the v2 phases-11/13a aspirational-gate mistake. Plan v3 updated 2026-04-29 (memory) with the 19e/f/g/h/i/j re-numbering. Gates green: - cargo fmt --check - cargo clippy -p simple-someip-embassy-net --all-targets -D warnings - cargo test -p simple-someip-embassy-net --test loopback - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo check --workspace --all-targets Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Missed in 10fdfdc; adds tokio + futures-util + transitive deps that the loopback test pulls in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `SocketHandle` trait that abstracts how the transport socket
is stored and shared (`Arc<T>` on std, `StaticSocketHandle<T>` on
bare metal). `Server` and `EventPublisher` are now generic over
`H: SocketHandle<Socket = F::Socket>` rather than holding
`Arc<F::Socket>` directly. This unblocks consumers whose `F::Socket`
is `!Sync` — most notably `embassy-net`'s `UdpSocket<'static>`,
which borrows from `Stack`'s `RefCell<Inner>` and so cannot satisfy
the previous `F::Socket: Send + Sync` bound.
Trait shape (matches `SubscriptionHandle`'s permissive bound profile):
pub trait SocketHandle: Clone + 'static {
type Socket: TransportSocket + 'static;
fn socket(&self) -> &Self::Socket;
}
pub trait WrappableSocketHandle: SocketHandle {
fn wrap(socket: Self::Socket) -> Self;
}
`WrappableSocketHandle` is split out because `StaticSocketHandle`
deliberately cannot `wrap` without an allocator — `Box::leak` /
static-cell init can't be expressed inside a trait method that
returns `Self`. Server's existing constructors (which bind sockets
internally and need to wrap) require `H: WrappableSocketHandle`;
no-alloc consumers using `StaticSocketHandle` need a future
external-bind constructor variant (out of scope).
Impls shipped:
- `Arc<T>: SocketHandle + WrappableSocketHandle` in `std_handle_impls`.
- `StaticSocketHandle<T>: SocketHandle` (no-alloc) in
`bare_metal_handle_impls`.
Changes:
- `transport.rs`: add `SocketHandle` + `WrappableSocketHandle`
traits, the `Arc<T>` impl, and `StaticSocketHandle<T>`.
- `server/event_publisher.rs`: replace `T: TransportSocket + Send +
Sync` + `socket: Arc<T>` field with `H: SocketHandle` + `socket: H`.
All `self.socket.send_to(...)` become `self.socket.socket().send_to(...)`.
- `server/mod.rs`: add `H = Arc<F::Socket>` default type parameter
to `Server`. Drop defensive `F: Send + Sync`, `F::Socket: Send +
Sync`, `Tm: Send + Sync` from struct decl + all three impl blocks
(mod.rs:275, :430, :1065). Move `+ Send` return-type bound from
the impl-block level to method-level on `announcement_loop` (so
the bound is enforced at the call site, not propagated through
every method). Add `announcement_loop_local` returning `impl
Future + 'static` (no Send) for single-threaded executors over
`!Sync` transports. Replace `Arc::new(socket)` constructor calls
with `H::wrap(raw_socket)`.
- `tests/client_server.rs`: update `TestEventPublisher` type alias
to spell `Arc<TokioSocket>` for the new `H` slot.
Plus a single `Arc<FailingSocket>` annotation on the SD-NACK
regression test (mod.rs:1345) so type inference doesn't have to
reach across the deps-bundle indirection to find `H`.
Why C and not B (drop Send+Sync alone): the user explicitly asked
for the architecturally clean answer matching the existing handle-
abstraction pattern (`InterfaceHandle` / `E2ERegistryHandle` /
`SubscriptionHandle`). C ships that; B would have just relaxed
bounds without giving bare-metal-no-alloc consumers a path.
What this leaves for 19g:
- Lift `tests/bare_metal_e2e.rs`'s harness onto the loopback stack
pair from 19e using `Server::new_with_deps` (works for
`Arc<EmbassyNetSocket>` H) and `announcement_loop_local`. Mirrors
the parent's `client_receives_server_sd_announcement` and
`client_send_request_server_runloop_stable` tests with
`EmbassyNetFactory` swapped in for `MockFactory`.
What this leaves for a future phase (no-alloc Server):
- Add `Server::new_with_handles` / `new_passive_with_handles` that
take pre-built `H: SocketHandle` instances directly rather than
binding internally. Required for bare-metal-no-alloc consumers
using `StaticSocketHandle`.
Gates green:
- cargo build --workspace --all-targets
- cargo build --no-default-features --features client,server,bare_metal
- cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf
- cargo test --features client-tokio,server-tokio --test client_server (11/11 pass, serialized to avoid pre-existing port-reuse races)
- cargo test -p simple-someip-embassy-net --test loopback (1/1 pass)
- cargo fmt --check
- cargo clippy --tests (2 pre-existing pedantic warnings on phase-18a heapless code, unrelated)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lifts the parent crate's `tests/bare_metal_e2e.rs` harness onto the two-stack `LoopbackDriver` pair from 19e, using 19f's relaxed `Server` bounds (`SocketHandle` abstraction) so an `Arc<EmbassyNetSocket>` — `!Sync` because embassy-net's `UdpSocket<'static>` borrows from `Stack`'s `RefCell<Inner<D>>` — satisfies `H` for the Server. Two new tests in `simple-someip-embassy-net/tests/loopback.rs`: - `client_receives_server_sd_announcement`: real Server on stack A emits SD `OfferService` via `announcement_loop_local` (19f's `!Send` variant), real Client on stack B receives it via `bind_discovery`. Asserts a `ClientUpdate::DiscoveryUpdated` surfaces within 5 s. Both stacks join 224.0.23.0 at the smoltcp level before construction (the adapter's `join_multicast_v4` is a documented no-op since multicast group membership lives on the `Stack`, not the `UdpSocket`). - `client_send_request_server_runloop_stable`: passive Server on stack A; Client on stack B does `add_endpoint` + `send_to_service` to push a SOME/IP request through the loopback. Asserts the send returns Ok and the run-loop survives. No response assertion because `simple_someip::Server` exposes no public request-handler API — matching the parent crate's reference test. Harness additions: - `define_static_channels!` `LoopbackTestChannels` with the same pool sizing shape as `tests/bare_metal_e2e.rs`'s `E2ETestChannels`. - `LocalTokioSpawner: LocalSpawner` over `tokio::task::spawn_local`. `LocalSpawner` (not `Spawner`) because the Client's run-future captures `&self.unicast_socket`-style borrows across awaits over a `!Sync` socket, making the run future itself `!Send`. - `LocalTimer: Timer` over `tokio::time::sleep`, boxed-future shape matching the bare-metal-e2e `MockTimer`. - `MockSubscriptions(Arc<Mutex<Vec<SubKey>>>)` — same shape as bare-metal-e2e's mock. Cargo.toml: dev-dependency `simple-someip` is now re-pinned with `features = ["client", "server", "bare_metal", "std"]`. The `std` addition gates `RawPayload` / `VecSdHeader` / `Arc<Mutex<E2ERegistry>>` default impls — all needed for the host-side test harness — without affecting the production `[dependencies]` build (which stays `default-features = false`). Type-inference notes: - Both Server constructions in the new tests carry an explicit `Server<_, _, _, _, Arc<simple_someip_embassy_net::EmbassyNetSocket>>` annotation. Without it the compiler can't decide `H` across the `ServerDeps` indirection — same situation as `simple_someip`'s own SD-NACK test (`mod.rs:1345`) addresses. - `embassy_net::Stack::join_multicast_group` takes `T: Into<IpAddress>` but embassy-net 0.4 has no `core::net::Ipv4Addr -> IpAddress` blanket impl. Constructed via `embassy_net::Ipv4Address(addr.octets())` per the smoltcp shape. Gates green: - cargo fmt --check - cargo clippy -p simple-someip-embassy-net --all-targets -D warnings - cargo test -p simple-someip-embassy-net --test loopback (3/3 pass) - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo check --workspace --all-targets Phase 19 status: 19a-g all complete. Remaining 19 sub-phases: - 19h — `examples/embassy_net_client/` in-tree binary example. - 19i — CI: cross-build for thumbv7em + run loopback test. - 19j — Adapter README / CHANGELOG / `simple-someip-embassy-net` 0.1.0 release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a host-runnable workspace member that wires `simple-someip`
through the `simple-someip-embassy-net` adapter end-to-end. Mirrors
the in-process two-stack pattern from
`simple-someip-embassy-net/tests/loopback.rs` but as a concrete
binary with `println!` output, so a firmware author can `cargo run
-p embassy_net_client` and see the SD `OfferService` flow through
the loopback before swapping in their hardware MAC driver.
What it shows:
- `LoopbackDriver` pair as the in-memory stand-in for a hardware
driver. The `Driver` trait it implements is the same one a
vendor MAC implements; replacement is a one-file swap.
- Static `SocketPool<8, 1500, 1500>` declaration per side.
- `define_static_channels!` for the Client's no-alloc channel
pool.
- `LocalSpawner` impl backed by `tokio::task::spawn_local` (real
firmware swaps for `embassy_executor::Spawner::spawn`).
- `Timer` impl backed by `tokio::time::sleep` (real firmware
swaps for `embassy_time::Timer::after`).
- `Server::new_with_deps` with `H = Arc<EmbassyNetSocket>`
(default in 19f), `Server::announcement_loop_local` (the !Send
variant, required because `EmbassyNetSocket: !Sync`).
- `Client::new_with_deps_local` (the LocalSpawner-bounded path).
Output (verified locally):
[server] announcement loop spawned, emitting OfferService(0x5BAA) every 1s
[client] discovery bound on 169.254.1.2:30490
[client] received SD update: DiscoveryUpdated(...)
[example] roundtrip complete; exiting
Why tokio not embassy_executor (deviation from plan v3 19h text):
matches the established convention from `examples/bare_metal_client/`
which also uses tokio for the host runtime. Plan v3 named
`#[embassy_executor::main]` aspirationally; the established repo
pattern is "simple-someip in bare-metal mode, host runtime from
tokio." A user who wants `embassy_executor::main` specifically
can swap the `#[tokio::main]` line — the rest of the wiring is
unchanged.
Cargo:
- New workspace member `examples/embassy_net_client`.
- Deps pinned to match the adapter's transitive deps (embassy-net
0.4, embassy-sync 0.6) so cargo doesn't fork the dep tree.
- `embassy-time` with `std + generic-queue-8` features for the
host time driver embassy-net's TCP/IGMP code uses internally.
Gates green:
- cargo fmt --check
- cargo clippy -p embassy_net_client --all-targets -D warnings
- cargo build -p embassy_net_client
- cargo run -p embassy_net_client (exits cleanly on SD message)
- cargo check --workspace --all-targets
- cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf
What this leaves for 19i:
- CI job: cross-build `simple-someip-embassy-net` for thumbv7em
(already passes locally; needs a CI matrix entry) plus run the
adapter's loopback test in the standard host job.
What this leaves for 19j:
- README in `simple-someip-embassy-net/` explaining the adapter.
- CHANGELOG entry covering 19a-h as the 0.1.0 surface.
- Tag and publish `simple-someip-embassy-net` 0.1.0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces `Server::run_with_buffers(&mut self, unicast_buf: &mut [u8], sd_buf: &mut [u8])` as the no-alloc-friendly entry point for the server event loop. The existing `Server::run` becomes a thin convenience shim that heap-allocates two 65535-byte buffers via `alloc::vec!` and delegates. Why: bare-metal consumers (TC4D + future no-alloc targets) cannot call `Server::run` because it pulls in `alloc::vec![0u8; 65535]` for the recv buffers. Splitting the buffer allocation out of the event loop body lets those consumers supply their own storage (typically `static`- declared `[u8; 65535]` arrays) while leaving std consumers' ergonomics unchanged. Pre-existing 64 KiB sizing rationale carries over verbatim to `run_with_buffers`'s docs: peer SD messages are bounded by link MTU, but the server is a sink for any peer datagram landing on its SD/unicast port — a smaller buffer would silently truncate larger-than-MTU peer messages instead of surfacing them. Caller picks an appropriate size for their target. Reborrow nuance: inside the loop, `recv_from(&mut *buf)` rather than `recv_from(&mut buf)` because `unicast_buf` / `sd_buf` are now `&mut [u8]` parameters, not owned `Vec<u8>` locals. Direct `&mut buf` would produce `&mut &mut [u8]`. Clears 20-pre alloc audit's category-D recv-buffer item without breaking any std-side caller. Existing tests pass: - 11 client_server tests (serialized to avoid pre-existing port races) - 2 bare_metal_e2e tests - 3 simple-someip-embassy-net loopback tests Doesn't yet eliminate the other 20-pre findings: - D / 19f H = Arc<F::Socket> default — handled by separate `Server::new_with_handles` work - E.1 Arc<EventPublisher> — handled by separate `EventPublisherHandle` work - E.2 Arc<SdStateManager> — handled by separate `SdStateHandle` work What this leaves: the bare-metal consumer must still hold the 65535-byte buffers in static storage (or wherever the firmware can spare 128 KB total recv-buffer RAM). On TC4D specifically with a typical RAM budget the size may need to shrink to something like 1500 + a documented truncation caveat — that's a per-consumer decision now exposed via the new API surface. Gates green: - cargo fmt --check - cargo clippy --tests (2 pre-existing warnings, unrelated) - cargo build --workspace --all-targets - cargo build --no-default-features --features client,server,bare_metal - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo test --features client-tokio,server-tokio --test client_server -- --test-threads=1 - cargo test --features client,server,bare_metal --test bare_metal_e2e - cargo test -p simple-someip-embassy-net --test loopback Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `SdStateHandle` + `WrappableSdStateHandle` traits in `src/server/sd_state.rs` and threads them through `Server` as a new `Hsd` type parameter (default `Arc<SdStateManager>`). Mirrors the pattern established by 19f's `SocketHandle` / `WrappableSocketHandle`. Same shape, same Send/Sync defaults (neither bound at trait level — caller adds at use sites). Two impls ship: - `Arc<SdStateManager>: SdStateHandle + WrappableSdStateHandle` (the existing default; preserves std-side behavior). - `&'static SdStateManager: SdStateHandle` (no-alloc; user declares `static SD_STATE: SdStateManager = SdStateManager::new();` and supplies `&SD_STATE` via a future `Server::new_with_handles` constructor). `SdStateManager` itself becomes `pub` and `SdStateManager::new()` becomes `pub const fn` so the static-storage pattern compiles. The internal methods (`next_session_id_with_reboot_flag`, `reboot_flag`, `send_offer_service`) stay `pub(super)` — consumers shouldn't call them directly; they go through Server. Server's existing `new_with_deps` / `new_passive_with_deps` constructors require `Hsd: WrappableSdStateHandle` because they build the manager internally via `SdStateManager::new()` then `Hsd::wrap(...)`. The future `Server::new_with_handles` will take `Hsd: SdStateHandle` directly (no `wrap` step), enabling the no-alloc path with `&'static SdStateManager`. `announcement_loop`'s method-level `where` clause picks up the new `Hsd: Send + Sync` bound, mirroring the existing `H: Send + Sync` and `F: Send + Sync` bounds. The `_local` variant has no such requirement and works for any `Hsd: SdStateHandle`. Type-signature width: Server now reads `Server<R, S, F, Tm, H = Arc<F::Socket>, Hsd = Arc<SdStateManager>>`. Both defaults preserve every existing call site — `Server<R, S, F, Tm>` and `Server<R, S, F, Tm, Arc<SomeSocket>>` both still resolve correctly. No churn in `tests/` or `examples/`. Clears 20-pre alloc audit's category-E.2 finding. Combined with the 4f9d36e recv-buffer split, two of the four "no-alloc Server" remediation items are done. What this leaves: - E.1: `Arc<EventPublisher<R, S, H>>` field on Server. Same shape via an `EventPublisherHandle` trait — next branch. - D: `Server::new_with_handles` constructor that takes pre-built `H` + `Hsd` (and the future `Hep` for E.1) directly, skipping the `wrap` step. Lands after E.1 so the constructor's parameter list is final. Gates green: - cargo fmt --check - cargo clippy --tests (2 pre-existing warnings, unrelated) - cargo build --workspace --all-targets - cargo build --no-default-features --features client,server,bare_metal - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo test --features client-tokio,server-tokio --test client_server -- --test-threads=1 (11/11) - cargo test --features client,server,bare_metal --test bare_metal_e2e (2/2) - cargo test -p simple-someip-embassy-net --test loopback (3/3) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…irement Mirrors phase 20b's `SdStateHandle` work for the second of the two production-path Arc usages flagged by the 20-pre alloc audit (finding E.1). Adds: - `EventPublisherHandle<R, S, H>: Clone + 'static` trait in `src/server/event_publisher.rs`. Method: `fn publisher(&self) -> &EventPublisher<R, S, H>`. - `WrappableEventPublisherHandle<R, S, H>: EventPublisherHandle<...>` extension trait with `fn wrap(EventPublisher<R, S, H>) -> Self`. - `Arc<EventPublisher<R, S, H>>: EventPublisherHandle + WrappableEventPublisherHandle` (alloc-using std-side default; preserves existing behavior). - `&'static EventPublisher<R, S, H>: EventPublisherHandle` (no-alloc bare-metal path; user declares the static, supplies the reference into a future `Server::new_with_handles`). Server gains a third type parameter `Hep = Arc<EventPublisher<R, S, H>>` (default), threaded through the struct decl and all three impl blocks. Field `publisher: Arc<EventPublisher<R, S, H>>` becomes `publisher: Hep`. `Arc::new(EventPublisher::new(...))` in constructors becomes `Hep::wrap(EventPublisher::new(...))`. `Server::publisher()` accessor return type changes from `Arc<EventPublisher<R, S, H>>` to `Hep` — non-breaking for std users who pick up the default; bare-metal users get their chosen handle type back. Existing call sites pick up the `Hep` default; no churn in `tests/`, `examples/`, or any caller. All three Arc impls (SocketHandle, SdStateHandle, EventPublisherHandle) follow the same pattern: `Arc<T>` for std (alloc), `&'static T` for bare-metal (no-alloc), `Wrappable*` extension for the inline- construction path. Three of four remediation items in the 20-pre alloc audit are now done: the recv buffer (20a), `Arc<SdStateManager>` (20b), and `Arc<EventPublisher>` (20c). The last item — combining all three handle types into a single `Server::new_with_handles` constructor that accepts pre-built handles directly without the `wrap` step — lands in 20d. Type-signature width: Server now reads `Server<R, S, F, Tm, H = Arc<F::Socket>, Hsd = Arc<SdStateManager>, Hep = Arc<EventPublisher<R, S, H>>>`. Three defaults preserve every existing call site. After 20d, a bare-metal caller will spell out all three explicitly via the new constructor, and a std caller will keep accepting all three defaults via `Server::new_with_deps`. Gates green: - cargo fmt --check - cargo clippy --tests (2 pre-existing warnings, unrelated) - cargo build --workspace --all-targets - cargo build --no-default-features --features client,server,bare_metal - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo test --features client-tokio,server-tokio --test client_server -- --test-threads=1 (11/11) - cargo test --features client,server,bare_metal --test bare_metal_e2e (2/2) - cargo test -p simple-someip-embassy-net --test loopback (3/3) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the no-alloc-friendly counterparts to `Server::new_with_deps` and `Server::new_passive_with_deps`. Caller supplies all four storage handles (`H` for sockets, `Hsd` for SD state, `Hep` for EventPublisher) pre-built; Server stores them directly without calling `factory.bind(...)` internally and without invoking any `Wrappable*Handle::wrap` step. This is the constructor path bare-metal-no-alloc consumers need: they own their UDP transport (lwIP, vendor IP stack, etc.), bind sockets externally, and wrap them via `StaticSocketHandle` / `&'static SdStateManager` / `&'static EventPublisher<...>` — materializing the static storage themselves at boot. Surface additions: - `pub struct ServerHandles<F, Tm, R, S, H, Hsd, Hep>` — bundle of factory + timer + e2e_registry + subscriptions + the four pre-built handles. Mirrors `ServerDeps` for the same caller ergonomics. - `Server::new_with_handles(deps, config) -> Result<Self, Error>` — constructs an active server (announcement loop runnable, run loop runnable). Back-fills `config.local_port` from `unicast_socket.local_addr()` so SD offers advertise the bound port. - `Server::new_passive_with_handles(deps, config) -> Result<Self, Error>` — same shape, marks `is_passive = true`. - Re-exported via `simple_someip::ServerHandles`. Both constructors live in the existing `impl@521` block whose bounds (`H: SocketHandle`, `Hsd: SdStateHandle`, `Hep: EventPublisherHandle` — all without `Wrappable*`) match what the no-alloc path requires. Both are synchronous (`pub fn`, not `pub async fn`) — no `factory.bind()` to await. Std users who prefer the async- ergonomic path keep using the existing `new_with_deps` / `new_passive_with_deps`. Combined with phases 20a-c, this completes the four-item "no-alloc Server completion" remediation surfaced by the 20-pre alloc audit: - 20a: `run_with_buffers` — caller-provided recv buffer (clears audit category-D recv-buffer item). - 20b: `SdStateHandle` — drops `Arc<SdStateManager>` (clears audit E.2). - 20c: `EventPublisherHandle` — drops `Arc<EventPublisher>` (clears audit E.1). - 20d: this commit — `new_with_handles` + `new_passive_with_handles` (clears audit category-D socket-Arc item by exposing the pre-built-handle path). A consumer building TC4D firmware with `default-features = false, features = ["client", "server", "bare_metal"]` and banning `extern crate alloc` can now construct a Server via `Server::new_with_handles(...)` using `&'static`-backed handles, drive it via `run_with_buffers(&mut [u8; N], &mut [u8; N])` over `static`-declared receive buffers, and emit SD via `announcement_loop_local`. Zero `alloc::*` surfaces in any production code path under that feature combo. What this leaves: an actual no-alloc bare-metal example / integration test against `simple-someip-embassy-net` (or a future `simple-someip-lwip` adapter) using these constructors. That's a separate "validation" commit — 20d ships the API; the witness comes when the lwip adapter exists. Gates green: - cargo fmt --check - cargo clippy --tests (2 pre-existing warnings, unrelated) - cargo build --workspace --all-targets - cargo build --no-default-features --features client,server,bare_metal - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo test --features client-tokio,server-tokio --test client_server -- --test-threads=1 (11/11) - cargo test --features client,server,bare_metal --test bare_metal_e2e (2/2) - cargo test -p simple-someip-embassy-net --test loopback (3/3) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapses the three near-identical handle traits introduced in
19f / 20b / 20c into a single generic trait pair:
pub trait SharedHandle<T: 'static>: Clone + 'static {
fn get(&self) -> &T;
}
pub trait WrappableSharedHandle<T: 'static>: SharedHandle<T> {
fn wrap(value: T) -> Self;
}
Two blanket impls cover the alloc and no-alloc paths:
impl<T: 'static> SharedHandle<T> for &'static T { ... }
impl<T: 'static> SharedHandle<T> for Arc<T> { ... } // alloc-gated
impl<T: 'static> WrappableSharedHandle<T> for Arc<T> { ... } // alloc-gated
Deleted (each was a slot-specific copy of this same shape):
- `SocketHandle` / `WrappableSocketHandle` (transport.rs).
- `SdStateHandle` / `WrappableSdStateHandle` (server/sd_state.rs).
- `EventPublisherHandle<R, S, H>` /
`WrappableEventPublisherHandle<R, S, H>` (server/event_publisher.rs).
- `StaticSocketHandle<T>` (the `&'static T` blanket impl
subsumes its only purpose: carrying the `'static` lifetime).
Net trait count: 6 + 1 wrapper struct → 2 traits. ~60% less
boilerplate across the three former trait modules.
`Server`'s three handle bounds become uniform:
- `H: SharedHandle<F::Socket>`
- `Hsd: SharedHandle<SdStateManager>`
- `Hep: SharedHandle<EventPublisher<R, S, H, F::Socket>>`
`EventPublisher` gains an explicit `T: TransportSocket`
parameter — the price of carrying `T` as a generic on
`SharedHandle<T>` rather than as an associated type. The
struct grows a `PhantomData<T>` field so the type parameter is
well-formed without affecting drop-check.
Method calls collapse to one name everywhere:
- `H::socket()` / `Hsd::sd_state()` / `Hep::publisher()` →
`.get()` (consistent across all three slot types).
Default type-param expressions adjust to spell the `T`:
- `Hep = Arc<EventPublisher<R, S, H, <F as TransportFactory>::Socket>>`.
Test type-aliases gain the new `T` slot:
- `tests/client_server.rs::TestEventPublisher`
- `event_publisher.rs::TestEventPublisher` (internal)
- The `AlwaysFailSocket` regression-test type alias.
Pure rename / shape-change patch — no behavior change. The
runtime behavior of every public API call is identical to
20d's; this is type-system tidying motivated by the adversarial
review.
Adversarial-review observations addressed:
- "Three near-identical handle traits is a code smell" — fixed.
- "We didn't generalize this into a single Shared<T> trait" —
done.
Trade-off accepted: `EventPublisher` signature widens from
`<R, S, H>` to `<R, S, H, T>`. The cost is one extra type
parameter at the EventPublisher layer; the win is removing six
trait definitions and one wrapper struct.
Gates green:
- cargo fmt --check
- cargo clippy --tests (2 pre-existing warnings, unrelated)
- cargo build --workspace --all-targets
- cargo build --no-default-features --features client,server,bare_metal
- cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf
- cargo test --features client-tokio,server-tokio --test client_server -- --test-threads=1 (11/11)
- cargo test --features client,server,bare_metal --test bare_metal_e2e (2/2)
- cargo test -p simple-someip-embassy-net --test loopback (3/3)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First test in the simple-someip crate that catches **protocol non-compliance** bugs against an external SOME/IP-SD reference (the COVESA vsomeip implementation), rather than running our own impl on both sides of the wire and only catching internal- consistency issues. Scope: single SD `OfferService` reception. simple-someip's `Client::bind_discovery()` listens for vsomeip's announcement of a known service+instance pair, asserts the SD entry surfaces on the update stream within a 30 s timeout. That single signal is the load-bearing wire-conformance check we have zero of today. Subsequent phases will layer Subscribe/Ack roundtrips, request/response, E2E protect/check, etc. against the same reference. `#[ignore]`'d by default. The test depends on an external vsomeip Docker container being up — see the test file's module-level docs for the docker setup, the JSON config to mount, and the env-var (`SIMPLE_SOMEIP_TEST_INTERFACE`) to point at the test's listening interface. Phase 20g will wire this into CI via TestContainers-rs (or similar) once the manual setup is proven. Why ignored not a CI step yet: Per FW team confirmation, vsomeip-in-docker on CI is approved-in-principle but not yet stood up. Shipping the test infrastructure first lets the firmware team pick up the test locally for debugging during the codec-MVP integration; CI automation lands as 20g. Cleanup folded in: clippy warnings surfaced under broader feature combos (`--features client-tokio,server-tokio`) by prior phases: - `event_publisher.rs:57` doc-markdown `PhantomData` now backticked. - `event_publisher.rs:670/732` `clippy::type_complexity` `#[allow(...)]`'d on the test type aliases (with reason string explaining why). - `server/mod.rs:929` doc-markdown `TokioSocket` now backticked. - `sd_state.rs` `Default` impl added on `SdStateManager` (clippy::pedantic; bare-metal callers should still prefer the explicit `const` `new()` for `static` initializers). Default-features `cargo clippy --tests` had only 2 pre-existing warnings before this commit and still has only 2 after; no new warnings on the canonical CI gate. The broader-feature warnings were a 20e-introduced side-effect. Gates green: - cargo fmt --check - cargo clippy --tests (default features; 2 pre-existing warnings unrelated to this work) - cargo build --workspace --all-targets - cargo build --no-default-features --features client,server,bare_metal - cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf - cargo test --features client-tokio,server-tokio --test client_server -- --test-threads=1 (11/11) - cargo test --features client,server,bare_metal --test bare_metal_e2e (2/2) - cargo test -p simple-someip-embassy-net --test loopback (3/3) - cargo test --features client-tokio,server-tokio --test vsomeip_sd_compat (1 ignored as expected) What this leaves for 20g: - `tests/data/vsomeip-offerer/Dockerfile` building vsomeip from source. - TestContainers-rs (or equivalent) integration so `cargo test --features client-tokio,server-tokio --test vsomeip_sd_compat -- --ignored` works in a CI runner with Docker available. - vsomeip version pin matching whatever the FW team selects for production validation. - Subsequent conformance tests: Subscribe/Ack, request/response, E2E roundtrips. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `tools/size_probe/` workspace member that mirrors halo PR
#4429's `rust_simple_someip` C-callable FFI surface (header
encode/decode + E2E Profile 4/5 round-trips) and builds as a
`staticlib` for `thumbv7em-none-eabihf`. Used during phase
20-pre to estimate simple-someip's flash footprint.
Build + measure:
cargo build -p size_probe --release --target thumbv7em-none-eabihf
llvm-size target/thumbv7em-none-eabihf/release/libsize_probe.a
(rustup toolchain ships `llvm-size` under
`~/.rustup/toolchains/.../bin/`).
Why a probe instead of measuring simple-someip's rlib directly:
rlibs include compiler metadata that bloats them ~60×. A
staticlib with `extern "C"` entry points lets post-link
dead-code elimination strip everything an actual FFI consumer
wouldn't reach, giving a closer-to-real-world flash number.
First measurement (default release profile, no `opt-level=z`,
no LTO at the probe level): ~12 KB of simple-someip-specific
text + 14 KB of transitive dep code (heapless, thiserror,
tracing). Compiler-rt builtins and `core::fmt` chains aren't
simple-someip-unique — they're amortized firmware-wide — and
were excluded from the per-component breakdown.
NOT a production crate. Pure measurement tool. Includes a
panic-on-alloc stub `GlobalAlloc` to satisfy the link-target
requirement on builds where some transitive dep pulls
`extern crate alloc` even though the codec FFI surface itself
is alloc-free.
Why thumbv7em-none-eabihf and not the actual TC4D target:
halo's TriCore build pipeline uses an in-house LLVM-IR-to-
TriCore proxy + a private Docker image we don't have local
access to. cortex-m4f is the closest upstream-Rust-supported
target with similar code-density characteristics; gives a
defensible bracket for the real TC4D flash cost (likely within
±50% on the proxy toolchain).
Future use: when the Option-A stateful FFI surface lands,
re-add equivalent `extern "C"` shims for the new entry points
(`rust_handle_udp_rx`, `rust_tick`, etc.) and re-measure.
Lets us track the flash-cost delta from codec-only → full
state machines as that work progresses.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Makes phase 20f's `tests/vsomeip_sd_compat.rs` actually
runnable. Adds `tests/data/vsomeip-offerer/`:
- `Dockerfile` — multi-stage Ubuntu 22.04 base. Stage 1 builds
vsomeip 3.4.10 (the LumPDK / EnVision pinned version per
`LumPDK/packages/thirdparty/vsomeip/vsomeip.MODULE.bazel`)
from upstream tarball, plus our minimal C++ offerer. Stage 2
is a slim runtime image with just libvsomeip3 + the offerer
binary + entrypoint. ~463 MB final image.
- `offerer.cpp` — ~85 LOC. Calls `application->offer_service(0x1234,
0x0001, 1, 0)` and idles while vsomeip's SD subsystem emits
cyclic OfferService broadcasts.
- `offerer.json` — vsomeip configuration. Standard SD multicast
`224.0.23.0:30490` per spec defaults; cyclic_offer_delay=1000ms;
ttl=5s. `unicast` is templated at container start (see below).
- `entrypoint.sh` — substitutes `VSOMEIP_UNICAST` env var into the
JSON before exec'ing the offerer. Bails loudly if the env var
isn't set. The substitution exists because `unicast: 127.0.0.1`
doesn't work on Linux — `lo` lacks the `MULTICAST` flag by
default, so SD multicast never actually leaves the host. Caller
must pick a real interface IP via `ip route get 224.0.23.0`.
- `CMakeLists.txt` — builds offerer against `find_package(vsomeip3)`.
- `README.md` — full build + run + test invocation flow with the
multicast-on-lo gotcha documented.
Test file (`tests/vsomeip_sd_compat.rs`) module docs updated to
match the new harness shape. The `#[ignore]`'d test itself is
unchanged from 20f.
Verified end-to-end on 2026-04-29:
docker build --network=host -t vsomeip-offerer tests/data/vsomeip-offerer/
docker run --rm -d --name vsomeip-offerer --network host \
-e VSOMEIP_UNICAST=172.20.21.206 vsomeip-offerer
SIMPLE_SOMEIP_TEST_INTERFACE=172.20.21.206 \
cargo test --features client-tokio,server-tokio \
--test vsomeip_sd_compat -- --ignored --nocapture
# client_sees_vsomeip_offer_service ... ok in 0.59s
This is the FIRST wire-level conformance signal in the project.
Every prior test ran simple-someip on both sides of the wire and
couldn't catch protocol non-compliance against an external
reference. Today: simple-someip's Client successfully decoded a
real vsomeip-emitted SD `OfferService` entry — service ID,
instance ID, TTL, major/minor version, source address all
matched the spec.
What this proves:
- vsomeip 3.4.10 builds + runs from upstream source in our docker
- simple-someip's SD-receive code path is wire-conformant against
vsomeip's SD-emit path for OfferService entries (one rung)
What this does NOT prove (worth being explicit about):
- Anything on TC4D — all of this is x86_64 Linux + native upstream
Rust + tokio. No proxy LLVM-IR-TriCore exercise.
- Bidirectional wire compatibility — we only tested vsomeip ->
simple-someip. The reverse (simple-someip emits SD that vsomeip
parses) is the next test (phase 20h).
- Other SD entry types — FindService, SubscribeEventGroup,
SubscribeAck, SubscribeNack are all separate code paths.
- Anything stateful — request/response correlation, subscription
state, event publishing, E2E protect/check on real payloads.
- The lwip transport story — vsomeip uses its own UDP socket; nothing
about Halo's planned lwip integration was tested.
- The Option-A FFI shape — doesn't exist yet. This test went
through simple-someip's existing tokio/`Client` API, which
Halo won't use in production.
CI integration deferred. The test stays `#[ignore]`'d by default;
flipping it on `cargo test` would fail until a CI runner has
docker + the harness available. That's the next phase (20i?)
once we have the full conformance test set built out.
What this leaves:
- 20h: bidirectional SD test (simple-someip emits, vsomeip
subscribes; proves TX wire format).
- 20i+: SubscribeEventGroup roundtrip, request/response, E2E
conformance.
- Eventual CI: TestContainers-rs (or equivalent) to bring up
this docker on every PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two TX-direction tests covering simple-someip's SD emit path: `tx_announcement_loop_emits_wire_format_offer` (no docker, CI-gated): drives Server::announcement_loop and captures the emitted bytes on a second multicast socket joined to the SD group. Asserts every field of the SOME/IP envelope (service/method/message-type/protocol+iface versions, return code), the SD flags (unicast set), the OfferService entry body (service+instance+major+minor+TTL>0), and the IPv4 endpoint option (interface, port, UDP). Catches silent regressions in the emit path without requiring vsomeip up. `vsomeip_sees_simple_someip_offer_service` (full cross-impl, optional): keeps the existing docker-based subscriber test for cross-impl validation when run on a second host. Module docs now record the same-host caveat we hit: vsomeip's routing-host architecture binds both endpoints to 0.0.0.0:30490 with SO_REUSEPORT, and same-host multicast delivery between two such instances is non-deterministic (reproduced with vsomeip-offerer → vsomeip-subscriber on the same box). The docker test should be run on a second host sharing the multicast-capable network. CI: new step in the `test` job flips MULTICAST on `lo` and runs the no-docker test with `--ignored --exact` so only that test runs (the docker-dependent ignored tests stay skipped). Both vsomeip JSON configs aligned to multicast 239.255.0.255 to match simple-someip's hardcoded MULTICAST_IP. The subscriber.cpp / subscriber.json / entrypoint.sh role dispatcher round out the docker image so both offerer and subscriber roles ship in one container. Follow-up: simple-someip's SD MULTICAST_IP is hardcoded to the Luminar-internal 239.255.0.255; making it configurable would let us run vsomeip with its spec-default 224.0.23.0 for stricter conformance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the four highest-severity items from the consolidated phase 18→20h punch list: CRIT-1: `tools/size_probe` excluded from `[workspace]` and given its own empty `[workspace]` table so `cargo clippy --workspace --all-features` no longer trips E0152 against the probe's `#[panic_handler]` / `#[global_allocator]`. Probe still builds via `cd tools/size_probe && cargo build --release --target …`. CRIT-2: Dropped the bogus `'pool` lifetime parameter on `EmbassyNetFactory` and the `mem::transmute<&SocketPool, &'static>` that was identity-only by accident. Factory now takes `&'static SocketPool` directly; `&'static SocketPool` coerces straight to `&'static dyn SlotReclaim`. Same observable behaviour on every existing caller, less unsafe. CRIT-3: Added `_not_thread_safe: PhantomData<*const ()>` to `EmbassyNetFactory` so the factory is `!Send + !Sync`. embassy-net's `Stack` uses interior `RefCell` for its socket-set bookkeeping and is not safe to drive `bind()` on from multiple threads; this pins the factory to a single executor task at the type level. HIGH-4: Documented at the call site why `RecvError::Truncated → Err(Io(Other))` is a deliberate adapter choice rather than the trait's `truncated: true` semantics — embassy-net 0.4 doesn't deliver any bytes on truncation and doesn't surface the original datagram length, so we can't honor the trait truthfully. Operator-side fix is to size `SocketPool` `RX_BUF` ≥ link MTU. HIGH-5/6: `bind()` now honors `addr.ip()` (passes a full `IpListenEndpoint` instead of just the port) and reads the actual ephemeral port back from `socket.endpoint()` post-bind, so `local_addr()` reports truth instead of the bind-time `:0`. HIGH-21 + new shared `LINK_MTU` const: the loopback driver and example client previously declared raw `1500` link MTUs that silently coincided with `simple-someip`'s `UDP_BUFFER_SIZE`. Hoisted a `pub const LINK_MTU: usize = 1500` into `simple-someip-embassy-net` itself (with docs explaining it's the *link-layer* cap, distinct from `UDP_BUFFER_SIZE`'s *application*-payload cap) and switched both consumers to import it. MED-22 (partial): `EmbassyNetBindFuture` now wraps `core::future::Ready` instead of an ad-hoc `Option::take` that bare-panicked on second poll; same semantics, stdlib panic message. MED-38: Rewrote the `endpoint_to_socket_addr_v4` rationale comment; the previous version conflated "non-exhaustive" with "no `unreachable_patterns` attribute". Verified: `cargo clippy --workspace --all-features` green; `cargo test -p simple-someip-embassy-net --tests` all 3 pass; `cargo build -p simple-someip --target thumbv7em-none-eabihf --no-default-features --features client,bare_metal` green; size_probe still builds standalone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HIGH-7 + MED-36: Tied `extern crate alloc` and the `Arc<T>: SharedHandle<T>` impl to a single internal feature `_alloc`, implied by `server`, `embassy_channels`, and `std`. The previous `cfg(any(feature = "embassy_channels", feature = "server"))` was right by accident — duplicated across two locations and silently omitted `std`-only flavours. The new gate makes the coupling explicit so a future `client-tokio`-style consumer that legitimately needs `Arc<T>: SharedHandle<T>` will get it without a fresh cfg-juggling exercise. HIGH-8: Re-exported `OfferedEndpoint` unconditionally. It was gated on `feature = "std"` while the trait method `PayloadWireFormat::for_each_offered_endpoint` that produces it is unconditional, so no-std `client`-only consumers couldn't name the type returned by a method they were expected to call. Pre-existing bug surfaced as fallout: `cargo test --no-default-features` was failing on `src/protocol/sd/test_support.rs` since phase 18d removed `std` from the `client`/`server` feature set. The trait method `new_subscription_sd_header` is unconditional; the `TestPayload` impl was `#[cfg(feature = "std")]`. Same for `set_reboot_flag`. Both now unconditional, with `std::net::Ipv4Addr` swapped for the `core::net::Ipv4Addr` re-export the trait already uses. Verified: 13-config build matrix green; `cargo clippy --workspace --all-features` and `cargo clippy --no-default-features` clean; `cargo test --no-default-features` now compiles and runs (4 doc tests pass). `client + bare_metal` rlib still has 0 alloc-symbol references on `thumbv7em-none-eabihf`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HIGH-9: Three event loops use `select_biased!` (futures-util's pseudo-random `select!` requires `std`, which we dropped from client/server in 18d) but their comments still claim `select!`-style fairness. Fixed by: - `client/socket_manager.rs`: 2-arm send/recv. Added `prefer_recv_first` flip flag that toggles arm priority each iteration so a sustained one-sided load can't starve the other arm. Approximates pseudo-random fairness without `std`. - `server/mod.rs::run_with_buffers`: 2-arm unicast/sd. Same flip pattern with `prefer_sd_first`. - `client/inner.rs::run_future`: 4-arm control/sleep/discovery/unicast. Documented the deliberate top-down priority — control drives loop lifecycle, the other three aren't at real risk of sustained starvation in practice — with a forward-compat note pointing at the flip pattern if that ever changes. HIGH-10: Two CI audit holes plugged in the alloc-symbol step: 1. `find target/... | head -1` was nondeterministic and could read stale artifacts from earlier matrix steps. Pinned to the exact `target/thumbv7em-none-eabihf/debug/libsimple_someip.rlib` path. 2. `rm -f libsimple_someip*.rlib` doesn't invalidate cargo's fingerprint cache, so the rebuild on the next line could no-op and leave the previous step's artifact in place. Replaced with `cargo clean -p simple-someip --target ...` which removes both the rlib and the fingerprint. 3. `nm 2>/dev/null` silently passed when the tool itself failed (missing binutils, malformed rlib). Dropped `2>/dev/null`, added `set -o pipefail`, kept the `|| true` only for the no-match case. Verified: 478/478 lib tests pass under client-tokio,server-tokio; all 13 build-matrix combos green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HIGH-11: subscriber.json's `clients[].unreliable` was 30509, matching offerer.json instead of the simple-someip Server's ADVERTISED_PORT (30500). subscriber.json is paired with the simple-someip Server, not with offerer.json — the two configs are independent. Fixed to 30500 with a comment pinning the relationship. HIGH-12: SocketOptions docs called out `SO_REUSEADDR` for the SD port without mentioning the Linux requirement that BOTH SO_REUSEADDR and SO_REUSEPORT be set on an SD socket sharing the multicast port — the test was already setting both, but the trait docs only documented one. Updated to make the requirement explicit on both fields. HIGH-13: TX wire-format conformance test rewritten to capture TWO consecutive announcements and assert: - Exact TTL (3 s default), not just `> 0`. - Session-ID monotonicity across announcements via `request_id`. - `RebootFlag::RecentlyRebooted` on the first announcement, flipping to `Continuous` on the second. - Exactly one SD entry, exactly one SD option, with the expected `(first_options, second_options) == (1, 0)` count. - IPv4 endpoint pin already covered, plus the dead `let _ = RebootFlag::RecentlyRebooted` import-pin (RebootFlag is now genuinely used). HIGH-14: RX-direction test now verifies vsomeip's OfferService carries an IPv4 endpoint option with `port=30509 UDP` — a parser regression that silently dropped options would have passed the old entry-only check. HIGH-17: Module docs referred to multicast group `224.0.23.0` (vsomeip spec default) while simple-someip and offerer.json both override to `239.255.0.255`. Updated the `ip route get` walkthrough and the failure-mode iptables hint to match the group simple-someip actually uses, and explicitly noted the non-spec-default in both places. MED-29: Added `required-features = ["client-tokio", "server-tokio"]` to vsomeip_sd_compat in Cargo.toml so `cargo test` cleanly skips it under the wrong feature set instead of silently reporting "0 tests" while every test inside refers to types that aren't in scope. Verified: `cargo build --features client-tokio,server-tokio --tests` passes; the conformance tests stay `#[ignore]`'d so CI behavior is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HIGH-15: `tools/size_probe`'s `someip_header_encode` validated
`MessageType` via `MessageType::try_from(byte & 0xBF)`, which
masked off bit 6 — a reserved bit pattern like `0x40` would
silently coerce to `Request` instead of erroring. Switched to
`MessageTypeField::try_from(byte)` which validates the raw byte
and only strips the TP flag (`0x20`) internally.
HIGH-16: Same function ignored the caller-supplied `length`
field and passed `payload_len = 0` to `Header::new`, producing
encoded headers with `length = 8` regardless of what the C ABI
caller asked for. Now derives `payload_len = length - 8` (the
SOME/IP `length` field covers the 8 fixed bytes after itself
plus the payload), with `checked_sub` for under-flow safety.
LOW (size_probe): `payload_len + 12` and `payload_len + 4` in
the E2E round-trip stubs would wrap on a 32-bit target with
sufficiently large input. Switched to `checked_add`. Also
renamed `PanicAllocator` to `NullAllocator` — it never panics,
returns null on alloc, and the docstring now explains the
runtime null-deref discipline rather than implying link-time
failures.
HIGH-18: Server's `run_with_buffers` doc example used
`&mut UNICAST_BUF` on `static mut` — hard error in Rust 2024
and unsound on any edition. Rewrote the example as a
`static UnsafeCell<[u8; …]>` with an `unsafe impl Sync` anchored
to the single-task-owner invariant.
HIGH-20: `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
drive emission without a full Server lifecycle. The remaining
`reboot_flag()` / `next_session_id()` accessors stay
`pub(super)` + `cfg(test)` because they're deliberately racy and
only safe when no other emitter is concurrent. Doc link
`[Self::reboot_flag]` (which referred to the cfg(test) accessor
and broke under the public docs build) rewritten to point at the
production `next_session_id_with_reboot_flag` instead.
Adjacent doc-link fixes surfaced by the partial-feature
rustdoc gate:
- `SubscriptionManager::get_subscribers` referred to
`Self::for_each_subscriber`; the method lives on the
`SubscriptionHandle` trait. Re-pointed and additionally moved
the cfg gate from `feature = "std"` to `feature = "_alloc"`
with `alloc::vec::Vec` so the method is reachable in
`embassy_channels` and pure-`server,bare_metal` builds where
alloc is already in scope.
- `Server::publisher` referred to a removed `EventPublisherHandle`
trait alias (collapsed into `SharedHandle` in 19f / 20e).
- `E2ERegistryHandle::register` referred to bare
`E2ERegistryFull` instead of `crate::e2e::E2ERegistryFull`.
- `tokio_transport`'s named-future docs intra-doc-linked
`futures::future::BoxFuture`, which doesn't resolve under
`--all-features` (the futures crate isn't a direct dep). Made
it a code literal.
MED-30: Server's `run_with_buffers` docstring claimed
`tracing::warn!` on backend truncation; the run loop never
inspects `ReceivedDatagram::truncated`. Rewrote to describe the
current (no-warn) behaviour and reference the bare-metal v3
backlog.
HIGH-19: `TokioSpawner::spawn` used to spawn TWO tokio tasks
per call (the work future + a JoinHandle watcher for panic
logging) — `UNICAST_SOCKETS_CAP` extra tasks per Client. Now
wraps the work future in `PanicLoggingFut`, which uses
`std::panic::catch_unwind` + `AssertUnwindSafe` to log panics
inline and resolve the future cleanly. One task per spawn.
Verified: `cargo doc --no-deps --features {client | server,bare_metal | --all-features}`
all clean (no broken intra-doc links); `cargo test --features
client-tokio,server-tokio --lib` 478/478 pass; size_probe still
builds for thumbv7em.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MED-22: `Server::new_with_handles` and `new_passive_with_handles` overwrote `config.local_port` unconditionally. Now back-fills only when the caller passed `local_port = 0` and returns `Error::InvalidUsage` when a non-zero `local_port` doesn't match the unicast socket's bound port — matches the back-fill-only-on-zero discipline of `new_with_deps`. MED-35: `EventPublisher`'s `_phantom: PhantomData<T>` re-imposed `T: Send + Sync` redundantly with `H`'s bounds. Switched to `PhantomData<fn() -> T>` (unconditionally `Send + Sync`) so a future `!Send T` behind a Send static-mutex handle doesn't force `EventPublisher: !Send`. MED-28: `client_send_request_server_runloop_stable` in the embassy-net loopback test was vacuous — it constructed a passive server then spawned `server.run()`, which returns `Err(InvalidUsage)` on the first poll. Removed the no-op spawn, rewrote the doc to honestly describe what the test verifies (the client's send path, not a server runloop) and noted the kept-name is for git-blame continuity with the parent reference test. MED-37: Added per-package pedantic clippy gates for the bare-metal feature subsets (`client+bare_metal`, `server+bare_metal`, `client+server+bare_metal`) under `simple-someip` alone. The existing `--workspace --all-features` pass was right by feature unification but masked feature-specific regressions and tied parent-crate lint health to embassy-net adapter dep storms. Per-package gates surface a regression against the responsible feature flag. MED-30 (cont.): Documented `# Panics` on `SdStateManager::next_session_id_with_reboot_flag` (lifted to `pub` in the previous commit). The closure's `.unwrap()` is statically infallible; doc'd as a tripwire. Adjacent fixes surfaced by the new pedantic gates: - `Server::run_with_buffers` was 104 lines after the select-arm-flip pattern duplicated the recv body twice. Factored the arms to return only `(datagram, from_unicast)`; the `(len, addr, source)` derivation lives once below the select. Now 82 lines, no `#[allow]` needed. - `examples/embassy_net_client`: 4 pedantic violations (uninlined-format-args ×3, default_trait_access ×1). Fixed format-args inline; the default_trait_access on `dns_servers` is intentional (embassy-net's private heapless re-export type isn't reachable to spell out), now `#[allow]`'d with a one-line justification. - `MED-47` (rolled in): `SubscriptionManager::get_subscribers` was gated on `feature = "std"` but only requires `alloc`. Switched to the new internal `_alloc` cfg + `alloc::vec::Vec` return type so the method is reachable in `embassy_channels` and pure-`server,bare_metal` builds. Verified: `cargo clippy --workspace --all-features -D warnings -D clippy::pedantic` clean; per-package pedantic gates clean for all three bare-metal combos; 478/478 lib tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LOW (CHANGELOG.md): added an `[Unreleased]` section documenting
every change in this cleanup branch (Added / Changed / Fixed),
filling the gap the consolidated punchlist flagged on the 0.8.0
release line.
vsomeip TX-conformance assertion correction: the new test was
asserting `RebootFlag::Continuous` on the second announcement
under the misreading that the flag flips per-emission. Per
AUTOSAR SOME/IP-SD (and `SdStateManager`'s implementation), the
flag stays `RecentlyRebooted` until the session counter wraps
from 0xFFFF → 0x0001 — i.e. ~65535 announcements after boot. The
unit tests inside `sd_state.rs` cover the wrap transition itself;
this integration test now correctly asserts the flag is unchanged
across two ticks.
Full verification matrix (all green):
- `cargo fmt --all --check`
- `cargo clippy --workspace --all-features -D warnings -D clippy::pedantic`
- `cargo clippy --no-default-features -D warnings -D clippy::pedantic`
- `cargo clippy -p simple-someip --no-default-features --features {client,bare_metal | server,bare_metal | client,server,bare_metal} -D warnings -D clippy::pedantic`
- `cargo build` thumbv7em-none-eabihf no_std target across {bare_metal alone | server,bare_metal | client,server,bare_metal | client,bare_metal} — `client+bare_metal` rlib has 0 alloc-symbol references
- `cargo test --no-default-features` — 4 doc tests
- `cargo test --features client-tokio,server-tokio --tests --test-threads=1` —
478 lib + 11 client_server + 1 bare_metal_e2e + 3 vsomeip_sd_compat (all
ignored) all green. Note: `client_server` integration tests share
the SD multicast port (30490) and unicast ports across tests, so
parallel execution flakes; CI uses cargo-nextest which serializes.
This is pre-existing behaviour on `main`, not introduced by this
branch.
- `cargo test -p simple-someip-embassy-net --tests` — 3/3
- `SIMPLE_SOMEIP_TEST_INTERFACE=127.0.0.1 cargo test --features
client-tokio,server-tokio --test vsomeip_sd_compat
tx_announcement_loop_emits_wire_format_offer -- --ignored` — pass
on a multicast-enabled `lo`
- `cargo doc --no-deps` partial-feature subsets {client | server,bare_metal | --all-features} — zero warnings
- `cargo build --release --target thumbv7em-none-eabihf` size_probe (standalone workspace)
Branch summary: phase 20 cleanup closes the 73-item punch list
spanning 3 critical / 18 high / 22 medium / 30 low items in 7
commits. See CHANGELOG `[Unreleased]` section for the full
changes-by-area breakdown.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Phase 20 cleanup pass across simple-someip, its tokio/embassy-net backends, conformance tests, CI gates, and the size_probe tooling—focused on correctness/soundness, feature gating (alloc), and more precise SD wire-format validation.
Changes:
- Tighten
allocavailability under a single internal_allocfeature and alignArc-based handle impls / alloc-returning APIs accordingly. - Improve SD conformance validation (vsomeip RX/TX tests) and event-loop fairness comments/behavior under
select_biased!. - Fix/clarify embassy-net adapter behavior (bind semantics, truncation handling docs) and reduce tokio spawner task overhead via panic-logging wrapper.
Reviewed changes
Copilot reviewed 22 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/size_probe/src/lib.rs | Fix SOME/IP header encoding validation and length handling; harden roundtrip size checks; rename stub allocator. |
| tools/size_probe/Cargo.toml | Make size_probe a standalone workspace root to avoid host-workspace lang item clashes. |
| tools/size_probe/Cargo.lock | Add dedicated lockfile for the standalone size_probe workspace. |
| tests/vsomeip_sd_compat.rs | Strengthen RX/TX wire-format assertions; update docs for multicast group; add endpoint-option checks. |
| tests/data/vsomeip-offerer/subscriber.json | Correct subscriber port to match simple-someip TX conformance test server port. |
| src/transport.rs | Update SocketOptions docs; gate Arc handle impls on _alloc; fix doc-link target for E2ERegistryFull. |
| src/tokio_transport.rs | Remove extra watcher task per spawn by wrapping spawned futures with panic-logging future. |
| src/server/subscription_manager.rs | Gate get_subscribers on _alloc and return alloc::vec::Vec to support alloc-without-std builds. |
| src/server/sd_state.rs | Make session/reboot atomic accessor public for external harnesses; document atomic pairing. |
| src/server/mod.rs | Validate local_port handling for handle-based constructors; update run-loop fairness logic and docs. |
| src/server/event_publisher.rs | Adjust phantom marker to avoid unnecessarily imposing T: Send + Sync on the publisher type. |
| src/protocol/sd/test_support.rs | Remove std dependency in test-support trait helpers by using core::net types. |
| src/lib.rs | Gate extern crate alloc behind _alloc; re-export OfferedEndpoint unconditionally. |
| src/client/socket_manager.rs | Flip select_biased! arm priority per-iteration to approximate fairness without select!/std. |
| src/client/inner.rs | Correct/clarify select_biased! fairness commentary and intentional priority order. |
| simple-someip-embassy-net/tests/loopback.rs | Use shared LINK_MTU constant; correct passive-server test semantics/docs. |
| simple-someip-embassy-net/src/socket.rs | Document truncation-contract mismatch and IPv6 feature-unification behavior more explicitly. |
| simple-someip-embassy-net/src/lib.rs | Export LINK_MTU constant and document intended buffer sizing semantics. |
| simple-someip-embassy-net/src/factory.rs | Remove lifetime/transmute patterns; honor bind IP + ephemeral port readback; add thread-safety marker/docs. |
| examples/embassy_net_client/src/main.rs | Use LINK_MTU; minor logging/formatting and clippy-related adjustments. |
| Cargo.toml | Exclude tools/size_probe; introduce internal _alloc feature and add required-features gating for vsomeip test. |
| Cargo.lock | Remove size_probe from workspace lock due to workspace exclusion. |
| CHANGELOG.md | Add [Unreleased] entry describing the phase-20 cleanup changes and fixes. |
| .gitignore | Ignore tools/size_probe/target build output directory. |
| .github/workflows/ci.yml | Add per-feature clippy gates; make alloc-symbol audit deterministic and fail loudly on tool errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage pass: cover every executable code path THIS branch
introduced. Rubric is "everything new in this branch is covered" —
not "boost overall %". Pre-existing untested branches in
client/socket_manager, client/inner, etc. are out of scope.
## New tests (+14 lib, +2 adapter, +2 doctests)
`src/server/sd_state.rs`: 5 mock-socket tests for
`SdStateManager::send_offer_service`. Previously only exercised by
`#[ignore]`'d multicast tests. New `CapturingSocket` /
`FailingSocket` `TransportSocket` impls capture send_to bytes
without touching a real network. Asserts full SOME/IP+SD envelope
shape, session-id advancement, wrap-flag transition (0xFFFE →
0xFFFF → 0x0001 with reboot flip), TTL=0 round-trip, and error
propagation when the socket fails. Lifts file from 43% → 55% line.
`src/server/mod.rs`: 7 tests for `new_with_handles` /
`new_passive_with_handles` (the MED-22 validation logic added in
this branch had zero coverage before). `build_test_handles` helper
constructs a real `ServerHandles` over `TokioTransport` with
ephemeral ports. Tests cover: back-fill on `local_port = 0`, accept
matching port, reject mismatch (both active and passive variants),
plus passive `run_with_buffers` / `announcement_loop`
short-circuits returning `InvalidUsage`. Lifts file from 80.6% →
84.4% line.
`src/tokio_transport.rs`: 2 tests for `PanicLoggingFut` (new in
this branch). Verifies (a) normal completion passes through the
catch_unwind Ok arm and (b) a panicking inner future is caught,
logged, and resolved cleanly without crashing the runtime — the
panic-Err arm. Both arms now have coverage counts.
`simple-someip-embassy-net/tests/loopback.rs`: 2 tests for the
new `EmbassyNetFactory::bind` paths.
`factory_bind_returns_address_in_use_when_pool_exhausted` covers
the pool-exhausted fallback. `factory_bind_accepts_wildcard_ip`
covers the `addr.ip().is_unspecified()` branch that translates
`0.0.0.0` → embassy-net's `addr: None` mode.
`simple-someip-embassy-net/src/factory.rs`: 2 `compile_fail`
doctests on `EmbassyNetFactory` that verify the type stays
`!Send + !Sync` at the type level. Locks in the
`PhantomData<*const ()>` marker against any future change that
would accidentally re-impose thread-safety. (Copilot incorrectly
flagged the marker as a no-op; the compile_fail doctests are now
authoritative.)
## Adjacent fixes
- **CI doc gate**: two `[Error::Io]` intra-doc links in
`src/server/mod.rs` referenced a removed enum variant; replaced
with `[Error::InvalidUsage]` to match the actual error type.
- **CHANGELOG line 29**: "RecentlyRebooted flipping to Continuous
on the second" was wrong — the flag stays RecentlyRebooted until
session-counter wrap (~65k announcements). Reworded to match
the actual test assertion + sd_state semantics.
- **`tests/vsomeip_sd_compat.rs:646`**: same wording bug in the
test body comment.
- **`tests/vsomeip_sd_compat.rs:775`**: "first run, none in the
second" was ambiguous (the test ALSO has first/second
announcements). Rewrote to clarify it's the first/second
*options-runs* of the SD spec.
- **`Cargo.toml:17`**: `cargo build -p size_probe` no longer
resolves now that the probe is excluded from the workspace.
Updated the build instruction to use the standalone manifest.
- **`StaticSocketHandle` doc-rot**: 4 references to the trait alias
collapsed into `SharedHandle<T>` in phase 19f / 20e
(`examples/embassy_net_client/src/main.rs:24`,
`src/server/mod.rs:141,224,373`). Rewrote to reference the
`&'static T` shape and the blanket impl.
## Verified
- `cargo fmt --all --check`
- `cargo clippy --workspace --all-features -D warnings -D clippy::pedantic`
- `cargo clippy --no-default-features -D warnings -D clippy::pedantic`
- `cargo clippy -p simple-someip --no-default-features --features {client,bare_metal | server,bare_metal | client,server,bare_metal} -D warnings -D clippy::pedantic`
- `RUSTDOCFLAGS=-Dwarnings cargo doc --no-deps --no-default-features --features client` (the gate that blocked CI)
- `RUSTDOCFLAGS=-Dwarnings cargo doc --no-deps --no-default-features --features server,bare_metal`
- `RUSTDOCFLAGS=-Dwarnings cargo doc --no-deps --all-features`
- `cargo test --no-default-features` 4 doc tests pass
- `cargo test --features client-tokio,server-tokio --tests --test-threads=1` —
492 lib (478 + 14 new) + 11 client_server + 1 bare_metal_e2e + 3
ignored vsomeip — all green
- `cargo test -p simple-someip-embassy-net --tests` 5/5 (was 3, +2 new)
- `cargo test -p simple-someip-embassy-net --doc` 2/2 (the new
compile_fail Send/Sync assertions)
Total workspace coverage: 90.32% line / 93.57% function — line
coverage up from 89.12% baseline.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JustinKovacich
changed the base branch from
feature/phase20h_bidirectional_sd_conformance
to
feature/phase17_cleanup
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 50 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+33
| # Pinned to a version known to coexist with `simple-someip`'s | ||
| # `embassy-sync = "0.6"` dep. embassy-net 0.4.x is the last | ||
| # release line that builds against embassy-sync 0.6; later | ||
| # embassy-net releases (0.5+) require embassy-sync 0.7+, which | ||
| # would force a parallel-version cargo resolution that bloats the | ||
| # binary. Bumping both deps in lockstep is its own future phase. |
There was a problem hiding this comment.
The comment about pinning embassy-net = "0.4" to avoid an embassy-sync version fork doesn’t match the actual resolved dependency graph in this PR: the workspace Cargo.lock shows embassy-net 0.4.0 depending on embassy-sync 0.5.0, while this crate depends on embassy-sync 0.6. This means a parallel-version resolution already happens, so either update the comment to reflect reality (and why it’s acceptable), or adjust the dependency strategy to truly unify on a single embassy-sync version.
Suggested change
| # Pinned to a version known to coexist with `simple-someip`'s | |
| # `embassy-sync = "0.6"` dep. embassy-net 0.4.x is the last | |
| # release line that builds against embassy-sync 0.6; later | |
| # embassy-net releases (0.5+) require embassy-sync 0.7+, which | |
| # would force a parallel-version cargo resolution that bloats the | |
| # binary. Bumping both deps in lockstep is its own future phase. | |
| # Pinned to embassy-net 0.4.x for now. Note that this does *not* | |
| # unify the `embassy-sync` version across the resolved graph: | |
| # embassy-net 0.4 pulls in embassy-sync 0.5.x, while this crate | |
| # depends on embassy-sync 0.6 via `simple-someip` integration. | |
| # We keep 0.4.x anyway because newer embassy-net releases (0.5+) | |
| # move on to embassy-sync 0.7+, which would widen the split even | |
| # further unless the whole embassy stack is upgraded together. | |
| # Unifying on a single embassy-sync version is a separate future | |
| # phase that requires coordinated dependency bumps. |
Contributor
Author
There was a problem hiding this comment.
Verified — Copilot's correct, the original comment was a misread of the lockfile state. Cargo.lock confirms embassy-net 0.4.0 depends on embassy-sync 0.5.0, while this crate (and simple-someip) declare embassy-sync 0.6, so the parallel-version split is already there.
Fixed in 9b4b4c8. The comment now reflects reality:
embassy-net 0.4 itself depends on
embassy-sync 0.5.x, whilesimple-someip(and this crate) useembassy-sync 0.6. So the resolved Cargo.lock already carries both versions in parallel, which costs some binary size on firmware targets. We accept that today because the alternative is worse: newer embassy-net releases (0.5+) move on to embassy-sync 0.7+, widening the split further unless the whole embassy stack is upgraded together.
Unifying on a single embassy-sync version stays a follow-up phase requiring coordinated dep bumps across embassy-{sync,net,executor,time}.
Audit of the 16 tests added in `a6e13d4` surfaced two genuinely
defective tests + several weak assertions. All fixed.
## HIGH severity
`panic_logging_fut_catches_panic_and_resolves_cleanly` and
`panic_logging_fut_passes_through_normal_completion` were
spawner-integration tests that would have passed even if
`PanicLoggingFut` did nothing — tokio's default behaviour in
`current_thread` mode already absorbs task panics, and a healthy
async block runs whether or not the wrapper is in the path. False
confidence.
Rewrote both as direct unit tests against `PanicLoggingFut::poll`
itself, polling the wrapper with a no-op `Waker`:
- normal-completion test: wraps a future that bumps an
`AtomicUsize` poll-counter, asserts `Ready(())` AND that the
inner future was polled exactly once. A regression that
bypassed `inner.poll` would fail the counter check.
- catches-panic test: wraps a future that panics on first poll,
manually polls the wrapper, asserts `Ready(())`. A regression
that removed `catch_unwind` would unwind out of `poll` and
abort the test (failing it).
Added a third test, `tokio_spawner_isolates_panicking_tasks_from_runtime`,
which stays at the spawner-integration layer but bounds itself
with `tokio::time::timeout(1s, ...)` and verifies the
behavioural difference end-to-end: a panicking spawned task must
not prevent a subsequently-spawned healthy task from running.
With `PanicLoggingFut` the runtime stays alive and the healthy
task completes; without it, tokio's default already handles the
panic, so this test is a smoke test rather than a strong gate —
but combined with the unit tests above it covers both layers.
## MEDIUM severity
`send_offer_service_propagates_socket_errors` used `Err(_)` which
matched any error including unrelated regressions. Narrowed to
`Err(Error::Transport(TransportError::Io(IoErrorKind::NetworkUnreachable)))`
so it specifically asserts the propagated error from
`FailingSocket::send_to` rather than catching any random failure
mode.
`new_with_handles_back_fills_local_port_on_zero`: added an
`assert_ne!(bound_port, 0, ...)` precondition so the test can't
silently pass on a degenerate kernel-allocated port of 0.
`new_with_handles_rejects_local_port_mismatch` and the passive
counterpart used a clever-but-confusing `if bound_port == 1 { 2 }
else { 1 }` to pick a "bogus port" that wouldn't match. Replaced
with `bound_port.wrapping_add(1)` — deterministic, no privileged-
port mythology, and `assert_ne!` makes the distinctness explicit.
## LOW severity
Renamed the five `*_via_mock` sd_state tests to
`send_offer_service_through_mock_*` so the prefix groups them as
a unit. Added a section comment explaining why both the mock
tests AND the `#[ignore]`'d multicast tests below remain: the
mocks cover the encoding/framing path; the multicast tests
exercise the kernel-multicast `socket.send_to` that the mocks
can't observe. Don't delete one in favour of the other.
## Mislabel correction
The `passive_server_run_with_buffers_returns_invalid_usage` and
`passive_server_announcement_loop_returns_invalid_usage` tests
were claimed in `a6e13d4`'s commit message as covering "new code
this branch added." They don't — the `is_passive` short-circuits
in those methods predate the cleanup. They're still valuable
(they pin specific `InvalidUsage` tag strings that the
`new_with_handles` constructor relies on), so they remain. This
commit message corrects the scope claim.
## Verified
- `cargo fmt --all --check`
- `cargo clippy --workspace --all-features -D warnings -D clippy::pedantic`
- `cargo clippy --no-default-features -D warnings -D clippy::pedantic`
- `RUSTDOCFLAGS=-Dwarnings cargo doc --no-deps --no-default-features --features client`
- `cargo test --features client-tokio,server-tokio --tests --test-threads=1` —
493 lib (was 492, +1 from `tokio_spawner_isolates`) + 11 + 1 + 3 ignored
- 15 impacted tests rerun individually, all green
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The comment on `embassy-net = "0.4"` claimed the pin avoided a
parallel-version cargo resolution by keeping embassy-sync at 0.6.
That's wrong: embassy-net 0.4.0 itself depends on embassy-sync
0.5.x, so the resolved `Cargo.lock` already carries both 0.5.0
and 0.6.2 in parallel.
Updated the comment to reflect what's actually happening: the
parallel-version split exists today, we accept the binary-size
cost because newer embassy-net releases would widen it further,
and unifying on a single embassy-sync version is a future
coordinated-bump phase across embassy-{sync,net,executor,time}.
Caught by Copilot review on PR #113.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the consolidated phase 18→20h punch list — 3 critical / 18 high
/ 22 medium / ~30 low items pulled from the adversarial line-by-line
review (2026-04-29) plus Copilot reviews on PRs #97–#111. Each commit
is self-contained and tagged by severity for commit-by-commit review.
Commit map
5ad28ee— workspace clippy E0152 + embassy-net adapter soundness (CRIT-1/2/3, HIGH-4/5/6/21, MED-27/38)878122e—_alloccfg gate (HIGH-7),OfferedEndpointvisibility (HIGH-8)416b989—select_biased!arm-flip fairness (HIGH-9), CI alloc-symbol audit holes (HIGH-10)573346f— vsomeip conformance hardening (HIGH-11/12/13/14/17, MED-29)c5885ba— size_probe correctness (HIGH-15/16), sd_state half-public + doc-link rot (HIGH-20),static mutdoc-example UB (HIGH-18), tokio Spawner doubled tasks (HIGH-19), MED-30/47, intra-doc fixes62dfac3— server constructor validation (MED-22 cluster), Phantom marker (MED-35), per-package pedantic clippy gates (MED-37), embassy-net loopback test honesty pass (MED-28)7b0aa61— CHANGELOG[Unreleased]+ final verification + corrected vsomeip TX assertionLoad-bearing fixes
'poollifetime + an identity-onlytransmute<&SocketPool, &'static>;marked
EmbassyNetFactory: !Send + !SyncviaPhantomData<*const ()>because embassy-net's
StackinteriorRefCellisn't safe to drivebind()on from multiple threads.extern crate allocand theArc<T>: SharedHandle<T>impl to a single internal_allocfeatureimplied by
server/embassy_channels/std. Previouscfg(any(feature = "embassy_channels", feature = "server"))was rightby accident and silently omitted
std-only flavours.select_biased!fairness): three event-loop sites usedselect_biased!(select!needsstd, dropped in 18d) but commentsstill claimed
select!-style fairness. Server + socket_manager 2-armselects now flip arm priority each iteration to approximate that
fairness without pulling
std.announcements and asserts exact TTL (3 s default), session-ID
monotonicity, reboot-flag behaviour, and
(entries, options) == (1, 1).RX test now verifies vsomeip's OfferService carries the expected
IPv4 endpoint option — a parser regression dropping options would
have passed the old entry-only check.
TokioSpawner::spawnusedto spawn TWO tokio tasks per call (the work future + a
JoinHandlewatcher for panic logging) — that's
UNICAST_SOCKETS_CAPextra tasksper Client. Folded into
PanicLoggingFutviastd::panic::catch_unwind. One task per spawn.Test plan
cargo fmt --all --checkcargo clippy --workspace --all-features -D warnings -D clippy::pedanticcargo clippy --no-default-features -D warnings -D clippy::pedanticcargo clippy -p simple-someip --no-default-features --features {client,bare_metal | server,bare_metal | client,server,bare_metal} -D warnings -D clippy::pedantic(new gates added in this PR)client+bare_metalrlib has 0 alloc-symbol referencescargo test --no-default-features(4 doc tests; previously failed due totest_supportcfg-mismatch with the trait surface — fixed in878122e)cargo test --features client-tokio,server-tokio --tests --test-threads=1— 478 lib + 11 client_server + 1 bare_metal_e2e + 3 ignored vsomeipcargo test -p simple-someip-embassy-net --tests3/3SIMPLE_SOMEIP_TEST_INTERFACE=127.0.0.1 cargo test … tx_announcement_loop_emits_wire_format_offer -- --ignoredpasses on a multicast-enabledlocargo doc --no-depspartial-feature subsets {client | server,bare_metal | --all-features} — zero warningscargo build --release --target thumbv7em-none-eabihffor size_probe (now its own standalone workspace)Known caveats
tests/client_server.rsflakiness is pre-existing onmain(verified by checking outorigin/mainand running the sametest set). Tests share SD multicast port 30490 and unicast ports
across tests; CI uses
cargo llvm-cov nextestwhich serializes bydefault, so this is dormant in CI. Not introduced by this branch;
fix would be ephemeral ports per test as a follow-up.
new_with_handles/SocketPool::claim/release/Drop" — these areexercised indirectly via the embassy-net loopback integration suite.
Adding standalone unit tests is bare-metal plan v3 phase 21+ work.
bare-metal triad on
simple-someip; the embassy-net-adapter featurecombos still funnel through
--workspace --all-features.Source reviews
gh api repos/luminartech/simple_someip/pulls/N/comments)flagged independently by both review streams.
Coverage note
Workspace coverage on this branch is 90.32% line / 93.57% function.
For comparison,
origin/main(v0.7.0) is at 95.41% line / 98.85% function.The branch's percentages are lower but the comparison is misleading:
origin/main(v0.7.0)The branch added ~5,000 net new lines across phases 17 → 20h
(bare-metal trait surface, embassy-net adapter, vsomeip conformance
scaffolding, no-alloc primitives,
SharedHandle<T>consolidation,TC4D-flavor
size_probe). Of those new lines, ~81.5% are coveredon their own. The aggregate dropped because the new bare-metal /
no_std-only paths are below main's 95% baseline by construction —
many are reachable only from
&'static-handle / no-alloc consumersthat the host test harness can't fully exercise.
This branch's coverage commit (
a6e13d4) covers every executablecode path this branch added (rubric: "everything new in this
branch is tested"). Pre-existing untested no_std paths in
client/socket_manager,client/inner, etc. are intentionallyout of scope — addressing them is a multi-day fault-injection
harness job, not a cleanup-PR concern.