refactor!: encapsulation, serde validation, and the churn-audit corrections - #54
Merged
Merged
Conversation
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## fix/wire-conformance #54 +/- ##
========================================================
+ Coverage 89.19% 89.39% +0.20%
========================================================
Files 29 31 +2
Lines 5014 5432 +418
========================================================
+ Hits 4472 4856 +384
- Misses 542 576 +34 |
Most of the crate already hedges against ISO adding values, but eleven public types were missed. `ReadDTCInfoSubFunction` is the notable one: it sits directly above `ReadDTCInfoResponse`, which does have the attribute, and it has an in-source TODO plus sub-functions the crate does not model yet — so adding 0x1B post-tag would have been a breaking change. Enums (ISO reserves value space each one will grow into): ReadDTCInfoSubFunction, FileOperationMode, DTCExtDataRecordNumber, DTCSnapshotRecordNumber Structs with `pub` fields, matching how every other request/response struct in the crate is already declared: DTCFaultDetectionCounterRecord, SizePayload, NamePayload, SentDataPayload, FileSizePayload, DirSizePayload, PositionPayload Deliberately excluded, where the attribute would constrain nothing: private-field newtypes that can only be built through `new()` (DTCRecord, DataFormatIdentifier, SecurityAccessLevel, DTCStoredDataRecordNumber, the three DTC iterators) and the bitmask_enum-generated masks, where all 256 values are valid.
Every other public request/response type carries `#[cfg_attr(feature = "serde", derive(..))]` and the utoipa equivalent. The two macro-generated `RequestTransferExit` types carried neither, with no documented reason and no obstacle — their only field is `&[u8]`, so `#[serde(borrow)]` applies exactly as it does on `TransferDataRequest`. Enabling `serde` therefore left 2 of 32 public types unserializable. Pinned with the existing `derive_contract` test convention. Also drops the `serde_bytes` optional dependency. It was activated by the `serde` feature but referenced nowhere in the crate. `ReadDataByIdentifierRequest` keeps its carve-out: the `Dids::Native(&[u16])` zero-copy backing genuinely has no borrowed `Deserialize` impl, and the existing comment documents that correctly.
`RequestDownloadRequest::new` / `RequestUploadRequest::new` derive the *minimal* width for the address and size, but ISO 14229-1:2020 Table 441 makes the addressAndLengthFormatIdentifier a client choice, not a function of the values. Table 462's own example declares 3 bytes of memorySize for 0x00FFFF, which needs only 2 -- so the crate could not reproduce the standard's example frame, and it cannot talk to the many bootloaders that mandate a fixed ALFID (commonly 0x44) and answer requestOutOfRange otherwise. Add `new_with_widths`, which validates both widths against Table H.1 and rejects a value too large for the width declared for it -- the case that would otherwise truncate silently on the wire. `new` stays as the convenience path. Also document what `RoutineControlResponse::status_record` actually spans. Table 428 places an optional `routineInfo` byte at #5, immediately before the status record, and leaves its presence to the vehicle manufacturer -- nothing on the wire distinguishes the two layouts, so a general-purpose decoder cannot split them. The field name suggested it held only the status record while it in fact holds both, which shifts every positional read by one for a server that sends routineInfo. Modelling it as `Option<u8>` would require the decoder to know something the wire does not carry, so the honest fix is to say so, and to pin it with a test before someone "corrects" it into a wrong split.
`derive(Deserialize)` ignores both field visibility and constructor validation,
so for every type whose invariant lives in `new`/`try_from` it was a second,
unchecked way in. Proven from an integration test, which sees the crate as a
downstream user does:
serde_json::from_str::<SecurityAccessLevel>("255") -> Ok before, Err now
That one mattered: a level must fit 0x00..=0x7F because bit 7 of the
sub-function byte is SPRMIB, so the deserialized 0xFF encoded to a byte that
decoded back as a *different* level with suppression set. `DataFormatIdentifier`
likewise accepted `{"encryption_method":255,...}` for a field `new` caps at
0x0F, and the range-checked enums let a caller name a reserved variant directly
and skip `try_from` entirely.
These six types are one protocol byte, so route their serde impls through `u8`
with `serde(try_from = "u8", into = "u8")` -- `from` for `DataFormatIdentifier`,
whose byte form is total. That makes the serialized form the wire byte, which is
both unambiguous and impossible to construct an out-of-range value from.
A derived `ToSchema` would then describe the Rust shape (an object of nibbles, or
a oneOf of variant names) and disagree with what serde actually reads, so the six
get a hand-written integer schema instead. `SecurityAccessLevel` keeps its derive:
as a newtype its schema is already the inner `u8`.
`serde_json` becomes a dev-dependency for these tests; it is not a dependency of
the crate.
Still outstanding from the same finding: the composite requests
(`TesterPresentRequest`, `RequestDownload`/`UploadRequest`,
`CommunicationControlRequest`) need shadow-struct deserialization routed through
their constructors, which will also remove `ZeroSubFunction` and
`MemoryFormatIdentifier` from the public serde/OpenAPI surface.
Completes the previous commit. `TesterPresentRequest`,
`CommunicationControlRequest` and `RequestDownload`/`RequestUploadRequest` seal
their fields to hold a rule, and a derived `Deserialize` walked straight past
each one:
{"control_type":5,...,"node_id":null} accepted -> encoded 2 bytes that this
crate's own decoder then rejected
{"memory_address":1<<64-1,"memory_address_length":1,...}
accepted -> address truncated to one
byte on the wire
{"zero_sub_function":{"IsoSaeReserved":66}}
named a module-private type and skipped
its 0x00..=0x7F range check
Each now serializes through a private repr of its public wire parameters and
deserializes back through `new` / `new_with_node_id` / `new_with_widths`, so the
constructor's validation is the only way in. That also removes `ZeroSubFunction`
(module-private) and `MemoryFormatIdentifier` (`pub(crate)`) from the serialized
shape and from the generated OpenAPI schema, where they had been giving client
generators types downstream Rust cannot name.
The schemas are hand-written to follow the repr rather than the Rust fields, for
the same reason. `LengthFormatIdentifier` loses its derives too, and
`SuppressablePositiveResponse` loses derives that were pure dead codegen -- it is
never a field of anything.
Verified across the whole feature matrix, including `utoipa` without `serde`,
which is why the reprs are gated on either feature rather than on `serde` alone.
Mutation testing found 11 of 23 mutants surviving. These close the ones that represent real coverage gaps rather than equivalent code. Values the iterators yield were never asserted. Reversing the DTC bytes in `WwhObdDtcSeverityIter` while also swapping severity with status passed the entire suite, because every test touching it checked only counts and `is_ok()`. Same for `DtcAndStatusIter`'s status byte -- "every DTC reports status 0x00" was invisible -- and for all four header fields of the 0x42 response. `allowed_nack_codes_dispatches_for_every_modeled_variant` asserted only `!is_empty()`. Every modeled service has a non-empty table, so any of the 16 arms could return any other service's set. Each frame is now paired with the inherent table it must dispatch to; verified by mis-wiring the ReadDtcInfo arm and watching it fail with "ReadDtcInfo dispatched to the wrong NRC table". `record_lists_must_divide_evenly_into_records` looped `1..width`, so it never tested a list too short to hold one record -- the 0-valid/1-invalid boundary that is the entire point of the check. `all_three_iterators_terminate_and_stay_exhausted` drained with an unbounded `while next().is_some()`, so reverting the iterator fix hung `cargo test` instead of failing it, hiding the other failures. Its sibling's comment says it is bounded precisely to avoid that; now this one is too. Six "round-trip" tests encoded into a buffer they never read, checking only the length. Swapping the two bytes `EcuResetResponse` writes survived in both feature configurations. They now assert the bytes. Ten codec tests were gated on `alloc` only because they used `Vec` as a writer; a stack buffer works identically. That leaves the no_std configuration -- the one the crate targets first -- covering 228 tests rather than 186 at the start of this review, and takes `clippy --no-default-features --all-targets` from 21 warnings to zero. Also correct `assert_encode_size_agrees`' doc comment: `encoded_size` is a provided method that counts by encoding into a sink, so every quantity the helper compares comes from `encode` itself and it proves nothing about *which* bytes are written. Reading its ~78 call sites as byte-correctness coverage is what made the two gaps above possible. And the `negative_response_code` doctest asserted nothing when decode succeeded; it now uses `expect_err`.
Three of these were introduced by this branch and are corrections to text I wrote. `DtcRecord::high_byte`'s doc said "ISO 14229-1 Annex D.1 uses [it] to identify the system group (powertrain, body, chassis, network)", and CHANGELOG repeated it. Annex D.1 Table D.1 defines whole 3-byte `groupOfDTC` *values*, and its powertrain/chassis/body/network rows are literally "to be determined by vehicle manufacturer" -- there is no high-byte mapping. The one byte-level assignment the table makes is the opposite one: for 0xFFFF00-0xFFFFFE the *low* byte is a FunctionalGroupIdentifier. Clause 12.3.2.3 is explicit that ISO 14229-1 specifies no decoding method for the three bytes; that is the DTCFormatIdentifier's job. `low_byte`'s "carries the failure type" was SAE J2012 semantics stated as universal. Also false, also mine: "the last `new()` in the crate that was not [const]" (`CommunicationControlRequest` has two), "All 15 request types" (16 -- RequestUpload landed two commits earlier), "the seven structs must be built through `new()`" (one had no constructor at all), Table D.1 cited for FunctionalGroupIdentifier where it is Table D.15, and two counts of enums/accessors that match nothing countable. Pre-existing and more consequential: NRC 0x78 carried 0x73's description, calling the response-pending code a BlockSequenceCounter error. It is the code that extends P2*, and Annex A.1 obliges the server to send a final response regardless of SPRMIB -- the worst single NRC to describe wrongly for an audience writing ECU-facing code. And `p2_star_server_max` documented its scaling backwards: Table 29 gives it a 10 ms resolution, so reading the stored value as milliseconds under-waits by a factor of ten. `DtcStoredDataRecordNumber::new` rejected 0xF0 citing ISO. Clause 12.3.3.2 reserves only 0x00 for this parameter; 0x00/0xF0 is the snapshot record-number space, and both the check and the doc had been copied from there -- so this is a behaviour fix, not just a wording one. `ReadDtcInfoResponse`'s InsufficientData shortfalls counted `needed` from the sub-function byte and `available` from after it, so `needed - available` was one too large at three sites. Now both are payload-relative, pinned by a test. README: `AccessTimingParameters` is footnoted as :2013-only (removed in the 2020 edition the crate claims to target), `DynamicallyDefinedDataIdentifier` is renamed to the `DynamicallyDefineDataIdentifier` of Table 23 in both the table and the enum, six escaped bracket pairs that rendered as dead `[Request]` text on the crate's own front page are real links again, the re-export list no longer promises the codec traits "eventually" when they are already there, and the stray 0.1.0 reference is gone.
**Lossy conversions made fallible.** `From<u32> for DtcRecord` masked the top byte away, so `DtcRecord::from(0x01_0203)` and `from(0xFF01_0203)` were equal. A caller holding a DTC in a `u32` from elsewhere, one byte too wide, got a wrong DTC with no signal. Now `TryFrom<u32>` with a new `Error::InvalidDtcRecord`. `RequestDownloadResponse::new` / `RequestUploadResponse::new` are fallible and `max_number_of_block_length` is private behind an accessor. The slice's length becomes a single `lengthFormatIdentifier` nibble, so it cannot exceed 15 bytes -- a check that lived in `encode`, making a 16-byte slice constructible and only then unencodable. That is the same construct-then-fail asymmetry the transfer requests had, and it was reachable through the public field as well. **Reserved bits are no longer rewritten.** `LengthFormatIdentifier` kept only the high nibble, so `74 25 08 00` re-encoded as `74 20 08 00`. ISO 14229-1 leaves that low nibble undefined, which makes it not ours to clear -- the same reasoning as `d3facab`, which stopped `TesterPresentRequest` normalizing a reserved sub-function byte. The nibble is now retained from the wire, and a response the crate builds leaves it zero. Its property test had generated `high_nibble << 4`, so the low nibble was always zero and the property held trivially while the impl silently discarded it. **Three `Error` variants that nothing could construct are gone**: `NoDataAvailable`, `InvalidFileSizeParameterLength` and `InvalidDtcFormatIdentifier`. All three were in the NRC mapping and asserted by its test, so the suite was green on states no code path could reach. `InvalidFileSizeParameterLength` had no natural home either: an over-wide declared length is already reported as `InvalidWidth` by the codec, and a test pins that. Every remaining variant is reachable. **The `0x12` classification doc block was accurate for one of the eight services it named.** Only `0x31` and `0x85` reject a reserved sub-function at decode; the other six model the whole `0x00..=0x7F` space so the byte round-trips and the server can see what it was actually sent. The block now says which do what, and says explicitly that answering `0x12` for the rest is the server's job. **`fault_detection_iter` is now `dtc_fault_detection_iter`**, matching `DtcFaultDetectionIter`. Renaming `DtcSeverityAndStatusIter` to `WwhObdDtcSeverityIter` had made exact accessor/type agreement the convention 2:1 and left this behind.
…types The pass that made construction `const` did not make the inverse `const`, so a `const` server dispatch table could be built but not read: `u8::from(nrc)` and friends are trait methods, which stable Rust will not call in a `const fn`. Four types had no other byte path, and each of these lines was an E0015 before: const NRC_BYTE: u8 = NegativeResponseCode::ConditionsNotCorrect.value(); const DFI_BYTE: u8 = DFI.value(); const FORMAT_BYTE: u8 = DtcFormatIdentifier::Iso14229_1DtcFormat.value(); const CONTROL_BYTE: u8 = CommunicationControlType::DisableRxAndTx.value(); const DTC_U32: u32 = DTC.to_u32(); Adding `CommunicationControlType::value()` also unblocks the last two non-`const` constructors in the crate: `CommunicationControlRequest::new` and `::new_with_node_id` needed the byte only for their error payload. All 47 constructors are now `const`. The existing `From` impls delegate to the new inherent methods, so there is one source of truth per type. `Request` and `Response` now derive `Eq`/`PartialEq`. Every payload type already did, so `assert_eq!` worked on a payload but not on the frame that holds it -- which is why the crate's own tests fall back to `matches!` for frames. Dropped `clap::Parser` from `UdsIdentifier`. It made a 53-variant data enum into a top-level CLI parser with a `parse()` that reads `std::env::args`; `ValueEnum` is the derive that was wanted, and the pair compiled so the mistake was silently part of the public API.
`NamePayload` carried a `mode_of_operation` that the `RequestFileTransferRequest`
variant already *is*, with nothing keeping them in step -- and the field won:
RequestFileTransferRequest::AddFile(
NamePayload::new(FileOperationMode::DeleteFile, "/a"), dfi, size)
encoded a DeleteFile request and silently dropped the data-format identifier and
both file sizes; at frame level the leftover bytes surfaced as `TrailingBytes`
from a request the caller believed well-formed. The compiler could not catch it.
The mode byte now comes from the variant, via a new
`RequestFileTransferRequest::mode_of_operation()`, so the contradictory state is
unrepresentable. `NamePayload` is just the length-prefixed name, which is also
what makes it a coherent type rather than a bag.
`DtcStoredDataRecordNumber::new` returned `Result` while `From<u8>` accepted
anything, so the invariant it advertised was not one the type held --
`new(0x00)` failed and `from(0x00).value()` succeeded on the next line. It is now
total like both sibling record-number types, with an explicit `is_reserved()`
predicate for callers who want the check, and it gains the `PartialEq<u8>` both
siblings already had. All three now have the same shape.
That leaves `Error::ReservedForLegislativeUse` with no construction site, so it
goes the way of the other three unreachable variants. `FileOperationMode` gains
a `const value()` like its peers.
`PartialEq` on these enums is variant equality, not wire equality, so being able to name a byte-carrying reserved variant directly created a trap: `DtcFormatIdentifier::IsoSaeReserved(0x01)` is not equal to `Iso14229_1DtcFormat` yet encodes the same byte, so a downstream `if resp.format_identifier == DtcFormatIdentifier::Iso14229_1DtcFormat` returned false for a value that is byte-identical on the wire. `#[non_exhaustive]` on those variants makes the aliasing state unconstructible from outside, leaving the total `From<u8>`/`try_from` classifier -- which never aliases -- as the only way in. This is the pattern the four sub-function enums (`CommunicationControlType`, `SecurityAccessType`, `ResetType`, `DiagnosticSessionType`) already used; it now covers `DtcFormatIdentifier`, `FunctionalGroupIdentifier`, `FileOperationMode` and `ReadDtcInfoSubFunction`. The byte is still readable via `value()`. Pinned from the integration test, where the attribute actually applies, by checking the property that matters across all 256 bytes: the classifier preserves the byte, and equality with a named variant agrees with the wire. Also records this round's work in the CHANGELOG under three new sections.
Only visible under `cargo doc --document-private-items`, which CI does not run.
A per-commit validity review of this branch, one reviewer per slice, checked each
change against the ISO text rather than against the crate. Four of my changes were
wrong, not merely unnecessary. Each is reverted or corrected here rather than in the
commit that introduced it, so the correction is visible in the history instead of
hidden by a rewrite.
1. The lengthFormatIdentifier's reserved low nibble is not ours to keep.
ada8674 added `reserved_low_nibble` so a decode/re-encode would be byte-exact,
on the stated premise that "ISO 14229-1 leaves that low nibble undefined". It
does not. Tables 443 and 448 both say bits 3-0 are "reserved by document, to be
set to '0'", state that "the lower nibble shall be set to '0'", and give the byte
range as 0x00 to 0xF0. So 0x25 is not a legal lengthFormatIdentifier, and the old
zeroing behaviour was the conformant one -- the change made the crate re-emit a
byte the standard forbids, and paid for it with a private field on two public
response types and a PartialEq that distinguished responses by a nibble that must
always be zero.
The property test kept its full-u8 generator, which is the useful half of that
commit: it now asserts the normalization (`byte & 0xF0`) rather than a round trip.
The old `high_nibble << 4` generator held trivially whichever way the impl behaved.
2. An invalid addressAndLengthFormatIdentifier is NRC 0x31, not 0x13.
Tables 444 and 449 both list requestOutOfRange for "the specified
addressAndLengthFormatIdentifier is not valid". The code answered
IncorrectMessageLengthOrInvalidFormat, i.e. 0x13. Worse, bb80477 deleted the
`// NRC::RequestOutOfRange if ...` comment that recorded this, while editing those
exact lines -- so the fix removed the only in-tree note of a live defect. New
Error::InvalidAddressAndLengthFormatIdentifier carries the byte and maps to 0x31.
3. Error::IoError does not map to generalReject.
Annex A.1: 0x10 "shall only be implemented in the server if none of the negative
response codes defined in this document meet the needs of the implementation. At no
means shall this NRC be a general replacement." It also appears in no per-service
NRC table, including the crate's own -- so `err.negative_response_code()` returned a
code that every `allowed_nack_codes()` rejects. ISO does not model a transport
failure as an NRC at all; clause 7.4.1.6 surfaces it as `A_Result = error` to the
server application. There is no byte to send, so the method now returns
`Option<NegativeResponseCode>` and `None` for IoError.
Its doc block also claimed to be "Following ISO 14229-1" when the mapping is a
default over the clause 8.7.5 / Annex A.1 mandatory lane; clause 8.7.2 says a
specific NRC "is not guaranteed for all possible test pattern sequences". And
`mapping_only_produces_codes_the_iso_tables_allow` asserted something both weaker
than its name and refutable: the per-service tables are a floor, not a ceiling
(clause 9.4). Renamed to what it checks.
4. CommunicationType's serde had no hole to close, and the fix opened one.
d5a6180 routed it through `serde(try_from = "u8")` along with five types that
genuinely needed it. Its four unit variants are in bijection with 0x00..=0x03 and
TryFrom<u8> accepts all four, so nothing could be smuggled. But TryFrom<u8> reads
bits 1-0 of a whole communicationType byte and masks the rest, so deserializing 17
silently yielded Normal and re-serialized as 1 -- exactly the silent normalization
the commit set out to eliminate. Back to the derived variant-name form.
Also in scope, from the same review:
- ReadDtcInfoSubFunction::try_reserved. f260dee sealed IsoSaeReserved to keep bit 7
out of it, which is the one variant seal on this branch that guards a real
invariant -- but the enum has no public TryFrom<u8>, so sealing it left a tester
unable to originate a request for any unmodeled sub-function. That is a capability
removed, not a hole closed. try_reserved rejects bit 7 and is covered from
tests/public_api.rs, where #[non_exhaustive] actually applies.
- Table 13 was cited for two different things it does not say. The Cvt M/C/S/U legend
is Table 8 ("A_PDU parameter conventions"); the SPRMIB rule that "values of both '0'
and '1' shall be supported for all SubFunction parameter values" is Table 11
("SubFunction parameter structure"). Table 13 is "SubFunction parameter conventions"
and has only M and U, about whether the server supports a sub-function at all.
Corrected in three rustdoc sites and the CHANGELOG.
Two of the three test holes the review reported were real. I checked all three by
mutation rather than taking the reports at face value, and the third does not exist.
REAL: the DTC status byte was unprotected in the config the crate targets first.
`dtc_and_status_iter_roundtrip` asserted the status byte, but it was gated on
`alloc` because it used `collect()`. Mutating `DtcAndStatusIter::next` to
`DtcStatusMask::from(0)` left `cargo test --no-default-features` at 236 passed, 0
failed. So the assertion that commit c654ce8 added specifically to kill "every DTC
reports status 0x00" only ran under `--all-features`. Rewritten with `next()`, gate
removed; the same mutant now fails in both configs. no_std count 236 -> 237.
REAL: a frame for an unmodeled service would have passed spec_conformance silently.
`Request::Other` and `Response::Other` echo the SID and payload verbatim, so they
round-trip perfectly. Both suites now assert the decode is not `Other`, which is
what makes "this service has spec-example coverage" mean anything.
NOT REAL: the ECUReset powerDownTime mutant.
Reported as surviving all 264 tests. It does not: `[] => (Some(0), rest)` fails
`an_absent_power_down_time_is_distinguishable_from_a_reported_zero` and
`a_response_without_a_power_down_time_round_trips_unchanged`, in both
--all-features and --no-default-features. No change needed.
NOT REAL: the allowed_nack_codes dispatch swap.
Swapping the CommunicationControl and ControlDtcSetting arms does leave the suite
green, but that is correct rather than a gap: ISO gives those two services the same
four codes, and Tables 444/449 give RequestDownload and RequestUpload the same six.
The dispatch is unobservable for identical tables, so there is nothing to pin. Noted
in the test instead of chased.
Also corrected three claims that were false:
- spec_conformance said ISO gives no request byte tables for 0x10, 0x11 and 0x27, so
they appear "on the response side only". Wrong: Table 31, Table 38 and Tables
47/49/51 are all request byte tables. What actually happened is that the markdown
conversion merged the byte-value column into the description cell for three of them
("ECUReset Request SID 11 16") and I read an extraction artifact as a property of the
standard -- the same hazard the file already warns about for Tables 486/487. Table 49
survived intact and is now included (27 02 C9 A9), bringing the suite to 36 frames.
Table 472 (RequestUpload) stays out: bytes #7 and #8 of its memorySize are not
legible, and inferring them is exactly the mistake being corrected here.
- The suite's module doc claimed it catches "a layout error that both encode and decode
share". It does not, and the reviewer proved it: transposing p2_server_max and
p2_star_server_max in *both* directions leaves Table 32 round-tripping and all three
tests green. Rewritten to state what it does buy -- a legal frame the crate rejects,
and an encode/decode pair that disagree -- and what it does not.
- The CHANGELOG said 34 frames; it was 35 before this commit. Fixed to 36, which is the
same miscount class the docs commit exists to fix.
And one doc defect: Request::allowed_nack_codes read as a closed set. Clause 9.4 says
the Annex A.1 codes "shall be used in addition to" the per-service tables, and A.1
keeps the generally-supported codes out of them deliberately -- including 0x78
RequestCorrectlyReceivedResponsePending, which is in none of the 16 tables and is one
of the commonest codes in real traffic. A client using this slice as a validation
whitelist would reject every ResponsePending it saw. Documented as a floor.
The review found this branch added 15 #[non_exhaustive] attributes, of which 10 guard
nothing. Two reviewers disagreed about which, so I checked each encoder rather than
picking a side -- and the split is not the one either of them proposed.
Structs: five were data bags, two were mis-diagnosed.
SizePayload, FileSizePayload, DirSizePayload, PositionPayload and
DtcFaultDetectionCounterRecord have every field pub and derive their wire widths from
the values, so no combination can fail to encode. Under the crate's own rule --
encapsulate iff invariant-bearing -- they are data bags, and the seal only cost
downstream the struct literal (E0639). It cost more than that in practice:
DtcFaultDetectionCounterRecord became unbuildable outside the crate entirely and took
two follow-up commits and a new test file to repair. Seals dropped.
NamePayload and SentDataPayload are a different case, and the reviewer who called all
seven "no invariant" was wrong about these two. Their encoders do
`u16::try_from(name.len())` and `u8::try_from(block.len())` and fail -- so an
over-long value was constructible and only rejected later, which is precisely the
construct-then-fail asymmetry RequestDownloadResponse::new was made fallible to close.
Rather than drop their seals, `new` now enforces the bound. Now the seal guards
something and the type is consistent with its twin.
Variants: the seal did not work, and the mechanism that does work replaces it.
Verified from a real downstream crate: with a plain derived Deserialize,
`{"IsoSaeReserved":1}` builds a DtcFormatIdentifier whose value() is 0x01 -- the byte
Iso14229_1DtcFormat encodes -- that compares unequal to it. That is the exact aliasing
state f260dee claimed to make unconstructible, reachable in one line. The CHANGELOG
entry saying otherwise was false.
So the seal bought no guarantee, and it was not free: both `IsoSaeReserved(b)` and
`IsoSaeReserved(..)` are E0603 downstream, meaning a caller could not even read the
byte by matching. The only spelling that compiles is `IsoSaeReserved { 0: b, .. }`,
which is syntax most Rust developers have never written, let alone C developers new to
the language. And it was applied to 3 of roughly 30 byte-carrying variants, so
downstream could not predict whether naming one would compile.
DtcFormatIdentifier, FunctionalGroupIdentifier and FileOperationMode now do what
DtcSettingType and SubnetNumber already did: route serde through the byte with
serde(from = "u8", into = "u8"), which makes every byte classify through the same
function the wire uses, and add PartialEq<u8> so wire equality is directly
expressible. Seals dropped; the variants are nameable and destructurable again.
ReadDtcInfoSubFunction::IsoSaeReserved keeps its seal. It is the one that guards a
real invariant -- bit 7 is SPRMIB and is fused in at encode -- and it now has a door
(try_reserved, added in the previous commit).
Also: TryFrom<u8> for FileOperationMode never returned Err -- every arm was Ok, because
every byte outside 0x01-0x06 is ISOSAEReserved, which the enum represents rather than
rejects. A fallible signature that cannot fail. Now From<u8>.
Tests moved to where they mean something. The guard test that policed reachable
constructors for seven sealed structs is gone; in its place, tests/public_api.rs
constructs the five data bags with struct literals, checks the two bounded ones reject
an unencodable value, proves the serde bypass is closed for all 256 bytes, and proves a
reserved variant can be destructured downstream again.
The hand-written utoipa impls this branch introduced were a net regression on the
derived ones they replaced. Three defects, all reproduced against a real generated
document before fixing and all mutation-verified after:
1. Four dangling $refs, where the derive had zero.
ToSchema::schemas() defaults to a no-op and the derive is what overrides it, so a
hand-written impl that only provides schema() emits $refs to types the document never
defines. CommunicationControlType, CommunicationType, DataFormatIdentifier and
SubnetNumber were all referenced and undefined, which every client generator either
rejects outright or degrades to an untyped object. All four hand-written composites
now forward schemas() to their repr.
2. Two descriptions were implementation notes, and one had lost its verb.
utoipa's derive reads only literal doc attributes and silently drops
`#[doc = concat!(..)]`, so RequestDownloadRequest's published description began
"\nDeserializing routes through\nso a declared width..." -- the fragment carrying the
sentence's subject was gone, along with the type's real documentation. That macro's
repr doc is now plain `///` lines only, with a comment saying why.
TesterPresentRequest's description was the repr's internal rationale, and it named
ZeroSubFunction -- the module-private type the repr exists to keep out of the schema.
These docs are published verbatim, so the rationale moved to `//` lines and the `///`
text now reads as API documentation. Same for CommunicationControlRequest and for
CommunicationType, whose description had picked up my note about why it does not use
the byte form.
3. Every byte schema advertised a range four times wider than its deserializer accepts.
Delegating to `<u8 as PartialSchema>::schema()` emits
`{"type":"integer","format":"int32","minimum":0}` -- no maximum. But SubnetNumber
rejects anything above 0x0F, CommunicationControlType and SecurityAccessType above
0x7F, and DtcSettingType accepts only 65 of the 126 bytes in its range. A generated
client emitted an i32 and found the real bound as a runtime rejection. The whole
premise of these types is that their bytes have ranges, and the schema was the one
place that did not say so.
byte_schema! now takes a max and a description per type. DtcSettingType's set is not
contiguous, so minimum/maximum cannot express it and the description spells out the
four accepted ranges and says the bounds are looser than the real constraint.
The reason all three shipped is that tests/ contained no utoipa assertion at all, so
tests/openapi_schema.rs is the substantive part of this commit. It builds a real document
and checks that no $ref dangles, that no description is empty or names a private type or
starts with the whitespace a dropped concat! fragment leaves behind, and -- the strongest
of the three -- that each byte schema's `maximum` equals the largest byte serde actually
accepts. That last one does not hard-code a bound: it deserializes all 256 bytes to
discover it, so the schema cannot drift from the deserializer without failing.
Verified by mutation: dropping `.maximum(..)` fails the bound test; restoring
ZeroSubFunction to a description fails the leak test; removing the schemas() forwarding
from CommunicationControlRequest fails the dangling-ref test, naming all three children.
Removing TesterPresentRequest's forwarding does not fail, and correctly so -- its repr
has only scalar fields, so it $refs nothing and the forwarding there is defensive.
…ibutes
SubFunctionRepr and SubFunctionOnlyRepr were 108 lines: two mirror structs, four
conversion impls, four hand-written schema impls, two `cfg(any(serde, utoipa))` gates and
two `allow(dead_code)`. They existed to keep the module-private ZeroSubFunction out of the
serialized form and the schema while still range-checking the byte on the way in.
The invariant is single-field, so a field attribute expresses it directly. ZeroSubFunction
already had the TryFrom<u8> that does the checking -- the same classifier `decode` uses --
so routing serde through it with `serde(try_from = "u8", into = "u8")`, plus
`serde(rename = "sub_function")` and `schema(rename, value_type = u8, ...)` on the field,
does the whole job. The reprs re-implemented a check that was already one line away.
The commit that added them justified hand-writing whole-type schemas with "`value_type` is
field-level only". That is true, and it is exactly why it applies here: the leak *is* a
field. tester_present.rs goes 477 -> 369 lines with nothing hand-written left in it.
Verified equivalent, not assumed. I captured the serialized output, the rejection
behaviour and both schemas before the change and diffed after:
- serialization is byte-identical: `{"suppress_positive_response":false,"sub_function":0}`
and `{"sub_function":0}`, and a reserved 0x42 still round-trips unchanged;
- the rejection message is the same, now with serde_json's line/column appended, because
the error surfaces from a field rather than the container;
- the schemas are strictly better. They gained `maximum: 127`, which the reprs never
advertised despite existing to describe the range; the top-level description is now the
type's own doc instead of the repr's; and the fields have descriptions, which the reprs'
bare `u8` fields did not.
Two things the change exposed:
- Field descriptions are published too, and my first version of the field doc put the
implementation rationale in `///`, so it landed in the schema. Moved to `//`, and
tests/openapi_schema.rs now checks field descriptions for private type names as well as
top-level ones -- it had only covered the latter, which is why nothing caught it.
- The range check is now pinned from outside the crate. tests/public_api.rs asserts every
byte 0x80..=0xFF is rejected for both types, and every byte 0x00..=0x7F round-trips
unchanged, so the guarantee the reprs provided is tested rather than inferred from the
mechanism being present.
… set
A `ToSchema` in this crate describes the *serde* representation, not the Rust shape.
Several types serialize as a single protocol byte -- a `DataFormatIdentifier` is `33`, not
`{"compression_method":2,"encryption_method":1}` -- and their schemas are hand-written to
match. So a `utoipa`-without-`serde` build published a schema for a wire format that build
could not produce.
Supporting that combination also cost real complexity. Every type reachable only through a
serde repr needed `#[cfg(any(feature = "serde", feature = "utoipa"))]` rather than a single
predicate, plus `#[cfg_attr(not(feature = "serde"), allow(dead_code))]` and two lines of
comment explaining why fields that are never read still have to exist. That is four
attributes and a paragraph of justification apiece, to keep compiling a configuration
nobody wants. Both are gone; `cfg(feature = "serde")` now says the whole thing.
The feature powerset is 16 combinations rather than 20 as a result, all clean, and all four
`thumbv6m-none-eabi` builds still pass -- `serde` remains the one optional integration
usable on bare metal, which this does not change.
Also adds the feature table the README never had. The crate has five features and
documented none of them, which for a `no_std` crate is the first thing a reader needs; a
new implication between two of them with nowhere to record it is what made this the moment
to fix that. Both implications are called out with their reasons, since neither is
guessable: `utoipa` implying `serde` for the reason above, and `utoipa`/`clap` implying
`std` because their derive macros expand to `std::`, `String` and `Vec` paths that cannot
compile under `#![no_std]`.
The `DataFormatIdentifier` example in that table is checked, not assumed --
`serde_json::to_string(&DataFormatIdentifier::new(2, 1)?)` is `33`.
…ose widths
`new_with_widths(dfi, address, address_len, size, size_len)` had five positional
parameters, four of them numeric, containing two transposable value/width pairs. Nothing
could catch a swap: all four are integers, and the ALFID nibbles are asymmetric (high is
memorySize, low is memoryAddress), so transposing the two widths silently truncated a value
on the wire and produced the one parameter a bootloader answers requestOutOfRange for.
It also duplicated `new`. They were independent code paths, and they disagreed -- verified
before changing anything:
new(NONE, 0x1_0000_0000_0000, 0x10) -> InvalidMemoryAddress (NRC 0x31)
new_with_widths(NONE, 0x1_0000_0000_0000, 5, ..) -> IncorrectMessageLength... (NRC 0x13)
One input, one rejection, two different bytes on the wire depending on which constructor a
caller happened to reach for. No test covered it.
`AddressAndLengthFormatIdentifier` is now public and carries the widths as a unit:
- named for the ISO parameter it is, rather than the invented `MemoryFormatIdentifier`, which
is consistent with `DataFormatIdentifier` and `DtcFormatIdentifier` already following ISO;
- private nibbles behind `new(memory_size_length, memory_address_length)` and accessors, so a
value outside Table H.1 cannot exist -- the widths were `pub` fields on a `pub(crate)`
struct before, which is why the request had to re-check them;
- built from a byte with `try_from(0x44)`, which is how the requirement actually reaches a
caller ("this bootloader needs ALFID 0x44") and cannot be transposed;
- `PartialEq<u8>` so `alfid == 0x44` reads the way the spec does.
`new_with_alfid(dfi, alfid, address, size)` replaces the five-arg version, and `new` derives
the minimal widths and **delegates** to it, so exactly one function decides whether a
(value, width) pair is acceptable. New `Error::InvalidMemorySize` completes the pair with
`InvalidMemoryAddress`; both map to NRC 0x31, which is what Tables 444 and 449 require for a
memoryAddress/memorySize that "is not valid", and they name which parameter was at fault
rather than collapsing into one length error.
Pinned by `both_constructors_reject_an_over_wide_address_the_same_way`, which is the test
that did not exist. Mutation-verified: putting the old
`IncorrectMessageLengthOrInvalidFormat` back on the address path fails four tests across
both services.
The serde shape changes with it. The widths were two derived scalars, flattened by the repr
because the type behind them was `pub(crate)`; they are now the single
`address_and_length_format_identifier` byte, so the JSON matches Table 441 instead of
paraphrasing it, and the width validation happens in that type's own `try_from` rather than
being re-implemented in the repr's conversion. Its schema carries the real bounds and notes
that the accepted set is not contiguous -- 0x15, 0x25, 0x35 and 0x40 to 0x43 sit inside
0x11..=0x45 and are rejected -- which tests/openapi_schema.rs now enforces along with the
rest.
Making a constructor fallible does nothing while the field it guards is `pub`.
`WriteDataByIdentifierRequest::new` rejected an empty data record, and then:
let mut req = WriteDataByIdentifierRequest::new(0xF190, &[0x01])?;
req.data = &[]; // walks straight around the check
req.encode_to_slice(&mut buf) // Ok([0xF1, 0x90])
That is a 3-byte frame this crate's own decoder answers NRC 0x13 to -- ISO 14229-1:2020
Table 277 marks dataRecord byte #1 mandatory and Figure 26 states a four-byte minimum. So
the crate could be made to emit traffic it refuses to read, and the `Result` was theatre.
`#[non_exhaustive]` does not help: it blocks a struct literal, not assignment to a field of
a value you already have.
Two of the three were my own, added two commits ago when I made `NamePayload::new` and
`SentDataPayload::new` fallible to justify keeping their seals -- and left both fields `pub`,
which is the same mistake in the same round. All three fields are now private with
accessors, which is what the crate's rule (encapsulate iff invariant-bearing) already said
to do.
The serde half is closed with field-level `deserialize_with` hooks in a new
`shared::bounded`, not with mirror structs: each invariant is a property of one field, so
there is nothing for an aggregate to coordinate. That is the lesson from the TesterPresent
reprs, which spent ~100 lines on a single-field invariant.
One honest limitation, pinned by a test rather than papered over. `&'de [u8]` needs a format
with a native byte-string type, and JSON has none -- serde_json fails with "invalid type:
sequence, expected a borrowed byte array" before any length check runs. My first version of
this test asserted that an empty `"data":[]` was rejected and passed, which looked like the
guard working; it was rejecting every array, empty or not. So the byte-slice guards are only
exercised by a format that can borrow bytes (CBOR, MessagePack, bincode). They are still the
right place for the check, since the crate does not choose the caller's format, but the test
now says so explicitly and will fail if JSON ever starts accepting these.
The `&str` guard *is* reachable through JSON, because JSON strings borrow. That one is tested
for real -- a name one byte past u16::MAX is refused, u16::MAX exactly is accepted -- and
mutation-verified by disabling the bound.
…ssigns it
The shared macro emitted one un-parameterised doc for both services, written for download.
For `RequestUploadResponse` it was wrong on three counts, all verified verbatim against the
standard:
| download (Table 443) | upload (Table 448)
constrains | TransferData *requests* | TransferData *responses*
reports the server's | *receive* buffer | *send* buffer
must accept full len | the *server* | the *client*
pad bytes forbidden | yes, explicitly | clause absent
Table 443: "to include in each TransferData request message from the client ... allows the
client to adapt to the receive buffer size of the server ... A server is required to accept
transferData requests that are equal in length to its reported maxNumberOfBlockLength".
Table 448: "shall be included in each TransferData positive response message from the server
... allows the client to adapt to the send buffer size of the server ... A client is
required to accept transferData responses that are equal in length to the reported
maxNumberOfBlockLength".
So the old text pointed an upload user at the wrong message type, the wrong buffer, and the
wrong party, and carried a pad-byte prohibition that Table 448 does not state -- reasonably,
since nothing on the upload path is written to server memory. `$verb` had been threaded
through the request side of this macro; the response side got nothing.
Both now come from a `block_length_accessor_doc` parameter, and the upload one names the
download accessor explicitly so the difference is visible from the place someone would
confuse them.
One thing this exposed: my first version put the corrected prose on the **private** field,
where rustdoc does not publish it. The correction was invisible to exactly the audience it
was for. Checking the generated HTML rather than the source is what caught it -- the field
doc is now a one-liner pointing at the accessor, and the accessor carries the semantics.
The `docs: correct claims that were not true` commit on this branch renames `DynamicallyDefinedDataIdentifier` to `DynamicallyDefineDataIdentifier`, which is what ISO 14229-1:2020 Table 23 calls service 0x2C -- "Define", not "Defined". The SID conversion table added in #51 spells it the old way, so it stops compiling here. Worth noting that this is the table doing its job: it is written out independently of the conversions precisely so that a change to the enum has to be acknowledged rather than silently agreed with. Kept as its own commit rather than folded into the renaming commit, so the dependency between the two is visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README.md is this crate's front page *and* its crate-level documentation, via
`#![doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/README.md"))]`. So
`[`Request`]` in it is a rustdoc intra-doc link, not prose.
mdformat escapes `[` and `]`, which turns every one of those into literal text.
Measured on the generated HTML rather than assumed -- links from the crate index:
unescaped after mdformat
Request 3 1
Response 3 1
UdsServiceType 2 1
The hook has been configured in this repo for a while but was never enforced;
the reusable CI workflow adopted in #48 runs pre-commit, which is what surfaced
it. It first showed up as a plain formatting failure on this branch, because the
documentation commit here deliberately unescaped six bracket pairs to make those
links work -- so the hook and the commit want opposite things.
Excluding one file rather than reverting the unescaping: the links are worth more
than uniform prose formatting, and every other markdown file in the repo is still
formatted. The alternative -- writing every link with an explicit
`[`Error`](crate::Error)` target, which mdformat leaves alone -- is what an earlier
commit on this branch removed as redundant.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
zheylmun
force-pushed
the
fix/wire-conformance
branch
from
July 31, 2026 14:00
9d4cc95 to
f8bde8c
Compare
zheylmun
force-pushed
the
refactor/encapsulation-and-serde
branch
from
July 31, 2026 14:00
33c7459 to
af68b7f
Compare
2 tasks
JustinKovacich
approved these changes
Aug 10, 2026
JustinKovacich
left a comment
JustinKovacich
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the incremental diff over #53 (opus review pass). Encapsulation is sound — every privatized field has an accessor, nothing needed is removed (NamePayload::mode_of_operation intentionally dropped in favor of RequestFileTransferRequest::mode_of_operation()), and dropping #[non_exhaustive] fixes downstream constructibility. Serde/OpenAPI validation routes through the same TryFrom the wire decoder uses, so no valid frame can be rejected; 4 dangling schema $refs fixed. Spec-gap fixes f (DtcStoredDataRecordNumber total) and h (public AddressAndLengthFormatIdentifier + new_with_alfid) verified correct. Tests strengthened and vacuous ones removed; #53's spec_conformance oracle intact and extended. No code blockers. One documentation nit (a stale LengthFormatIdentifier CHANGELOG entry describing the reverted-away behavior) will be fixed in the changelog-finalize step before tagging. Integration to main is via a single merge of the stack tip per the agreed strategy.
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.
Last of the follow-on branches splitting
api/consistency-pass-1. Stacked on #53, which isstacked on #51 — review those first; this PR's diff is only its own 23 commits.
With this merged,
api/consistency-pass-1is fully represented and can be deleted.This branch is the contested one. It contains the encapsulation and
serde/OpenAPIwork andthe audit that found much of that work to be wrong, so several commits here undo earlier commits
here. That is deliberate: the alternative was to fold the corrections into what they corrected,
which hides the fact that the first attempt shipped a defect.
Commit Message Details
The audit corrections, and what each undoes
Four changes were not merely unnecessary but wrong, and are reverted or corrected in
fix!: undo four changes the churn audit found wrong:LengthFormatIdentifier::reserved_low_nibble0x00-0xF0. The old zeroing behaviour was the conformant one; the change made the crate re-emit a byte the standard forbids0x13requestOutOfRange(0x31). Worse, the ALFID commit had deleted the comment recording this while editing those exact linesError::IoError→generalReject0x10"shall only be implemented … if none of the negative response codes defined in this document meet the needs" and "at no means shall this NRC be a general replacement". It appears in no per-service table, sonegative_response_code()returned a code everyallowed_nack_codes()rejects. ISO surfaces transport failure asA_Result = error(7.4.1.6), so the method now returnsOptionCommunicationType'sserde(try_from = "u8")0x00..=0x03, so nothing could be smuggled — butTryFrom<u8>masks the upper bits, so deserializing17silently yieldedNormaland re-serialized as1. The fix introduced the exact silent normalisation it set out to eliminateAlso:
Table 13was cited for two different things it does not say. TheCvtM/C/S/U legend isTable 8; the SPRMIB "both '0' and '1' shall be supported" rule is Table 11. Corrected in
three rustdoc sites and the changelog.
Sealing: 15 attributes added, 10 guarding nothing
seal only what bears an invariantsplits them by checking each encoder, and the split is not theone either reviewer proposed:
SizePayload,FileSizePayload,DirSizePayload,PositionPayloadandDtcFaultDetectionCounterRecordhave all-pubfields and derive their wirewidths from the values, so no combination can fail. Under the crate's own rule they are data bags.
The seal cost a downstream the struct literal (
E0639) and bought nothing — and madeDtcFaultDetectionCounterRecordunbuildable outside the crate entirely.NamePayloadandSentDataPayloadencoders dou16::try_from(len)/u8::try_from(len)and fail, so an over-long value was constructible andonly rejected later.
newnow enforces the bound.serde_json::from_str::<DtcFormatIdentifier>(r#"{"IsoSaeReserved":1}"#)builds the aliasing statethe seal claimed to prevent. Sealing blocked the Rust literal and did nothing about
serde, whilemaking both
IsoSaeReserved(b)andIsoSaeReserved(..)E0603downstream — so a caller could noteven read the byte by matching. Replaced with
serde(from = "u8", into = "u8")plusPartialEq<u8>, which closes it for all 256 bytes and restores destructuring.ReadDtcInfoSubFunction::IsoSaeReservedguards a real invariant(bit 7 is SPRMIB, fused at encode) but had no public
TryFrom<u8>, so sealing it left a testerunable to request any unmodeled sub-function.
try_reservedrejects bit 7.serde/OpenAPI: the mechanism shrankTesterPresentreprs are gone. 108 lines — two mirror structs, four conversions, fourschema impls, two
cfg(any(..))gates, twoallow(dead_code)— replaced by three fieldattributes. The invariant is single-field, and
ZeroSubFunctionalready had theTryFrom<u8>that does the checking. Verified equivalent: serialization is byte-identical, and the schema gained
maximum: 127, which the reprs never advertised despite existing to describe that range.$refs where the derive it replaced hadzero, because
ToSchema::schemas()defaults to a no-op and only the derive overrides it. Twodescriptions were implementation notes — one naming the private
ZeroSubFunction— and one hadlost its verb, because utoipa silently drops
#[doc = concat!(..)]. Every byte schema advertised0..2³¹while its deserializer rejected above0x7F, or0x0F.utoipanow impliesserde, because aToSchemahere describes the serde representation.That removed the last
cfg(any(serde, utoipa))gates and bothallow(dead_code).API shape
new_with_widths(dfi, addr, addr_len, size, size_len)→new_with_alfid(dfi, alfid, addr, size),with
AddressAndLengthFormatIdentifierpublic under its ISO name and its nibbles private. The twoconstructors were independent paths that disagreed: the same over-wide address produced
InvalidMemoryAddressfrom one andIncorrectMessageLengthOrInvalidFormatfrom the other — NRC0x31versus0x13for one input, untested.newnow derives its widths and delegates.Three
pubfields past a fallible constructor were closed:WriteDataByIdentifierRequest::datalet
req.data = &[]walk around the check and encode a frame the crate's own decoder rejects.Issue URL
No linked issue — final split of the
api/consistency-pass-1work. The No Issue labelpr-lint.ymloffers still does not exist in this repo (noted on #48, #51 and #53).Testing
--all-features(251 lib + 18public_api+ 3openapi_schema+ 3spec_conformance+ 6 doctests) and 258--no-default-features, all passing on this branch inits own right.
cargo clippy --all-targets -D warnings -D clippy::pedanticis 0 across all three configs.This branch completes that cleanup:
mainreports 22 under--no-default-features, fix!: align the wire format with ISO 14229-1:2020 #53 brought itto 17, and this brings it to 0.
cargo fmt --all --check,cargo doc --no-deps --all-featuresandcargo hack check --feature-powerset(16/16) clean. All fourthumbv6m-none-eabicombinationsbuild.
api/consistency-pass-1, the fully-verified source branch, and the only differences are the CIfiles from
main, two markdown table re-alignments frommdformat, and the SID conversion testsadded in refactor!: apply the C-CASE naming convention and ISO service spellings #51. Nothing was lost or altered in the split — which matters here, because this branch
was assembled from a contiguous cherry-pick range plus two commits lifted out of upstream order.
RequestTransferExit's serdederives that I had removed in fix!: align the wire format with ISO 14229-1:2020 #53 (where its commit is absent) and had not restored here (where it
is present), and a stale variant name in the SID test table after this branch renames
DynamicallyDefinedDataIdentifierto ISO'sDynamicallyDefineDataIdentifier.rereredisabled throughout. Left enabled, it silently auto-staged staleresolutions from earlier history rewrites and dropped two non-empty commits while leaving the tree
and the whole suite green.