Port #126 (polled bare-metal) onto the #125 stack — handoff#134
Closed
JustinKovacich wants to merge 200 commits into
Closed
Port #126 (polled bare-metal) onto the #125 stack — handoff#134JustinKovacich wants to merge 200 commits into
JustinKovacich wants to merge 200 commits into
Conversation
Covers the send path end-to-end (SOME/IP envelope, SD flags, OfferService entry fields, and the IPv4 endpoint option) plus session-id advancement and wrap-through-zero exercised via send_offer_service itself, and a smoke test for Server::start_announcing. All `#[ignore]`d pending the loopback MULTICAST-flag fix on this branch; without that fix, hosts drop the multicast packet silently and the tests time out on recv. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Rustfmt on stable wraps the let-else continue blocks and the assert_offer_matches signature differently than I hand-wrote. Let cargo fmt normalize the style. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Per AUTOSAR SOME/IP-SD, the reboot bit on emitted SD messages must flip from RecentlyRebooted to Continuous once the session counter wraps past 0xFFFF. SdStateManager already owns the counter, so it also tracks wrap (new has_wrapped: AtomicBool latched exactly on the 0xFFFF -> 0x0001 transition) and exposes reboot_flag() as the single source of truth. The four SD emission paths — SdStateManager::send_offer_service, Server::send_unicast_offer, send_subscribe_ack_from_view, and send_subscribe_nack_from_view — all now consume the tracked flag instead of hardcoding Flags::new(true, true). Coverage: four new non-ignored unit tests cover the state machine (fresh, sub-wrap, exactly-on-wrap, monotonic-after-wrap); assert_offer_matches takes an expected RebootFlag; the existing wrap multicast test now asserts first-emit=RecentlyRebooted and second-emit=Continuous across the boundary. Responds to PR #75 feedback on src/server/sd_state.rs:90. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Round-2 Copilot review caught a real correctness bug in the prior reboot-flag refactor: the four SD emission paths read `reboot_flag()` BEFORE advancing the session counter, so the message whose session_id crosses 0xFFFF -> 0x0001 (where `has_wrapped` actually latches) still advertises `RebootFlag::RecentlyRebooted`. The flip only lands on the NEXT emission — violating AUTOSAR SOME/IP-SD semantics that say the wrap message itself should carry `Continuous`. Reordered in all four sites: call `next_session_id()` first so `has_wrapped` latches, then read `reboot_flag()` for this specific message. Sites: - `SdStateManager::send_offer_service` (sd_state.rs) - `Server::send_unicast_offer` (mod.rs) - `Server::send_subscribe_ack_from_view` (mod.rs) - `Server::send_subscribe_nack_from_view` (mod.rs) Added short comments at each site pointing at the canonical ordering note on `send_offer_service`. Also reworded the multicast-loopback `#[ignore]` comment block and per-test message to remove the stale branch-name reference (`feature/firmware_someip_conversion`) — the underlying dependency is the `lo` MULTICAST flag, not a branch-specific fix. New wording says "skipped on hosts whose `lo` lacks the MULTICAST flag" with the `ip link show lo` diagnostic pointer. Coverage: the existing ignore-gated wrap test `send_offer_service_wraps_session_id_through_zero_on_send` already asserts the pre-wrap/post-wrap flag transition on-the-wire; with the ordering fix it now passes in environments that run ignored tests (would have FAILED before this commit — which is why the bug slipped past the first round). The non-ignored state-machine tests (`reboot_flag_flips_to_continuous_exactly_on_wrap` et al.) are unaffected and still green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot round-3 flagged the `#[ignore]` reason on
start_announcing_emits_first_offer_within_timeout for still
carrying a branch-specific phrase ("re-enable after lo fix on
this branch"), which becomes stale once merged. Replaced with
a durable prerequisite description:
requires loopback multicast support (MULTICAST on lo)
Matches the companion rewording in a638a4b on the sd_state.rs
multicast-loopback harness comment block.
Addresses Copilot comment 3132878961.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eatures behind gates
- inner.rs: on request_queue overflow, reject the ControlMessage by
sending Err(Error::Capacity("request_queue")) on every oneshot it
carries (via new ControlMessage::reject_with_capacity helper) rather
than silently dropping it — the drop previously cancelled the oneshot
and panicked callers awaiting with .unwrap().
- inner.rs: on pending_responses.insert saturation, destructure the
returned (request_id, response) and send
Err(Error::Capacity("pending_responses")) on the response sender;
previously the sender was dropped, panicking PendingResponse::response.
- mod.rs: update the send_to_service saturation doc to match the new
explicit-error behavior.
- mod.rs: fix the module-header footprint doc — SESSION_CAP lives in
session.rs, not inner.rs.
- session.rs: gate the saturation warn! behind a one-shot saturation_warned
latch; previously the warning fired on every check() against every new
key once the map was full, which meant log-spam at the packet rate.
- error.rs: drop #[non_exhaustive] on client::Error; downstream crates
relying on exhaustive matches shouldn't silently break before a planned
breaking release.
New tests: reject_with_capacity_notifies_every_sender covers every
ControlMessage variant (including SendToService's dual senders);
capacity_overflow_warns_only_on_first_hit locks in the saturation latch.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Commit cb1d0d1 added explicit capacity-error delivery when pending_responses.insert overflows — without the explicit Err send, the dropped Sender would cause PendingResponse::response().await to panic on RecvError instead of surfacing a clean Result. The recovery logic was inline in the SendToService run-loop arm, which made it hard to exercise without driving 64 live sockets. Lift it to a small `track_or_reject_pending_response` helper on Inner so the SendToService arm delegates, and the branch is testable against the exposed `pending_responses` map directly. Added two tests: - `track_or_reject_pending_response_inserts_when_room_available`: happy path — entry lands in the map, the sender is still live (receiver stays pending). - `track_or_reject_pending_response_rejects_on_saturation`: fill map to PENDING_RESPONSES_CAP, invoke the helper with one more request, assert the map is unchanged and the caller's receiver resolves to Err(Error::Capacity("pending_responses")) — which is the invariant cb1d0d1 guarantees. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Round-2 Copilot feedback on PR #76 caught 5 resolvable issues (6th rejected — see below): - src/client/inner.rs: the three `push_front(...).ok()` re-enqueue sites (SetInterface x2, SendSD, Subscribe) silently dropped the returned Err. Swapped each to `if let Err(rejected) = ... { ... rejected.reject_with_capacity("request_queue"); }`, matching the primary `push_back` overflow arm. These branches are defensive — by construction the slot we just popped is free — but a future refactor that changes queue usage would otherwise reintroduce the silent-drop-of-oneshot-senders regression cb1d0d1 was specifically written to prevent. - src/server/subscription_manager.rs: `list.push(...).ok()` on a freshly-allocated `SubscribersList` (first subscriber in a new event group) swapped to `.expect(...)` with a message naming the `SUBSCRIBERS_PER_GROUP >= 1` invariant. Tripwires a future cap-0 regression at test time instead of silently losing the only subscriber. - src/client/session.rs: reword the `SessionTracker` doc comment's reference to non-existent "module docs" — point directly at the `SESSION_CAP` constant instead. Rejected: src/client/error.rs `Capacity` variant as a breaking exhaustive-match break. This is consistent with the earlier decision recorded on #75/#80 to accept the breaking change rather than carry deprecated shims or wrap everything in `Error::Io` — the current release is a breaking version bump by design, and hiding the variant in `Io` would lose the lowercase-snake_case tag semantics the new error was specifically designed to carry. No production behavior changes — just defensive tripwires, docs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address two clippy::pedantic warnings introduced by this branch's own commits: - missing_panics_doc on SubscriptionManager::subscribe — the heapless::Vec::push.expect on the first-insert path can only trip if SUBSCRIBERS_PER_GROUP is set to zero; document that. - cast_possible_truncation on 'i as u32' in the saturation test for pending_responses — use u32::try_from with an expect that documents why the 64-cap fits.
Copilot round-3 on PR #76 (comment 3138669839) caught a real gap in `track_or_reject_pending_response`: the helper only handled `Err((key, value))` (map-full + new-key), but `heapless::IndexMap:: insert` on an existing key returns `Ok(Some(old_sender))` and the old sender was silently dropped. If `request_id` is ever reused while an older pending entry is still live — the documented example is `session_counter` wrap-around — the caller awaiting the original request would see a `RecvError` on channel cancellation, which `PendingResponse::response()` turns into a panic. Reworked the `if let Err(...)` into a full `match` with all three arms: - `Ok(None)` — normal insert, nothing to do. - `Ok(Some(displaced))` — `warn!` that the slot was replaced, complete the displaced sender with `Err(Error::Capacity("pending_responses"))` so the original caller gets a clean `Result`, not a `RecvError` panic. - `Err((_req_id, r))` — existing saturation path (unchanged). Also updated the doc comment on `track_or_reject_pending_response` to describe the displacement contract. Coverage: new test `track_or_reject_pending_response_completes_displaced_sender` explicitly inserts the same key twice and asserts the first receiver resolves to `Err(Error::Capacity("pending_responses"))` before the second sender still sits pending in the map. Addresses Copilot comment 3138669839. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The old docstring explained why client::Error is not #[non_exhaustive] but didn't acknowledge that adding variants to a non-#[non_exhaustive] enum is itself a breaking change for downstream exhaustive matches. Rewrite the note to call that out explicitly and flag future #[non_exhaustive] as a planned breaking-release change, and record the new Capacity variant under the Unreleased Added section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SubscriptionManager::subscribe previously returned () and logged a warn! on capacity failure, which let handle_sd_message send SubscribeAck to a client whose subscription had silently been dropped — the client would believe it was subscribed but never receive any events. - subscribe now returns Result<(), SubscribeError> with SubscribersPerGroupFull / EventGroupsFull variants; existing capacity/refresh semantics are unchanged. - handle_sd_message inspects the result and emits SubscribeNack with a reason derived from the SubscribeError variant on rejection. - EventPublisher::register_subscriber surfaces the same Result so external SD dispatchers can take the same corrective action. - SubscribeError is re-exported from server::mod. - Tests updated to consume the Result; subscription_manager overflow tests now assert the specific error variant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ording - session::check: expand the doc comment with an explicit "Capacity behavior" section — the saturation path does not update stored state for new keys, which contradicts the old "Always updates the stored state" wording. Describe the actual behavior (normal path updates, new-key inserts under saturation are silently dropped and return Initial repeatedly) so callers don't rely on a strict guarantee the implementation doesn't hold. - session::check saturation warn!: include sender + transport in the log line so diagnosing which peer lost reboot-detection state no longer requires out-of-band correlation. - SubscriptionManager::subscribe dedup branch: rename the log from "Refreshed existing subscriber" to "already subscribed; skipping" and add a comment making it explicit that no per-subscriber state is modified. The old wording implied a refresh (TTL bump, etc) the code never performed. - inner.rs test comment: tokio::sync::oneshot doesn't drop the sender when the receiver is dropped — it flips send() to a failure mode. Rewrite the comment to describe the actual stash purpose (keep channels open for later observation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- server::run NACK arm: stop allocating a String with format!() held across the SubscribeNack await; log the SubscribeError at warn! separately and pass a static "subscription rejected" reason string to send_subscribe_nack_from_view. Cleaner and avoids the borrow- across-await style issue Copilot flagged. - EventPublisher::register_subscriber: add # Errors section describing the two SubscribeError variants, that the subscriber is NOT registered on Err, and that external dispatchers should NACK on Err just like the server's own run() loop does. - CHANGELOG: add server::SubscribeError to Added, and list the breaking signature changes on SubscriptionManager::subscribe and EventPublisher::register_subscriber under Changed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ubscribe docs - CHANGELOG.md: drop the two "Changed" bullets about `std` becoming the default feature and `thiserror`/`tracing` moving to `default-features = false`. Those landed in 0.6.0/0.6.1 (commit f161980, already on main) and are not changes made in this PR. - server/mod.rs: when a SubscribeError rejects a subscription, match on the variant and pass a specific `&'static str` reason to `send_subscribe_nack_from_view` (`"subscribers_per_group_full"` / `"event_groups_full"`) instead of the generic `"subscription rejected"`. The NACK log line now reflects the real cause, and the static-str choice avoids any String-across-await allocation. - server/subscription_manager.rs: rewrite the `subscribe` docs to say the duplicate path is idempotent / deduplicated rather than implying TTL-refresh semantics that don't exist today. Flag the future-TTL extension point explicitly so the doc and the log wording stay in sync if that behavior is added later. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…urns an error, added unit tests
Commit 343da67 added a `required_size() > UDP_BUFFER_SIZE` pre-check to `EventPublisher::publish_event` but left the new branch uncovered. Regression guard added as `publish_event_pre_encode_exceeds_udp_buffer_returns_capacity_error`: registers a subscriber (the pre-check sits after the `subscribers.is_empty()` early return, so the test needs one or else hits the false-positive Ok(0) path), constructs a 1501-byte fixture (16-byte header + 1485-byte payload, one over the cap), calls publish_event, asserts Err(Error::Capacity("udp_buffer")). Mirrors the fixture pattern from `send_raw_message_exceeding_udp_buffer_returns_capacity_error` on the client side. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- src/lib.rs: UDP_BUFFER_SIZE doc now enumerates exactly which send paths honor this cap (SocketManager::send, publish_event, publish_raw_event) and explicitly calls out the SD announcement / SubscribeAck / SubscribeNack paths that still use heap Vec buffers as a known gap planned for the bare-metal no_alloc refactor. - src/server/event_publisher.rs: reworded "stack buffer sized to MTU" comments at the two buffer-allocation sites — the buffer lives in the async future's state, not literally on the stack, and the cap is a UDP payload limit, not an Ethernet MTU. New wording points at the UDP_BUFFER_SIZE docs for the distinction. The two `use std::vec` comments (event_publisher.rs:335, socket_manager.rs:362) were verified to be false positives: removing the imports breaks the lib-test build with 4 errors about `vec!` macro not in scope. Same no_std mechanics as the prior #75-1 resolution — reply posted on the comment threads. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three doc-only Copilot round-3 nits on PR #77: - src/client/mod.rs: the per-`SocketManager` memory-footprint blurb implied the second `[u8; UDP_BUFFER_SIZE]` is always allocated. In fact `socket_loop_future` only allocates that scratch buffer when the send path actually needs E2E protection (the destination key is in the `E2ERegistry`); plain sends pay only ~1.5 KiB. Reworded the always-live vs peak budget so the ~24 KiB number is no longer presented as the steady-state cost. - src/client/socket_manager.rs: the E2E-overflow test comment said "8 over MTU", but `UDP_BUFFER_SIZE` is documented as a UDP payload cap, not an Ethernet-MTU-safe size. Reworded to "8 bytes over UDP_BUFFER_SIZE". - src/lib.rs: the `UDP_BUFFER_SIZE` doc referenced bare `Error::Capacity("udp_buffer")`, which is ambiguous at the crate root (no `crate::Error` exists). Qualified to `client::Error::Capacity(...)` / `server::Error::Capacity(...)`. Addresses Copilot comments 3138697909, 3138698042, 3138698156. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…event
Adds a unit test that registers a Profile4 E2E profile for the message
key and publishes a message whose raw encoded size fits UDP_BUFFER_SIZE
(1496 bytes) but whose protected size does not (1508 bytes, after the
12-byte Profile4 header). Asserts that publish_event returns
Error::Capacity("udp_buffer") — exercising the post-protect guard that
was previously only covered on the client send path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- publish_event: log now says "E2E-protected datagram (... header + protected payload)" so the 16+protected_len value is identified as the full SOME/IP datagram size, not the payload. - test fixture comment: "8 over MTU" → "8 bytes over UDP_BUFFER_SIZE" for terminology consistency with the rest of the PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_len
`header_len + payload.len()` used unchecked `usize` addition. On a
system with large enough `payload.len()` the sum can wrap, silently
bypassing the `> UDP_BUFFER_SIZE` guard and corrupting the slice
operations that follow. Switch to `checked_add` and treat overflow the
same as exceeding `UDP_BUFFER_SIZE` — return
`Error::Capacity("udp_buffer")`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Client/server "E2E-protected payload" logs now say "E2E-protected datagram (header + protected payload)" since the logged value is the full SOME/IP datagram size, not the payload. - publish_raw_event checked_add-fail log now describes the real condition (usize overflow) and includes the input lengths, instead of falsely pointing at UDP_BUFFER_SIZE. - Oversize fixtures in socket_manager + event_publisher tests are now sized from UDP_BUFFER_SIZE (and PROFILE4_HEADER_SIZE implicitly for the E2E case) instead of hardcoded 1480/1485. Fixtures stay valid if the cap is retuned. - client/mod.rs memory-footprint doc no longer hardcodes "(1500 bytes)" or "~1.5 KiB" as load-bearing numbers; the scaling is now expressed in terms of UDP_BUFFER_SIZE with the current-default numbers as a parenthetical reference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The module-level "# `Send` and multithreaded executors" section showed a HRTB bound on `<T as TransportSocketSendFut<'a>>::Fut: Send` as the way consumers should bind `Send`. No such trait exists in this crate — with RPITIT the returned future type is anonymous and cannot be named, and introducing a GAT-style escape hatch would pollute the trait for the common single-threaded case. Replaced with the reviewer-preferred pattern: wrap the call in an `async move` block and require `T: Send + 'static` on the captured state. A tokio-backed implementation whose underlying `UdpSocket` is already `Send + Sync` produces `Send` futures automatically via async-block capture inference, so no trait-level bound is required. Implementations holding `!Send` state fail the `T: Send` bound at the `tokio::spawn` call site, which is the actionable location. Docs-only change; `cargo test --doc` passes on the new ignore-fenced example. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ers; run future 7696->3528 B (#125 PR3 T2+T3) send_offer_service + the 3 SD helpers now write into caller-provided scratch threaded via run_with_buffers (two distinct &mut [u8], borrowck-guaranteed non-aliasing across the concurrent recv/announce loops). tokio run_inner allocates them internally so Server::run is unchanged; bare-metal callers pass their own. Dead post-encode checks -> debug_assert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…uard on buf.len() (#125 PR3 T4) publish_event_with_buffers / publish_raw_event_with_buffers take caller scratch instead of future-resident [u8; UDP_BUFFER_SIZE] arrays; the E2E overflow guard now checks msg_buf.len() (was UDP_BUFFER_SIZE) — the same OOB-panic bug class PR 2 fixed on the client, now on the server publish path (regression test RED->GREEN). Tokio publish_event/publish_raw_event kept as ergonomic _alloc wrappers so the app API is unchanged. bm_server_publish_future ~3320 -> 320 B. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r extraction; fix comment typo (#125 PR3 T5) bm_server_run_future is 3528 B post-extraction; budget = ceil64(3528*1.25)=4416 (was 9664 for the pre-PR3 7696). Also fixes a 1400->1500 UDP_BUFFER_SIZE comment typo flagged in the Task 4 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task 4's new doc comments wrote unresolvable intra-doc links surfaced by the
feature-subset cargo doc gate: [`Error::Capacity("udp_buffer")`] (the paren
isn't a valid path) -> [`Error::Capacity`]`("udp_buffer")` matching the
runtime/sd_state form; [`EventPublisher::publish_event`] and [`Vec`] -> code
spans. Doc-comment-only; cargo doc client / server,bare_metal / all-features
all clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y + docs/CHANGELOG (#125 PR3 T6) Whole-branch review fixes: - announce_only_with_buffer (PUBLIC) takes caller scratch and threads it to announce_loop, so the supplementary-server announce future is no longer buffer-resident for bare-metal callers (the prior doc pointed at pub(super) announce_loop — non-actionable). announce_only_future kept as the _alloc tokio convenience that allocs internally. - publish_event_with_buffers: undersized protected_buf (E2E BufferTooSmall) now maps to Capacity("udp_buffer") for contract symmetry with msg_buf. - publish_raw_event_with_buffers: direct undersized-buffer regression test. - Wrapper docs trued-up (buffer-length contract, not UDP_BUFFER_SIZE); CHANGELOG breaking-signature note. Fixed a cross-cfg doc link. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Valid findings fixed (all landed here on the PR3 branch per request): - #133: publish_raw_event_with_buffers now guards buf.len() < 16 explicitly, so an empty payload + sub-header buffer returns Capacity("udp_buffer") instead of a protocol I/O error (the payload>buf-16 guard saturated to 0>0 and missed it). + regression test case. - #132 doc accuracy: corrected stale "Box::leak"/"leaks" wording in ClientDeps::tokio, the new_with_* constructor, the socket_manager test_buf helper, and the PR2 plan snippet — TokioBufferProvider is Arc-backed (freed on drop), not leaked. - #132 example: bare_metal_client buffer pool 9 -> 10 (UNICAST_SOCKETS_CAP + discovery + release-lag slack), matching the tokio provider + the doc guidance, to avoid a transient Capacity on evict-then-rebind. - #132 doc: tidied the ServerConfig::with_announce intra-doc link. REJECTED (false positives): the ~26 "passing &mut [0u8; N] into an async fn and awaiting it fails to compile (temporary dropped while borrowed)" comments on #133. These are inline `helper(&mut [0u8; N]).await` calls — the temporary lives until the end of the statement, so the borrow is valid across the await. The code compiles and is green (bare_metal_e2e 9/9, nextest 521, clippy clean). Copilot misapplied the "store future then await later" rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Port-target branch off the #133 tip with a handoff for Feliciano: the keep/drop/adapt classification of #126's 7 commits, the union NonSdRequestCallback (already in the stack), the target server/E2E/ subscription APIs to adapt polled.rs against, the divergent-API touch points in polled.rs, and the rationale for NOT adopting the const-generic E2ERegistry refactor into the stack. polled.rs is a port (not a rebase) because it sits on #126's own divergent server/e2e rework. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ecv loop A bare-metal provider that co-offers several services over one shared SD/unicast socket runs a single receive loop; the others are announce-only with no receive path. That loop must SubscribeAck for every co-offered service, not just its primary — otherwise subscribes for the siblings are NACKed and their events never reach a subscriber. ServerConfig gains accepted_offers (heapless::Vec<AcceptedOffer, 16>) + with_accepted_offer / try_with_accepted_offer / accepts_offer. The SD Subscribe handler short-circuits its four single-service guards when the incoming (service, instance, event_group) matches an accepted offer; the ack/registration path already keys off the entry view, so co-offered subscribes register and ack under their own service. Empty accepted_offers preserves exact single-service behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brings the bare_metal_runtime (callback fn-ptr transport + RxMailbox + embassy executor + single composed task), bare_metal_tasks (offer/subscribe/ rx futures + run_someip), sd_codec (no-alloc SOME/IP+SD builders/parsers), and heapless_payload over from the embassy union branch, adapted to the #125 stack APIs: - recv-only via Server::run_with_buffers + ServerConfig::with_announce(false) (the runtime drives its own combined multi-offer announce); announce_send_buf is untouched when announce is off, so an empty slice is passed. - co-offered Subscribe acceptance via the server's new accepted_offers. - DispatchFn already on the union contract (ctx/source/e2e_status). - HeaderView::upper_header_bytes accessor for sd_codec E2E. Feature bare-metal-runtime = [bare_metal, server, dep:embassy-executor] (nightly static task storage; impl_trait_in_assoc_type at crate root). cargo +nightly check --features bare-metal-runtime: clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Real fixes: doc backticks/indentation (heapless_payload, header, runtime, server docs); map_or -> is_some_and; match -> let...else; elided a redundant lifetime; bundled the sleep-deadline wrap cast behind a named local. Justified #[allow]s (each with a one-line rationale): too_many_arguments on the SD announce/publish helpers (a params struct would only relocate the list); needless_pass_by_value on runtime::init (it reborrows a &'static mut buffers field + is the FFI ownership boundary); cast_possible_wrap on the wrap-aware sleep clock compare; large_enum_variant on HeaplessPayloadKind (no-alloc, can't box); result_large_err on the fallible ServerConfig builders (try_with_accepted_offer and the pre-existing try_with_event_group, which the larger ServerConfig tipped over the lint threshold). cargo +nightly clippy --features bare-metal-runtime and --features server-tokio: 0 warnings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bare-metal-runtime is no-alloc and pulls a nightly-only crate feature (impl_trait_in_assoc_type), so it is mutually exclusive with the alloc features (std/_alloc/embassy_channels/*-tokio). A single --all-features build can no longer compile it (E0308 on the no-alloc server handles; E0554 on stable). The three --all-features invocations (clippy, doc, coverage) move to an enumerated $ALLOC_FEATURES list (every feature except bare-metal-runtime, workspace-verified), and a new nightly 'bare-metal-runtime' job runs clippy + a thumbv7em build-std target build + docs for the runtime's own combos. Together the lanes still cover every feature; neither combines no-alloc with alloc. A compile_error! guard on all(bare-metal-runtime, _alloc) turns the otherwise-cryptic E0308 into an explicit explanation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ported runtime/heapless_payload docs linked to items that don't exist in a no-std/no-alloc doc build: `raw_payload`/`RawPayload` (std-only + private), an unqualified `RxMailbox`, and a stale `Server::recv_only_future` (it's `run_with_buffers` on the #125 server). Under RUSTDOCFLAGS=-D warnings these fail the per-feature doc jobs (`server,bare_metal`, the alloc lane, and the nightly bare-metal-runtime lane). Point RawPayload refs at plain prose, fully-qualify RxMailbox, and fix the stale method link. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
FelicianoAngulo2
force-pushed
the
feat/polled-port-onto-125
branch
from
June 24, 2026 16:03
5516204 to
20b60af
Compare
`new` used `msg_type as u8` — the enum discriminant — as the wire byte. That coincides with the wire value only for Request/RequestNoReturn/ Notification (0/1/2); Response and Error diverge (discriminant 3/4 vs wire 0x80/0x81). The new non-SD getter RESPONSE path is the first code to frame a `Response`, so it is the first to emit the wrong byte (0x03), which a conformant SOME/IP peer rejects. Map each variant to its wire encoding instead; add a unit test covering all five variants + TP flag. Wire-visible change: getter Response/Error datagrams now carry 0x80/0x81. Flagged breaking for any consumer that tolerated the old (invalid) value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…version Two correctness fixes on the shared recv loop, plus their tests. Response framing (#2): dispatch_non_sd_request trusted the callback's returned length and sliced send_buf[..16+len] with no upper bound, so an out-of-range return panicked — and on the bare-metal runtime that panic unwinds across the extern "C" boundary (UB). Bound the length against the response buffer and drop+warn on overflow. Co-offer major version (#6): a Subscribe for a co-offered service was accepted on a (service, instance, event_group) match that ignored the major version, so a mismatched-version subscribe was silently accepted — unlike the primary service, which rejects it. AcceptedOffer now carries major_version and accepts_offer matches it; with_accepted_offer / try_with_accepted_offer take it as a 4th argument (new-in-this-PR API, free pre-release). Bare-metal build_server threads entry.major_version. Tests: responder framing (request-id echo, 0x80 type, body) + oversized- length rejection; co-offered subscribe accepted on matching version and rejected on mismatch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…otent Buffer aliasing (#3): someip_task held a whole-struct &'static mut to RuntimeBuffers for its entire (never-resolving) lifetime, while publish() and deinit() took a second &mut to its tx_scratch / offer_scratch fields — aliasing UB, and an actual data race in the getter+publish case where both write tx_scratch. Add a dedicated, platform-owned publish_scratch (API_SCRATCH_CAP = 512 B, shared by the serial publish/deinit callers, sized to fit deinit's StopOffer; caps publish payloads at 496 B), and switch someip_task to disjoint per-field addr_of_mut! reborrows so no retag ever covers publish_scratch. Verified with Miri: the old whole-struct pattern is flagged Stacked-Borrows UB, the new per-field pattern is clean. Idempotent init (#4): init() returned 0 on a repeat/re-entrant call (after only an EXECUTOR_INIT load), silently ignoring the new config and leaking the platform's freshly-handed &'static mut buffers. Claim init atomically via a new INIT_CLAIMED compare_exchange at entry (closing the race/re-entrancy window) and return -4 on re-entry so the caller knows its config was not adopted; release the claim on the -1/-2 failure paths so a corrected retry can proceed. EXECUTOR_INIT stays the poll-ready gate, published last so poll() never touches an uninitialized executor. +512 B static RAM (publish_scratch) + 1 byte (INIT_CLAIMED); no future- size impact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t variables to always build with exact required memory
JustinKovacich
added a commit
that referenced
this pull request
Jun 27, 2026
…criber test after #134 rebase bf21b2c made the SOME/IP server caps env-configurable but lowered the *defaults* (SUBSCRIBERS_PER_GROUP 16->1, EVENT_GROUPS_CAP 32->4, ServerConfig EVENT_GROUP_IDS_CAP 32->1, ACCEPTED_OFFERS_CAP 16->4) for every build. Those consts are shared with the std/tokio server, so plain std consumers that never inject SIMPLE_SOMEIP_MAX_* were silently capped (e.g. one subscriber per event group), and 11 lib unit tests + the multi-subscriber integration test went red. - Gate the fallback defaults by feature: bare-metal keeps the tight values (the host build injects exact sizes via the env vars — the memory win), std builds keep the historical generous defaults. Env override still applies to both via from_env_or(option_env!(...)). - Expose ServerConfig::SUBSCRIBERS_PER_GROUP_CAP so callers/tests can adapt to the build-time capacity. - from_env_or: fix clippy::pedantic (doc backticks, manual_assert, i32->usize sign-loss via is_ascii_digit) and gate on `server` (its only callers) to drop the dead_code warning on client-only / bare_metal-only builds. Test fixes (rebase artifacts from porting onto #134): - test_multiple_subscribers_receive_events / test_client_send_sd_auto_binds_discovery sent to Ipv4Addr::LOCALHOST while create_server binds SERVER_IP (127.0.0.2, the #130 dual-socket split) -> subscribes hit a dead address, 0 registered. Restore SERVER_IP. Also gate the second-subscriber assertion on SUBSCRIBERS_PER_GROUP_CAP so it passes at the bare-metal default of 1 and exercises the real fan-out under SIMPLE_SOMEIP_MAX_SUBS>=2. - inner.rs: restore the discovery_unicast_socket teardown in unbind_discovery dropped during the rebase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JustinKovacich
added a commit
that referenced
this pull request
Jun 27, 2026
…criber test after #134 rebase bf21b2c made the SOME/IP server caps env-configurable but lowered the *defaults* (SUBSCRIBERS_PER_GROUP 16->1, EVENT_GROUPS_CAP 32->4, ServerConfig EVENT_GROUP_IDS_CAP 32->1, ACCEPTED_OFFERS_CAP 16->4) for every build. Those consts are shared with the std/tokio server, so plain std consumers that never inject SIMPLE_SOMEIP_MAX_* were silently capped (e.g. one subscriber per event group), and 11 lib unit tests + the multi-subscriber integration test went red. - Gate the fallback defaults by feature: bare-metal keeps the tight values (the host build injects exact sizes via the env vars — the memory win), std builds keep the historical generous defaults. Env override still applies to both via from_env_or(option_env!(...)). - Expose ServerConfig::SUBSCRIBERS_PER_GROUP_CAP so callers/tests can adapt to the build-time capacity. - from_env_or: fix clippy::pedantic (doc backticks, manual_assert, i32->usize sign-loss via is_ascii_digit) and gate on `server` (its only callers) to drop the dead_code warning on client-only / bare_metal-only builds. Test fixes (rebase artifacts from porting onto #134): - test_multiple_subscribers_receive_events / test_client_send_sd_auto_binds_discovery sent to Ipv4Addr::LOCALHOST while create_server binds SERVER_IP (127.0.0.2, the #130 dual-socket split) -> subscribes hit a dead address, 0 registered. Restore SERVER_IP. Also gate the second-subscriber assertion on SUBSCRIBERS_PER_GROUP_CAP so it passes at the bare-metal default of 1 and exercises the real fan-out under SIMPLE_SOMEIP_MAX_SUBS>=2. - inner.rs: restore the discovery_unicast_socket teardown in unbind_discovery dropped during the rebase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Auto-closed when its stacked base branch was deleted during post-0.8.0 branch cleanup. Base content is now in |
JustinKovacich
changed the base branch from
feature/pr3_125_server_buffers
to
main
June 29, 2026 20:45
Contributor
Author
|
Verified fully folded into |
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
2 participants
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.
Port #126 (polled bare-metal) onto the #125 stack — handoff
Draft / WIP for @FelicianoAngulo2. This branch is the port target for #126, branched off the #133 tip (top of the completed #125 stack:
#131 → #127 → #129 → #132 → #133). It builds clean and currently contains only the handoff doc.Full handoff:
docs/simple_someip/plans/2026-06-17-pr126-polled-port-handoff.md.Why a port, not a rebase of #126
#126'spolled.rsis built on its own divergent refactor of the alloc-free server, theE2ERegistry(const-generic), and a newSingleContextRawMutex— and the #125 stack reworked those same areas differently (#131 alloc-free, PR 2/PR 3 e2e + caller-buffer model). A straightgit rebaseof #126 onto #133 hits 15 conflict hunks and replays commits that duplicate/contradict the stack. Sopolled.rsgets ported onto the stack's APIs rather than rebased.Decision already made (the union callback)
NonSdRequestCallbackis already the agreed union on the stack (src/server/mod.rs:666):fn(ctx: usize, source: SocketAddrV4, service_id: u16, method_id: u16, payload: &[u8], e2e_status: u8)— library-side parse,ctx/source/e2e_statuscarried. So #126's4c2531d "remove non-SD observer"is dropped (we keep the observer); no stack change needed.#126 commits — keep / drop / adapt
4faac4a(alloc-free dup — superseded by feat(server): make server alloc-free on bare_metal, and surface inbound non-SD unicast #131),4c2531d(observer removal — superseded by the union).25ba82b,3b8e483,e7c956c(thepolled.rscontent).929962a(const-genericE2ERegistry) —polled.rsadapts to the stack's non-const-generic registry rather than the stack adopting the refactor (it would re-open audited E2E code, force an API migration on consumers like dft, and add flash cost).372c7d4(SingleContextRawMutex) — bring the primitive if polled needs it; your call.What's needed (see the handoff doc for the full adaptation list + suggested order)
git checkout origin/feat/polled-bared-metal -- src/polled.rsonto this branch; wiremod polled;.DispatchFn; clear the open feat(bare_metal): Add simple_someip polled based for bare-metal builds #126 review punch list; verify the thumb/no-alloc builds.Stacked on #133 (base
feature/pr2_125_client_async_state's childfeature/pr3_125_server_buffers); keep stacked, never merge-down. #128 (embassy-mem-channel-cap, still a draft) stacks after this lands.🤖 Generated with Claude Code