Refactor: Implement no_std and zero-copy API#42
Open
zheylmun wants to merge 13 commits into
Open
Conversation
- 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.
There was a problem hiding this comment.
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/IterableWireFormattraits withEncode/Decode/DecodeIteraimed atno_stdand slice-based decoding. - Introduces/propagates zero-copy
*Tx/*Rxservice 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 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) |
| UdsServiceType::WriteDataByIdentifier => Self::WriteDataByIdentifier(payload), | ||
| _ => return Err(Error::ServiceNotImplemented(service)), | ||
| }; | ||
| Ok((request, &[])) |
| }; | ||
| 1 + payload | ||
| } | ||
|
|
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) | ||
| } |
| UdsServiceType::WriteDataByIdentifier => Self::WriteDataByIdentifier(payload), | ||
| _ => return Err(Error::ServiceNotImplemented(service)), | ||
| }; | ||
| Ok((response, &[])) |
Comment on lines
21
to
+24
| [features] | ||
| default = ["std"] | ||
| std = ["alloc", "byteorder-embedded-io/std", "embedded-io/std", "thiserror/std"] | ||
| alloc = [] |
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
+86
to
+87
| fn from(_err: std::io::Error) -> Self { | ||
| Self::IoError(embedded_io::ErrorKind::Other) |
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.
This refactoring introduces
no_stdcompatibility and a new zero-copy API for the UDS crate, enabling its use in embedded and bare-metal environments without relying on the Ruststdlibrary.Key Changes:
WireFormat,SingleValueWireFormat, andIterableWireFormattraits with a newEncode/Decode/DecodeItertrait family. These new traits are designed forno_stdand facilitate zero-copy operations.Tx(transmit) andRx(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_stdby Default: The crate is nowno_stdcompatible by default.stdandallocfunctionalities are gated behind features, allowing users to opt-in toVec-based types and other conveniences whenallocorstdare available.embedded_io::ErrorKindfor I/O errors, ensuringno_stdcompatibility.EncodeandDecodetraits, leveraging direct byte conversions (to_be_bytes,from_be_bytes) instead ofbyteorderwhere possible.no_stdbuilds on bare-metal targets (thumbv6m-none-eabi) with and without theallocfeature.byteorderwithbyteorder-embedded-ioand removestracing, further reducing thestdfootprint.This change significantly improves the crate's flexibility and performance, especially for embedded systems development.