Skip to content

Refactor: Implement no_std and zero-copy API#42

Open
zheylmun wants to merge 58 commits into
mainfrom
feature/no_std
Open

Refactor: Implement no_std and zero-copy API#42
zheylmun wants to merge 58 commits into
mainfrom
feature/no_std

Conversation

@zheylmun

@zheylmun zheylmun commented May 27, 2026

Copy link
Copy Markdown
Contributor

This refactoring introduces no_std compatibility and a new zero-copy API for the UDS crate, enabling its use in embedded and bare-metal environments without relying on the Rust std library.

Key Changes:

  • New Serialization Traits: Replaces the WireFormat, SingleValueWireFormat, and IterableWireFormat traits with a new Encode/Decode/DecodeIter trait family. These new traits are designed for no_std and facilitate zero-copy operations.
  • Zero-Copy Types: Introduces zero-copy Tx (transmit) and Rx (receive) types for requests, responses, and associated payloads. These types borrow from existing buffers, minimizing allocations and making the API suitable for resource-constrained systems.
  • no_std Support (opt-in): The crate can be built no_std by disabling default features (default-features = false). The std feature is enabled by default for backwards compatibility; std and alloc gate Vec-based types and other conveniences. Build with default-features = false (optionally + alloc) for bare-metal targets.
  • Error Handling: Updates the error type to use embedded_io::ErrorKind for I/O errors, ensuring no_std compatibility. The std::io::Error conversion preserves the original ErrorKind.
  • Primitive & Service Integration: All fixed-size primitives and core UDS services have been migrated to implement the new Encode and Decode traits, leveraging direct byte conversions (to_be_bytes, from_be_bytes) instead of byteorder where possible.
  • Build System Updates: Adds new CI jobs to validate no_std builds on bare-metal targets (thumbv6m-none-eabi) with and without the alloc feature.
  • Dependency Adjustments: Replaces byteorder with byteorder-embedded-io and removes tracing, further reducing the std footprint.

This change significantly improves the crate's flexibility and performance, especially for embedded systems development.


Update — architectural review & API alignment (follow-up scope)

Before landing, we did a thorough architectural review to align the public API with the revised scope: a pure, synchronous, no_std/no_alloc UDS codec that owns no transport or async runtime (DoIP/UDSonIP and friends live downstream). Design spec and step-by-step plan are committed under docs/superpowers/. Guiding principle: simplicity for C developers new to Rust — concrete types over generics.

API changes (breaking, intentionally done before publish to minimize churn):

  • Removed the orphaned DiagnosticDefinition trait + UdsSpec — nothing consumed them.
  • Stripped all generic identifier machinery — removed the Identifier/RoutineIdentifier traits, the impl_identifier! macro, the blanket trait impls, and ProtocolIdentifier/ProtocolPayloadTx/ProtocolRoutinePayloadTx. Kept the concrete UDSIdentifier/UDSRoutineIdentifier enums with direct Encode/Decode impls.
  • De-genericized the service builders to carry raw payloads: ReadDataByIdentifierRequestTx over &[u16]; WriteDataByIdentifierRequestTx over &[u8] with a fixed u16 WriteDataByIdentifierResponse; RoutineControlRequestTx/RoutineControlResponseTx over raw bytes.
  • Strict Tx/Rx naming — fixed-size bidirectional types take no suffix; borrowed TX/RX types take Tx/Rx (renamed the stragglers).
  • Symmetric Other { service, data } pass-through on both Request and Response for known-but-unmodeled services; removed the redundant UdsResponse type and the Error::ServiceNotImplemented variant.
  • Moved is_positive_response_suppressed off the Encode trait (now inherent on Request and the service types), keeping the codec trait purely about serialization. This incidentally fixed a latent bug where CommunicationControl's SPRMIB bit could be set on the wire while the query returned false.

Quality & docs:

  • Added an encode/encoded_size agreement test helper and applied it crate-wide (guards against the two drifting).
  • Documented the Decode remainder/borrow contract in C-familiar terms; added a README Integration section (runtime-agnostic usage + borrow semantics) and a Service coverage boundary; corrected the README intro, which previously (and now incorrectly) called embedded support a non-goal.
  • Fixed the no_std test matrix: test code used bare vec!/Vec under #![no_std], so cargo test --no-default-features[ --features alloc] didn't compile. Gated those alloc-dependent tests behind feature = "alloc" and forwarded embedded-io/alloc in the alloc feature so Vec<u8>: embedded_io::Write resolves.

Verification (full matrix green): builds on std / --no-default-features --features alloc / --no-default-features and both thumbv6m-none-eabi combos; cargo test passes (94 std/alloc, 74 pure-no_std); clippy clean on all three host combos; rustdoc clean; cargo fmt --check clean.

zheylmun added 13 commits April 2, 2026 13:41
- Add std/alloc feature flags with std as default
- Add embedded-io dependency, disable default features on thiserror/tracing
- Replace IoError(std::io::Error) with IoError(embedded_io::ErrorKind)
- Drop Vec<u8> from InvalidDiagnosticIdentifierPayload error variant
- Drop String from ReservedForLegislativeUse error variant
- Replace panicking From<u8> with TryFrom for RoutineControlSubFunction and DtcSettings
- Replace format!+Vec.join() Debug impls with core::fmt loops
- Add #![cfg_attr(not(feature = "std"), no_std)] to lib.rs
- Add new no_std-compatible trait hierarchy: Encode (TX), Decode<'a> (RX),
  and DecodeIter<'a> (RX streaming) alongside existing WireFormat traits
- Fix byteorder-embedded-io and embedded-io dependencies to use
  default-features = false, propagating std feature properly
- Gate Vec<u8> WireFormat impls behind alloc feature
Add no_std-compatible Encode/Decode impls for:
- All integer primitives (u8-u128, i8-i128), f32, f64
- DTCRecord (encode, decode, decode_iter)
- TesterPresent (request + response)
- DiagnosticSessionControl (request + response)
- EcuReset (request + response)
- CommunicationControl (request + response)
- ControlDTCSettings (request + response)
- ClearDiagnosticInfo (request)
- RequestDownload (request, with stack-buffer encode replacing temp Vec)
- NegativeResponse
- Blanket Encode/Decode/DecodeIter for Identifier types

Also adds Error::io() helper for embedded_io error conversion and
disambiguates existing test code to use fully-qualified WireFormat calls.
Create borrowed TX types alongside existing Vec-based types:
- TransferDataRequestTx<'d> / TransferDataResponseTx<'d>
- SecurityAccessRequestTx<'d> / SecurityAccessResponseTx<'d>
- RequestDownloadResponseTx<'d>
- ReadDataByIdentifierRequestTx<'d, DID>
- ProtocolPayloadTx<'d> / ProtocolRoutinePayloadTx<'d>

All TX types use &'d [u8] or &'d [T] instead of Vec, implement Encode
+ Decode, and support const fn construction where possible.
Add lazy iterator types for zero-alloc DTC record parsing:
- DtcAndStatusIter: iterates (DTCRecord, DTCStatusMask) pairs
- DtcFaultDetectionIter: iterates DTCFaultDetectionCounterRecord
- DtcSeverityAndStatusIter: iterates (DTCSeverityMask, DTCRecord, DTCStatusMask)

Add ReadDTCInfoResponseRx<'a> enum with zero-copy Decode impl
covering subfunctions 0x01-0x02, 0x07-0x09, 0x0A-0x0E, 0x14-0x15,
0x42. Stores raw record bytes and provides lazy iterators.
- Add DiagnosticDefinitionTx trait with Encode-based bounds for no_std TX
- Implement DiagnosticDefinitionTx for UdsSpec
- Add RequestRx<'a> enum: zero-copy request decoding from byte slices
- Add ResponseRx<'a> enum: zero-copy response decoding from byte slices
- Add UdsResponseRx<'a>: raw zero-copy response (replaces UdsResponse)
- RX enums don't require DiagnosticDefinition — variable payloads stored
  as raw &'a [u8] for on-demand parsing
Behind #[cfg(feature = "alloc")]:
- collect_all() on DtcAndStatusIter, DtcFaultDetectionIter,
  DtcSeverityAndStatusIter for collecting lazy iterators into Vec
- to_owned() on TransferDataRequestTx/ResponseTx for converting
  to allocating TransferDataRequest/Response
- to_owned() on SecurityAccessRequestTx/ResponseTx for converting
  to allocating SecurityAccessRequest/Response
- extern crate alloc when alloc feature is enabled
- Deprecate WireFormat, SingleValueWireFormat, IterableWireFormat traits
  with messages pointing to Encode/Decode/DecodeIter replacements
- Deprecate UdsResponse in favor of UdsResponseRx
- Add roundtrip tests for the new no_std API:
  - TesterPresent Encode/Decode roundtrip
  - TransferDataRequestTx Encode/Decode roundtrip
  - ResponseRx decoding (TesterPresent, NegativeResponse)
  - RequestRx decoding (EcuReset)
  - DtcAndStatusIter lazy parsing
  - Const construction verification
BREAKING: Remove the entire old API surface:
- Remove WireFormat, SingleValueWireFormat, IterableWireFormat traits
- Remove old DiagnosticDefinition trait (std-dependent bounds)
- Remove old Request<D>/Response<D> enums and all builder methods
- Remove old Vec-based service types (SecurityAccessRequest,
  TransferDataRequest, etc.) and their impls
- Remove old UdsResponse, ProtocolPayload, ProtocolRoutinePayload
- Remove ReadDTCInfoResponse (old Vec-based) and all subfunction
  response types
- Remove ReadDataByIdentifierRequest/Response (old Vec-based)
- Remove RoutineControlRequest/Response (old Vec-based)
- Remove WriteDataByIdentifierRequest/Response (old Vec-based)
- Remove RequestFileTransferRequest/Response impls
- Remove all WireFormat/SingleValueWireFormat/IterableWireFormat impls

Rename no_std types to take over clean names:
- DiagnosticDefinitionTx → DiagnosticDefinition
- RequestRx → Request, ResponseRx → Response
- UdsResponseRx → UdsResponse
- *Tx<'d> types keep Tx suffix for now (renamed in follow-up)
Migrate test modules from removed WireFormat/SingleValueWireFormat to
the new Encode/Decode API:
- Replace WireFormat::encode with Encode::encode
- Replace SingleValueWireFormat::decode with Decode::decode
- Replace required_size() with encoded_size()
- Switch SecurityAccess/TransferData tests to *Tx types
- Remove RequestFileTransfer encode/decode tests (service not yet
  migrated)
- Remove old read_data_by_identifier, routine_control, and
  write_data_by_identifier tests (types removed)

72 tests passing.
- Make pub(crate) constructors public (users now construct types directly)
- Add doc comments to all newly-public functions
- Remove dead helper methods (get_shortened_memory_address/size,
  MemoryFormatIdentifier::from_values)
- Remove unused constants and type aliases
- Add Default impl for TesterPresentResponse
- Fix all clippy warnings (zero remaining)
Remove the unused `tracing` dependency, which pulled in `tracing-core`
and required atomic CAS unavailable on targets like thumbv6m-none-eabi,
silently blocking real embedded builds.

Migrate remaining services off Vec/String to borrowed-slice Tx types,
add Encode/Decode for the RequestFileTransfer types, and wire them into
the Request/Response dispatch enums. Implement Encode on the Request and
Response enums so a full message (SID byte + payload) can be framed,
making encode/decode symmetric round-trips.

Add a CI job that builds both no_std combos against thumbv6m-none-eabi
and run clippy on the no_std/alloc feature combos as a guardrail.
Copilot AI review requested due to automatic review settings May 27, 2026 02:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the crate’s wire-format layer to support no_std usage and introduces a new zero-copy API surface (slice-based RX and borrowed TX types), alongside feature-gated std/alloc support and updated CI.

Changes:

  • Replaces the old WireFormat/SingleValueWireFormat/IterableWireFormat traits with Encode/Decode/DecodeIter aimed at no_std and slice-based decoding.
  • Introduces/propagates zero-copy *Tx/*Rx service types and updates request/response framing to borrow from input buffers.
  • Updates dependencies/features (embedded-io + byteorder-embedded-io), error handling, and CI jobs for bare-metal targets.

Reviewed changes

Copilot reviewed 31 out of 32 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/traits.rs Introduces new Encode/Decode/DecodeIter traits and updates identifier blanket impls.
src/services/write_data_by_identifier.rs Migrates service request/response encoding to Encode and updates tests.
src/services/transfer_data.rs Adds zero-copy TransferData*Tx types and slice-based decode/encode.
src/services/tester_present.rs Converts to Encode/Decode and adjusts constructors + tests.
src/services/security_access.rs Adds zero-copy SecurityAccess*Tx types and slice-based decode/encode.
src/services/routine_control.rs Refactors routine control TX encoding around Encode.
src/services/request_download.rs Refactors to Encode/Decode and switches to stack-buffer encoding for address/size.
src/services/read_data_by_identifier.rs Replaces Vec-based request with borrowed-slice TX request and simplifies tests.
src/services/negative_response.rs Converts negative response framing to Encode/Decode.
src/services/mod.rs Re-exports updated *Tx/*Rx types and iterators.
src/services/ecu_reset.rs Converts ECU reset to Encode/Decode, updates constructors/tests.
src/services/diagnostic_session_control.rs Converts diagnostic session control to Encode/Decode, updates tests.
src/services/control_dtc_settings.rs Converts control DTC settings to Encode/Decode, updates tests.
src/services/communication_control.rs Converts communication control to Encode/Decode, updates tests.
src/services/clear_dtc_information.rs Converts clear DTC info to Encode/Decode, updates tests.
src/service.rs Switches UdsServiceType formatting to core::fmt for no_std.
src/response.rs Refactors response parsing/encoding to zero-copy Response<'a> and adds UdsResponse<'a>.
src/request.rs Refactors request parsing/encoding to zero-copy Request<'a>.
src/protocol_definitions.rs Introduces borrowed protocol payload types (ProtocolPayloadTx, ProtocolRoutinePayloadTx).
src/lib.rs Enables no_std via feature gating, updates exports, adds new API tests, and adjusts enums to TryFrom.
src/error.rs Refactors error type for no_std (embedded_io::ErrorKind) and adds conversion helpers.
src/common/primitive_generics.rs Implements Encode/Decode for primitives using to_be_bytes/from_be_bytes.
src/common/format_identifiers.rs Removes old wire-format impls and keeps identifier parsing/validation.
src/common/dtc_status.rs Converts DTC-related types to Encode/Decode/DecodeIter and simplifies errors.
src/common/dtc_snapshot.rs Removes old snapshot list logic, keeps record-number type/tests.
src/common/dtc_ext_data.rs Removes old ext-data record/list wire-format implementations.
src/common/diagnostic_identifier.rs Switches formatting traits to core::fmt for no_std.
Cargo.toml Adds std/alloc features and switches deps to embedded-io ecosystem.
Cargo.lock Updates lockfile for new dependencies and transitive updates.
.github/workflows/ci.yml Adds no_std (thumbv6m) build jobs and expands clippy matrix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/request.rs Outdated
Comment on lines +66 to +73
let request = match service {
UdsServiceType::ClearDiagnosticInfo => {
let (req, _) = <ClearDiagnosticInfoRequest as Decode>::decode(payload)?;
Self::ClearDiagnosticInfo(req)
}
Self::EcuReset(_) => EcuResetRequest::allowed_nack_codes(),
Self::SecurityAccess(_) => SecurityAccessRequest::allowed_nack_codes(),
Self::RequestDownload(_) => RequestDownloadRequest::allowed_nack_codes(),
_ => &[NegativeResponseCode::ServiceNotSupported],
}
}
}

impl<T: DiagnosticDefinition> WireFormat for Request<T> {
fn required_size(&self) -> usize {
1 + match self {
Self::ClearDiagnosticInfo(cdi) => cdi.required_size(),
Self::CommunicationControl(cc) => cc.required_size(),
Self::ControlDTCSettings(ct) => ct.required_size(),
Self::DiagnosticSessionControl(ds) => ds.required_size(),
Self::EcuReset(er) => er.required_size(),
Self::ReadDataByIdentifier(rd) => rd.required_size(),
Self::ReadDTCInfo(rd) => rd.required_size(),
Self::RequestDownload(rd) => rd.required_size(),
Self::RequestTransferExit => 0,
Self::RoutineControl(rc) => rc.required_size(),
Self::SecurityAccess(sa) => sa.required_size(),
Self::TesterPresent(tp) => tp.required_size(),
Self::TransferData(td) => td.required_size(),
Self::WriteDataByIdentifier(wd) => wd.required_size(),
}
}

/// Serialization function to write a [`Request`] to a [`Writer`](std::io::Write)
/// This function writes the service byte and then calls the appropriate
/// serialization function for the service represented by self.
fn encode<W: Write>(&self, writer: &mut W) -> Result<usize, Error> {
// Write the service byte
writer.write_u8(self.service().request_service_to_byte())?;
// Write the payload
Ok(1 + match self {
Self::ClearDiagnosticInfo(cdi) => cdi.encode(writer),
Self::CommunicationControl(cc) => cc.encode(writer),
Self::ControlDTCSettings(ct) => ct.encode(writer),
Self::DiagnosticSessionControl(ds) => ds.encode(writer),
Self::EcuReset(er) => er.encode(writer),
Self::ReadDataByIdentifier(rd) => rd.encode(writer),
Self::ReadDTCInfo(rd) => rd.encode(writer),
Self::RequestDownload(rd) => rd.encode(writer),
Self::RequestTransferExit => Ok(0),
Self::RoutineControl(rc) => rc.encode(writer),
Self::SecurityAccess(sa) => sa.encode(writer),
Self::TesterPresent(tp) => tp.encode(writer),
Self::TransferData(td) => td.encode(writer),
Self::WriteDataByIdentifier(wd) => wd.encode(writer),
}?)
}

fn is_positive_response_suppressed(&self) -> bool {
match self {
Self::CommunicationControl(cc) => cc.suppress_positive_response(),
Self::ControlDTCSettings(ct) => ct.is_positive_response_suppressed(),
Self::DiagnosticSessionControl(ds) => ds.suppress_positive_response(),
Self::EcuReset(er) => er.suppress_positive_response(),
Self::SecurityAccess(sa) => sa.suppress_positive_response(),
Self::TesterPresent(tp) => tp.suppress_positive_response(),
_ => false,
}
}
}

impl<D: DiagnosticDefinition> SingleValueWireFormat for Request<D> {
/// Deserialization function to read a [`Request`] from a [`Reader`](std::io::Read)
/// This function reads the service byte and then calls the appropriate
/// deserialization function for the service in question
///
/// *Note*:
///
/// Some services allow for custom byte arrays at the end of the request
/// It is important that only the request data is passed to this function
/// or the deserialization could read unexpected data
#[allow(clippy::too_many_lines)]
fn decode<R: Read>(reader: &mut R) -> Result<Self, Error> {
let service = UdsServiceType::service_from_request_byte(reader.read_u8()?);
Ok(match service {
UdsServiceType::CommunicationControl => {
Self::CommunicationControl(CommunicationControlRequest::decode(reader)?)
let (req, _) = <CommunicationControlRequest as Decode>::decode(payload)?;
Self::CommunicationControl(req)
Comment thread src/request.rs
UdsServiceType::WriteDataByIdentifier => Self::WriteDataByIdentifier(payload),
_ => return Err(Error::ServiceNotImplemented(service)),
};
Ok((request, &[]))
Comment thread src/request.rs
};
1 + payload
}

Comment thread src/response.rs Outdated
Comment on lines 62 to 71
let response = match service {
UdsServiceType::ClearDiagnosticInfo => Self::ClearDiagnosticInfo,
UdsServiceType::CommunicationControl => {
Self::CommunicationControl(CommunicationControlResponse::decode(reader)?)
let (resp, _) = <CommunicationControlResponse as Decode>::decode(payload)?;
Self::CommunicationControl(resp)
}
UdsServiceType::ControlDTCSettings => {
Self::ControlDTCSettings(ControlDTCSettingsResponse::decode(reader)?)
let (resp, _) = <ControlDTCSettingsResponse as Decode>::decode(payload)?;
Self::ControlDTCSettings(resp)
}
Comment thread src/response.rs
UdsServiceType::WriteDataByIdentifier => Self::WriteDataByIdentifier(payload),
_ => return Err(Error::ServiceNotImplemented(service)),
};
Ok((response, &[]))
Comment thread Cargo.toml Outdated
Comment on lines +21 to +24
[features]
default = ["std"]
std = ["alloc", "byteorder-embedded-io/std", "embedded-io/std", "thiserror/std"]
alloc = []
Comment thread src/services/request_download.rs Outdated
Comment on lines +56 to +61
let memory_address_length = (u64::BITS - memory_address.leading_zeros()).div_ceil(8) as u8;
let memory_size_length = (u32::BITS - memory_size.leading_zeros()).div_ceil(8) as u8;
let address_and_length_format_identifier = MemoryFormatIdentifier {
memory_size_length,
memory_address_length,
};
Comment thread src/error.rs Outdated
Comment on lines +86 to +87
fn from(_err: std::io::Error) -> Self {
Self::IoError(embedded_io::ErrorKind::Other)
Two CI failures (both gated by RUSTFLAGS=-Dwarnings):

- Deriving serde::Deserialize on RequestFileTransferRequestTx/
  ResponseTx failed because their variants hold lifetime-bearing
  types (NamePayloadTx<'a>, SentDataPayloadTx<'a>). serde only
  auto-borrows literal &str/&[u8], so the generated Deserialize<'de>
  impl lacked the 'de: 'a bound. Add cfg-gated serde(borrow) to each
  variant field carrying 'a.
- Remove the unused `Identifier` import in the traits test module;
  the trait is implemented via the macro's $crate::Identifier path.
@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.64080% with 241 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/response.rs 56.69% 55 Missing ⚠️
src/request.rs 63.28% 47 Missing ⚠️
src/services/request_file_transfer.rs 94.47% 26 Missing ⚠️
src/common/primitive_generics.rs 68.05% 23 Missing ⚠️
src/services/request_download.rs 81.25% 18 Missing ⚠️
src/common/dtc_status.rs 84.46% 16 Missing ⚠️
src/error.rs 0.00% 9 Missing ⚠️
src/services/control_dtc_settings.rs 87.50% 5 Missing ⚠️
src/services/security_access.rs 90.90% 5 Missing ⚠️
src/services/tester_present.rs 86.11% 5 Missing ⚠️
... and 14 more
Files with missing lines Coverage Δ
src/common/format_identifiers.rs 94.04% <ø> (-1.10%) ⬇️
src/services/read_data_by_identifier.rs 86.95% <100.00%> (-1.90%) ⬇️
src/services/read_dtc_information.rs 39.12% <ø> (-43.92%) ⬇️
src/services/write_data_by_identifier.rs 95.45% <100.00%> (+16.18%) ⬆️
src/traits.rs 100.00% <100.00%> (+18.08%) ⬆️
src/common/dtc_snapshot.rs 83.72% <95.00%> (-2.20%) ⬇️
src/common/util.rs 98.59% <97.72%> (-1.41%) ⬇️
src/service.rs 27.34% <0.00%> (+24.21%) ⬆️
src/services/clear_dtc_information.rs 92.72% <96.29%> (-1.03%) ⬇️
src/services/negative_response.rs 97.36% <96.42%> (+97.36%) ⬆️
... and 19 more

... and 1 file with indirect coverage changes

- impl_identifier!: macro lives at crate root, qualify the link
  with (crate::impl_identifier).
- TransferDataRequest / SecurityAccessRequest: point at the actual
  Tx-suffixed types (TransferDataRequestTx, SecurityAccessRequestTx).
- DTCSeverityRecord: link to the real iterator DtcSeverityAndStatusIter.
- DTCExtDataRecord: not a Rust type (UDS-spec concept), de-link to
  plain backticks matching the existing usage in the same doc.

cargo doc --all-features --no-deps now passes under RUSTDOCFLAGS=-Dwarnings.
Copilot AI review requested due to automatic review settings June 1, 2026 19:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 5 comments.

Comment thread src/lib.rs Outdated
Comment on lines +50 to +51
type RoutinePayload = ProtocolRoutinePayloadTx<'static>;
type DiagnosticPayload = ProtocolPayloadTx<'static>;
Comment thread src/traits.rs Outdated
Comment on lines +84 to +87
fn encode(&self, writer: &mut impl embedded_io::Write) -> Result<usize, Error> {
writer
.write_all(&<u16 as Into<u16>>::into((*self).into()).to_be_bytes())
.map_err(Error::io)?;
Comment thread src/request.rs
UdsServiceType::WriteDataByIdentifier => Self::WriteDataByIdentifier(payload),
_ => return Err(Error::ServiceNotImplemented(service)),
};
Ok((request, &[]))
Comment thread src/services/request_download.rs Outdated
Comment on lines +56 to +61
let memory_address_length = (u64::BITS - memory_address.leading_zeros()).div_ceil(8) as u8;
let memory_size_length = (u32::BITS - memory_size.leading_zeros()).div_ceil(8) as u8;
let address_and_length_format_identifier = MemoryFormatIdentifier {
memory_size_length,
memory_address_length,
};
Comment on lines 14 to 19
impl<Payload: Encode> WriteDataByIdentifierRequest<Payload> {
/// Create a new write-by-identifier request.
pub fn new(payload: Payload) -> Self {
Self { payload }
}

/// Get the allowed Nack codes for this request
#[must_use]
pub fn allowed_nack_codes() -> &'static [NegativeResponseCode] {
&WRITE_DID_NEGATIVE_RESPONSE_CODES
}
}
zheylmun added 8 commits June 1, 2026 16:38
Replace the convoluted `<u16 as Into<u16>>::into((*self).into())` with the
equivalent `Into::<u16>::into(*self).to_be_bytes()`.
Previously every std::io::Error collapsed to ErrorKind::Other, discarding
WouldBlock/TimedOut/etc. embedded_io provides From<std::io::ErrorKind>, so
map through err.kind() instead.
A memory_address or memory_size of 0 produced a 0-length nibble in the
MemoryFormatIdentifier, which is invalid per ISO-14229 (lengths must be >=1)
and encoded a frame that could not be decoded back. Clamp both computed
lengths to a minimum of 1 and add a round-trip regression test.
The helper and its WRITE_DID_NEGATIVE_RESPONSE_CODES constant were dropped
during the refactor while every other service kept theirs, an inconsistent
public-API regression. Restore both for consistency.
Hard-coding 'static on DiagnosticPayload/RoutinePayload prevented UdsSpec from
being used with the new borrowed zero-copy Tx payloads, defeating their
purpose. Parameterize the trait as DiagnosticDefinition<'a> and bind the
payload associated types to 'a; the DID/RID identifier types remain 'static.
Request implements Encode but used the default impl, always reporting false
even when the wrapped request had SPRMIB set. Forward to the inner request for
the five suppressible variants (ControlDTCSettings, DiagnosticSessionControl,
EcuReset, SecurityAccess, TesterPresent).
Request::decode and Response::decode discarded the remainder from inner Decode
calls and unconditionally returned an empty remainder, silently accepting
trailing bytes after a well-formed frame. Add Decode::decode_exact, which
errors on any unconsumed bytes, and use it for the fixed-size sub-decoders so a
single buffer is treated as exactly one frame.
Architectural review before landing the no_std refactor: pure sync codec
scope, remove orphaned DiagnosticDefinition + identifier machinery,
de-genericize builders, unify Tx/Rx naming and the raw-passthrough escape
hatch, and codec-trait hygiene.
zheylmun added 10 commits June 2, 2026 14:00
Add backticks to service names in the README support table and to
DoIP/UDSonIP in the integration section. Convert the floating ///
comment block in lib.rs (which clippy mis-attributed as a doc comment
for SUCCESS) to regular // comments to silence empty_line_after_doc_comments.
The coverage notes were plain // comments invisible to rustdoc; move
them into the README front-page docs where they render.
The intro still claimed embedded support was a non-goal, contradicting
the no_std rearchitecture and the new Integration section.
Pre-existing: test code used bare vec!/Vec under #![no_std]. Gate those
tests behind feature=alloc and import alloc::{vec, vec::Vec} so all
feature combos compile and std/alloc combos run. Also forward the alloc
feature to embedded-io so Vec<u8> implements embedded_io::Write under
alloc-only builds.
zheylmun added 16 commits June 3, 2026 12:19
CI sets RUSTFLAGS=-Dwarnings, so the unused `vec::Vec` import (the test
module uses only `vec!`) failed the all-features Build-and-Test and MSRV
jobs. Import only `alloc::vec`.
Phase 1 (deliberate crate-root exports + complete the codec-incomplete
descriptors) and a Phase 2 open-questions framing (naming, typed-vs-raw
RX symmetry, codec dedup) for landing the no_std breaking change as one
cohesive chunk.
Resolves the Phase 2 open questions: suffix-marks-asymmetry naming rule,
wrapping descriptors into the Request/Response enums (RDBI excepted),
variable-length integer codec dedup, and primitive macro merge.
Copilot AI review requested due to automatic review settings June 4, 2026 00:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 40 out of 42 changed files in this pull request and generated 6 comments.

Comment thread src/common/dtc_status.rs
Comment on lines +468 to 474
impl<'a> Decode<'a> for DTCStoredDataRecordNumber {
fn decode(buf: &'a [u8]) -> Result<(Self, &'a [u8]), Error> {
if buf.is_empty() {
return Err(Error::InsufficientData(1));
}
Ok(Self(value))
}
}

impl From<u8> for DTCStoredDataRecordNumber {
fn from(value: u8) -> Self {
Self(value)
Ok((Self::from(buf[0]), &buf[1..]))
}
Comment on lines +309 to +316
impl<'a> Decode<'a> for UDSIdentifier {
fn decode(buf: &'a [u8]) -> Result<(Self, &'a [u8]), Error> {
if buf.len() < 2 {
return Err(Error::IncorrectMessageLengthOrInvalidFormat);
}
let raw = u16::from_be_bytes([buf[0], buf[1]]);
Ok((Self::try_from(raw)?, &buf[2..]))
}
Comment on lines +332 to +339
impl<'a> Decode<'a> for UDSRoutineIdentifier {
fn decode(buf: &'a [u8]) -> Result<(Self, &'a [u8]), Error> {
if buf.len() < 2 {
return Err(Error::IncorrectMessageLengthOrInvalidFormat);
}
let raw = u16::from_be_bytes([buf[0], buf[1]]);
Ok((Self::from(raw), &buf[2..]))
}
Comment on lines +49 to 52
impl<'a> Decode<'a> for WriteDataByIdentifierRequest<'a> {
fn decode(buf: &'a [u8]) -> Result<(Self, &'a [u8]), Error> {
Ok((Self { payload: buf }, &[]))
}
Comment on lines +69 to +79
impl<'a> Decode<'a> for RoutineControlRequest<'a> {
fn decode(buf: &'a [u8]) -> Result<(Self, &'a [u8]), Error> {
if buf.is_empty() {
return Err(Error::InsufficientData(1));
}
let sub_function = SuppressablePositiveResponse::try_from(buf[0])?;
Ok((
Self {
sub_function,
raw_payload: &buf[1..],
},
Comment thread src/test_util.rs
Comment on lines +9 to +18
pub(crate) fn assert_encode_size_agrees<T: Encode>(value: &T) {
let mut buf = [0u8; 512];
let mut writer = buf.as_mut_slice();
let written = value.encode(&mut writer).unwrap();
assert_eq!(
written,
value.encoded_size(),
"encode wrote {written} bytes but encoded_size() reported {}",
value.encoded_size()
);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants