Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 43 additions & 35 deletions src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,17 +378,19 @@ fn decode_register_address(payload: &[u8]) -> Option<u16> {
/// concern. The result always satisfies [`is_discovery_request`]. Never
/// panics for any input.
///
/// The header fields this builds match TC18 §12.6.1 Table 16 "Discovery
/// request" (TC18.txt lines 2370-2381): `acf_msg_type` = `ACF_ABB`,
/// `byte_bus_id` = `00000000000b`, `evt` = `0000b`, and a read-direction
/// `op` — §12.6.1's own opening sentence (TC18.txt line 2365) is "A
/// discovery request is a read request", and TC18 encodes a read as
/// `op = 0` (the request-header tables' "if op = 0 this is read_size, else
/// segment_num", TC18.txt line 1169, and §12.9's "op=0 (read request)",
/// TC18.txt line 3207). Table 16's own `op` row reads "1b (read request)",
/// which contradicts both and is treated here as a specification typo.
/// The header fields this builds match TC18 §12.6.1 Table 18 "Discovery
/// request" (TC18.txt lines 2746-2757; table renumbered from Table 16
/// since this note was written — see issue #164): `acf_msg_type` =
/// `ACF_ABB`, `byte_bus_id` = `00000000000b`, `evt` = `0000b`, and a
/// read-direction `op` — §12.6.1's own opening sentence (TC18.txt line
/// 2741) is "A discovery request is a read request", and TC18 encodes a
/// read as `op = 0` (the request-header tables' "if op = 0 this is
/// read_size, else segment_num", TC18.txt line 1235, and §12.9's "op=0
/// (read request)", TC18.txt line 3604). Table 18's own `op` row reads
/// "1b (read request)", which contradicts both and is treated here as a
/// specification typo.
///
/// One Table 16 row this deliberately does **not** match:
/// One Table 18 row this deliberately does **not** match:
/// `byte_msg_payload` is specified as "none", whereas this builder emits
/// the [`DISCOVERY_REGISTER_ADDRESS_LEN`]-byte register-address prefix this
/// module's Provenance note describes. See requirement `REQ-DISC-027` in
Expand Down Expand Up @@ -485,19 +487,21 @@ pub fn is_discovery_configure_request(msg: &AcfAbbMessage) -> bool {
/// performs no register I/O either — see that module's own doc comment for
/// why. Never panics for any input.
///
/// The header fields this builds match TC18 §12.6.2 Table 17 "Discovery
/// response" (TC18.txt lines 2414-2428): `acf_msg_type` = `ACF_ABB`,
/// `byte_bus_id` = `00000000000b` (here by echo-back of the request's own
/// EP0 `byte_bus_id`), `evt` = `0000b`, a read-direction `op` (see
/// [`build_discovery_request`] for why a read is `op = 0` despite Table
/// 17's "1b (read request)" row), and a `byte_msg_payload` that "contains
/// RC Server's register map content starting from address 0x00000".
/// The header fields this builds match TC18 §12.6.2 Table 19 "Discovery
/// response" (TC18.txt lines 2790-2804; table renumbered from Table 17
/// since this note was written — see issue #164): `acf_msg_type` =
/// `ACF_ABB`, `byte_bus_id` = `00000000000b` (here by echo-back of the
/// request's own EP0 `byte_bus_id`), `evt` = `0000b`, a read-direction
/// `op` (see [`build_discovery_request`] for why a read is `op = 0`
/// despite Table 19's "1b (read request)" row), and a `byte_msg_payload`
/// that "contains RC Server's register map content starting from address
/// 0x00000".
///
/// Two Table 17 rows this deliberately does **not** implement: the
/// Two Table 19 rows this deliberately does **not** implement: the
/// response length is *not* clamped to the discovery request's `read_size`
/// (`acf_msg_length ≤ read_size of discovery request`, TC18.txt line
/// 2422), and this function builds no `stream_id` at all, so Table 17's
/// `stream_id = localMAC + unique_id 0x0000` row is a transport-level
/// (`acf_msg_length ≤ read_size of discovery request`, TC18.txt lines
/// 2798-2799), and this function builds no `stream_id` at all, so Table
/// 19's `stream_id = localMAC + unique_id 0x0000` row is a transport-level
/// concern here. See requirements `REQ-DISC-029`/`REQ-DISC-030` in
/// `.fusa-reqs.json`, which record both gaps.
//fusa:req REQ-DISC-004
Expand Down Expand Up @@ -1028,25 +1032,27 @@ mod tests {
assert!(is_discovery_request(&request));
}

// ── TC18 §12.6.1 Table 16 / §12.6.2 Table 17 field conformance ──────────
// ── TC18 §12.6.1 Table 18 / §12.6.2 Table 19 field conformance ──────────

/// TC18 §12.6.1 Table 16 "Discovery request" (TC18.txt lines
/// 2370-2381), asserted against the *encoded* frame rather than the
/// in-memory struct, so the bit positions are exercised too. Expected
/// values are written out as literals taken from Table 16:
/// TC18 §12.6.1 Table 18 "Discovery request" (TC18.txt lines
/// 2746-2757; table renumbered from Table 16 since this note was
/// written — see issue #164), asserted against the *encoded* frame
/// rather than the in-memory struct, so the bit positions are
/// exercised too. Expected values are written out as literals taken
/// from Table 18:
///
/// | Table 16 field | Required value |
/// | Table 18 field | Required value |
/// |----------------|----------------|
/// | `acf_msg_type` | `ACF_ABB` (`0x0E`) |
/// | `Byte_bus_id` | `00000000000b` |
/// | `evt` | `0000b` |
/// | `op` | read request |
///
/// `op` is asserted as `0`: §12.6.1's opening sentence (TC18.txt line
/// 2365) says "A discovery request is a read request", and TC18 encodes
/// 2741) says "A discovery request is a read request", and TC18 encodes
/// a read as `op = 0` ("if op = 0 this is read_size, else segment_num",
/// TC18.txt line 1169; "op=0 (read request)", TC18.txt line 3207).
/// Table 16's own "1b (read request)" row contradicts both.
/// TC18.txt line 1235; "op=0 (read request)", TC18.txt line 3604).
/// Table 18's own "1b (read request)" row contradicts both.
#[test]
//fusa:test REQ-DISC-025
fn tc18_table_16_discovery_request_header_fields_match_the_required_values() {
Expand All @@ -1065,11 +1071,13 @@ mod tests {
assert_eq!(frame[5], 0x42);
}

/// TC18 §12.6.2 Table 17 "Discovery response" (TC18.txt lines
/// 2414-2428), asserted against the encoded frame with literal expected
/// values taken from Table 17: `acf_msg_type` = `ACF_ABB` (`0x0E`),
/// `Byte_bus_id` = `00000000000b`, `evt` = `0000b`, `op` = read (see
/// the Table 16 test above for the op-bit polarity), and
/// TC18 §12.6.2 Table 19 "Discovery response" (TC18.txt lines
/// 2790-2804; table renumbered from Table 17 since this note was
/// written — see issue #164), asserted against the encoded frame with
/// literal expected values taken from Table 19: `acf_msg_type` =
/// `ACF_ABB` (`0x0E`), `Byte_bus_id` = `00000000000b`, `evt` = `0000b`,
/// `op` = read (see the Table 18 test above for the op-bit polarity),
/// and
/// `byte_msg_payload` = "RC Server's register map content starting from
/// address 0x00000".
#[test]
Expand Down
18 changes: 10 additions & 8 deletions src/mdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,31 +126,33 @@
//! ## Divergence note: `mdio_mode` does **not** select Clause 22 vs Clause 45
//!
//! **This module's [`MdioAddressingMode`] contradicts TC18 and must not be
//! relied on for wire conformance.** TC18 §13.7.13.3 Table 57 "Usage of ABB
//! message for mdio requests" (TC18.txt line 5676) defines `mdio_mode` as an
//! MMD-vs-MMS access-kind and access-width selector, not an IEEE 802.3
//! clause selector:
//! relied on for wire conformance.** TC18 §13.7.13.3 Table 60 "Usage of ABB
//! message for mdio requests" (TC18.txt line 6088; corrected from the
//! stale "Table 57"/"line 5676" citation this note previously carried — see
//! the "Editorial note: pre-existing §13.7.13 citation drift" section below
//! and issue #164) defines `mdio_mode` as an MMD-vs-MMS access-kind and
//! access-width selector, not an IEEE 802.3 clause selector:
//!
//! | `mdio_mode` | meaning (TC18 Table 57) |
//! | `mdio_mode` | meaning (TC18 Table 60) |
//! |-------------|-------------------------|
//! | `01b` | MMD, single word access |
//! | `01b` *(as printed — see below)* | MMD, multiple byte access |
//! | `10b` | MMS, single word access |
//! | `11b` | MMS, multiple (double) word access |
//!
//! Table 57 as printed lists `01b` twice and never lists `00b`, so one of
//! Table 60 as printed lists `01b` twice and never lists `00b`, so one of
//! the two MMD rows is a spec typo whose intended code point (`00b` for one
//! of them) this crate cannot resolve from the text alone. Either way, the
//! `Clause22 = 0` / `Clause45 = 1` / `Spare2` / `Spare3` mapping below is
//! **wrong** against Table 57: TC18 assigns no `mdio_mode` value to a
//! **wrong** against Table 60: TC18 assigns no `mdio_mode` value to a
//! Clause-22-vs-Clause-45 choice at all, and it leaves at most one code
//! point unallocated rather than two. Correcting this is a behavior change
//! deliberately not made in the requirements-completeness pass that
//! discovered it; the accompanying requirement entry records the divergence
//! as not-implemented, and the surrounding provenance note is retained below
//! only as the historical record of how the wrong mapping arose (it was
//! derived from `ROADMAP.md`'s restatement, never from TC18 itself).
//! Table 57 also fixes the payload widths this module does not model:
//! Table 60 also fixes the payload widths this module does not model:
//! `mdio_address` "as per IEEE & OA SPI spec", and `mdio_payload` data
//! fields of 16 bits for MMD, 32 bits for MMS0 and MMS1, and 16 bits for
//! every other MMS.
Expand Down
62 changes: 36 additions & 26 deletions src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5307,9 +5307,9 @@ mod tests {
//fusa:test REQ-CMP-009
fn request_kind_discriminants_match_tc18_table_5_condition_type_bytes() {
// TC18 §11.2.2, Table 5 "Different types of conditional requests"
// (TC18.txt line 1186): "The first byte in the message_timestamp
// field is used to indicate the type of condition." Table 5's own
// rows, transcribed as literals:
// (TC18.txt lines 1256-1257): "The first byte in the
// message_timestamp field is used to indicate the type of
// condition." Table 5's own rows, transcribed as literals:
// 0x0F, 0x8F -> Compound
// 0x0B, 0x8B -> Compound wait
// 0x0E, 0x8E -> Triggered
Expand All @@ -5324,14 +5324,16 @@ mod tests {
assert_eq!(RequestKind::Chained.to_u8(), 0x01);
assert_eq!(RequestKind::Timed.to_u8(), 0x0A);

// TC18 §11.2.3.1 Table 11 (line 1679) "request_type 0x05",
// §11.2.3.2 Table 12 (line 1733) "request_type 0x06", and
// §11.2.3.3 Table 13 (line 1792) "request_type 0x07".
// TC18 §11.2.3.1 Table 13 (line 1985) "request_type 0x05",
// §11.2.3.2 Table 14 (line 2056) "request_type 0x06", and
// §11.2.3.3 Table 15 (line 2142) "request_type 0x07". (Tables
// renumbered from 11/12/13 since this note was written — see
// issue #164.)
assert_eq!(RequestKind::ClearAll.to_u8(), 0x05);
assert_eq!(RequestKind::ClearNonSafestate.to_u8(), 0x06);
assert_eq!(RequestKind::ClearSingle.to_u8(), 0x07);

// TC18 §11.2.2 (line 1186): "If the MSB of the identifier (0x8x) is
// TC18 §11.2.2 (line 1258): "If the MSB of the identifier (0x8x) is
// set the request is treated as a safety request".
for (safety, base) in [
(RequestKind::SafetyCompound, RequestKind::Compound),
Expand Down Expand Up @@ -5371,7 +5373,7 @@ mod tests {
#[test]
//fusa:test REQ-SEQ-005
fn sequencer_bank_and_state_stay_within_tc18_256_sequencer_and_state_ceiling() {
// TC18 §12.10 "Sequencers" (TC18.txt line 3463): "The number of
// TC18 §12.10 "Sequencers" (TC18.txt lines 3870-3871): "The number of
// sequencers and states per sequencer are limited to 256 by this
// definition. An RC Server implementation may support only a lower
// number of sequencers."
Expand Down Expand Up @@ -5405,13 +5407,14 @@ mod tests {
#[test]
//fusa:test REQ-ERRH-001
fn request_module_error_outcomes_carry_their_tc18_table_27_wire_codes() {
// TC18 §12.9.6 Table 27 "Error codes in responses" (TC18.txt line
// 3413), transcribed as literals: SEQUENCER_NOT_KNOWN = 2,
// REQUEST_CANCELED = 5, REQUEST_REJECTED = 11,
// INVALID_PARAMETER = 15, CHAIN_ABORTED = 16.

// TC18 §11.2.2.1 (line 1203): a compound request naming a sequencer
// the RC Server does not have.
// TC18 §12.9.6 Table 30 "Error codes in responses" (TC18.txt lines
// 3812-3844; table renumbered from Table 27 since this note was
// written — see issue #164), transcribed as literals:
// SEQUENCER_NOT_KNOWN = 2, REQUEST_CANCELED = 5,
// REQUEST_REJECTED = 11, INVALID_PARAMETER = 15, CHAIN_ABORTED = 16.

// TC18 §11.2.2.2 (TC18.txt lines 1384-1385): a compound request
// naming a sequencer the RC Server does not have.
let unknown_sequencer = CompoundGateConfig {
sequencer_num: 4,
start_state: SequencerState(1),
Expand All @@ -5423,8 +5426,9 @@ mod tests {
Some(2)
);

// TC18 §11.2.2.1 (line 1203): the sequencer is known but is not in
// the request's cmp_start_state, so the request is not due.
// TC18 §11.2.2.2 (TC18.txt lines 1384-1385): the sequencer is known
// but is not in the request's cmp_start_state, so the request is
// not due.
let unmet_gate = CompoundGateConfig {
sequencer_num: 0,
start_state: SequencerState(3),
Expand All @@ -5436,8 +5440,9 @@ mod tests {
Some(11)
);

// TC18 §11.2.3 (line 1672): "Each request that is cancelled will send
// an error response with the error code = REQUEST_CANCELED."
// TC18 §11.2.3 (TC18.txt lines 1979-1980): "Each request that is
// cancelled will send an error response with the error code =
// REQUEST_CANCELED."
assert_eq!(
check_clear_all_cancellation().unwrap_err().tc18_wire_code(),
Some(5)
Expand All @@ -5454,27 +5459,32 @@ mod tests {
.tc18_wire_code(),
Some(5)
);
// TC18 §11.2.2.1 (line 1203) / §11.2.2.4 Table 9 (line 1586): the
// watchdog-overflow purge of non-safety-tagged requests is likewise a
// cancellation.
// TC18 §11.2.2.2 (TC18.txt lines 1384-1385) / §11.2.2.6 Table 11
// (TC18.txt line 1847): the watchdog-overflow purge of
// non-safety-tagged requests is likewise a cancellation.
assert_eq!(
check_watchdog_overflow_purge(RequestKind::Compound, true)
.unwrap_err()
.tc18_wire_code(),
Some(5)
);

// TC18 §11.2.2.4 Table 9 (line 1586): cs = 1 and "error occurred in
// one of the preceding requests" -> CHAIN_ABORTED.
// TC18 §12.9.6 Table 30 (TC18.txt lines 3839-3840): cs = 1 and
// "error occurred in one of the preceding requests" -> CHAIN_ABORTED.
// (Not Table 9/§11.2.2.4 as previously cited — that table's own
// `cs` field is unrelated; this exact phrase is the Table 30
// CHAIN_ABORTED row's own description text. See §11.2.2.6 Table 11,
// TC18.txt lines 1833-1838, for the `cs`-bit definition itself.)
assert_eq!(
check_chain_continuation(true, true)
.unwrap_err()
.tc18_wire_code(),
Some(16)
);

// TC18 §11.2.2 Table 5 (line 1186) names no 0x02 condition type, so
// decoding one is a parameter out of range -> INVALID_PARAMETER.
// TC18 §11.2.2 Table 5 (TC18.txt lines 1262-1266) names no 0x02
// condition type, so decoding one is a parameter out of range ->
// INVALID_PARAMETER.
assert_eq!(
RequestKind::from_u8(0x02).unwrap_err().tc18_wire_code(),
Some(15)
Expand Down
Loading