Define the line rate once, in wiredsensor-core
BAUD_RATE was declared separately in firmware/src/board.rs and bridge/src/main.rs with a comment asking that they be kept in agreement by hand. A baud mismatch between two ends of an RS485 segment is silent and presents as random CRC failures, which is a poor thing to debug and a poor thing to leave to a comment. It now lives in wiredsensor_core::timing alongside the arithmetic that derives from it, because the line rate is a property of the segment rather than of any one board. board.rs re-exports it, and the bridge takes a dependency on core purely to read it — it still knows nothing about the protocol itself. INTER_FRAME_GAP_US and CHAR_TIME_US move with it and are now derived at the definition site, with a test asserting they cannot go stale if the rate changes. Also fills two documentation gaps: the bridge and PC test suite were committed without any README coverage, and the account of the RTIM pitfall still described the incomplete understanding held before the hardware test found the real mechanism. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Oliver Walter
Generated
+1
@@ -1214,6 +1214,7 @@ dependencies = [
|
||||
"rp2040-hal",
|
||||
"usb-device",
|
||||
"usbd-serial",
|
||||
"wiredsensor-core",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -20,12 +20,15 @@ Written in Rust on **RTIC 2** + `rp2040-hal`.
|
||||
|
||||
| Path | Contents |
|
||||
|------------|--------------------------------------------------------------------------|
|
||||
| `core/` | `wiredsensor-core` — framing, CRCs, dispatch, SHT3x math. Pure, no I/O. |
|
||||
| `core/` | `wiredsensor-core` — framing, CRCs, dispatch, SHT3x math, line rate. Pure, no I/O. |
|
||||
| `firmware/`| `wiredsensor-fw` — the RTIC application, drivers and register access. |
|
||||
| `bridge/` | `wiredsensor-bridge` — a spare RP2040 as a USB-to-RS485 bridge, for testing. |
|
||||
| `tools/` | `wiredsensor.py` — PC-side master and end-to-end test suite. |
|
||||
|
||||
The split exists so the entire protocol can be exercised by ordinary host unit
|
||||
tests (`cargo test`) with no hardware and no emulator. `core` performs no I/O and
|
||||
touches no peripherals; everything hardware-shaped lives in `firmware`.
|
||||
The `core`/`firmware` split exists so the entire protocol can be exercised by
|
||||
ordinary host unit tests (`cargo test`) with no hardware and no emulator. `core`
|
||||
performs no I/O and touches no peripherals; everything hardware-shaped lives in
|
||||
`firmware`.
|
||||
|
||||
## Pinout
|
||||
|
||||
@@ -199,18 +202,30 @@ unknown cmd 0x7E → 01 7E 00 01 A0
|
||||
`PING` is the intended first bring-up step: it exercises framing, CRC and the
|
||||
RS485 driver-enable turnaround without involving the sensor at all.
|
||||
|
||||
## Per-unit configuration
|
||||
## Configuration
|
||||
|
||||
Everything a unit needs is in `firmware/src/board.rs`:
|
||||
Per-unit settings live in `firmware/src/board.rs`:
|
||||
|
||||
```rust
|
||||
pub const UNIT_ADDRESS: u8 = 0x01; // this node's bus address
|
||||
pub const DEVICE_SERIAL: u32 = 0x0000_0001;
|
||||
pub const BAUD_RATE: u32 = 19_200;
|
||||
pub const SENSOR_I2C_ADDR: u8 = 0x44; // match the ADDR strap
|
||||
pub const SENSOR_PERIOD_MS: u32 = 1_000;
|
||||
```
|
||||
|
||||
The **line rate is not** among them. It belongs to the segment rather than to any
|
||||
one board, so it is defined once in `core/src/timing.rs` and read by both the
|
||||
node and the bridge:
|
||||
|
||||
```rust
|
||||
pub const BAUD_RATE: u32 = 19_200; // wiredsensor_core::timing
|
||||
```
|
||||
|
||||
A baud mismatch between two ends of a bus is silent and presents as random CRC
|
||||
failures, which is a poor thing to debug — hence one definition rather than a
|
||||
copy per firmware. `INTER_FRAME_GAP_US` and `CHAR_TIME_US` derive from it, and a
|
||||
unit test asserts they cannot go stale if it changes.
|
||||
|
||||
The address is compile-time by design: no flash wear, no commissioning protocol,
|
||||
and no way for a bus glitch to renumber a live node. The cost is one image per
|
||||
unit, so keep `UNIT_ADDRESS` and `DEVICE_SERIAL` the only things that differ.
|
||||
@@ -223,7 +238,7 @@ is already reserved for it.
|
||||
|
||||
```bash
|
||||
cargo build --release -p wiredsensor-fw # firmware, thumbv6m-none-eabi
|
||||
cargo test -p wiredsensor-core --target x86_64-unknown-linux-gnu # 42 host tests
|
||||
cargo test -p wiredsensor-core --target x86_64-unknown-linux-gnu # 43 host tests
|
||||
```
|
||||
|
||||
The host-target flag is needed because `.cargo/config.toml` defaults the whole
|
||||
@@ -283,6 +298,82 @@ if you want to see it.
|
||||
Note `16c0:27dd` is the pid.codes generic CDC-ACM pair, intended for development.
|
||||
Replace it before shipping.
|
||||
|
||||
## End-to-end bus testing
|
||||
|
||||
`bridge/` turns a spare RP2040 into a USB-to-RS485 bridge, and
|
||||
`tools/wiredsensor.py` drives the protocol from the PC through it. Together they
|
||||
test the bus itself rather than just the node's internals.
|
||||
|
||||
Two design choices make this a usable test instrument rather than a mirror of
|
||||
the firmware's own assumptions:
|
||||
|
||||
- **The bridge is protocol-agnostic.** Bytes from USB go out on the pair, bytes
|
||||
from the pair come back up USB. It knows nothing of frames, addresses or CRCs,
|
||||
so it cannot have a bug that happens to agree with the node's.
|
||||
- **The PC side reimplements the wire format** from the specification above
|
||||
rather than sharing `wiredsensor-core`. Shared code would let a framing or CRC
|
||||
bug cancel out and every test pass regardless.
|
||||
|
||||
### Wiring
|
||||
|
||||
You need two transceiver modules. The bridge mirrors the node's pin assignment,
|
||||
so one diagram covers both boards: `GPIO0 → DI`, `GPIO1 ← RO`, `GPIO2 → DE+RE`.
|
||||
|
||||
```
|
||||
bridge A ────────── A node
|
||||
B ────────── B
|
||||
GND ────────── GND
|
||||
[120Ω] [120Ω] ← across A-B at each end; both are segment ends
|
||||
```
|
||||
|
||||
Ground is common if both boards are USB-powered from the same host. If only one
|
||||
is, run **VSYS → VSYS** plus **GND → GND** between them — not `3V3(OUT)`, which
|
||||
back-feeds the regulator on the unpowered board.
|
||||
|
||||
### Running
|
||||
|
||||
```bash
|
||||
cargo build --release -p wiredsensor-bridge
|
||||
cp target/thumbv6m-none-eabi/release/wiredsensor-bridge /tmp/bridge.elf
|
||||
picotool load -x /tmp/bridge.elf # hold BOOTSEL while plugging in
|
||||
```
|
||||
|
||||
The bridge is `16c0:27de`, the node `16c0:27dd`. Find it with:
|
||||
|
||||
```bash
|
||||
for d in /dev/ttyACM*; do udevadm info -q property -n $d | grep -q 27de && echo $d; done
|
||||
```
|
||||
|
||||
```bash
|
||||
./tools/wiredsensor.py --port /dev/ttyACM2 measure
|
||||
./tools/wiredsensor.py --port /dev/ttyACM2 monitor
|
||||
./tools/wiredsensor.py --port /dev/ttyACM2 -v test # -v dumps bus traffic
|
||||
```
|
||||
|
||||
### What the suite covers
|
||||
|
||||
15 checks. Framing and CRC arithmetic are already covered by the host tests;
|
||||
what only a real bus can verify is everything timing-dependent:
|
||||
|
||||
- **Silence where required** — frames for another unit, broadcasts, bad CRCs,
|
||||
truncated frames and frames whose `LEN` lies must all produce *no reply*. A
|
||||
node that wrongly answered would collide with whoever was actually addressed.
|
||||
- **Driver-enable turnaround** — every intact reply is evidence that `DE` was
|
||||
held past the final stop bit. Release it early and the last byte truncates.
|
||||
- **No state leakage** between back-to-back requests.
|
||||
- **Binary transparency** across the full legal frame-size range.
|
||||
- **Counter integrity** — inject one corrupt frame, assert `crc_errors` rises by
|
||||
exactly one.
|
||||
|
||||
This earned its keep immediately: it caught a frame-truncation bug that both the
|
||||
host tests and the sensor verification were structurally incapable of reaching,
|
||||
because it needed real bus timing *and* a frame longer than the RX FIFO
|
||||
watermark. See the first item under *Details that are easy to get wrong*.
|
||||
|
||||
Frames must fit one 64-byte USB packet, since each host write becomes exactly one
|
||||
DE-bracketed transmission. Every real command is at most 21 bytes; only an
|
||||
oversized `PING` can approach the limit.
|
||||
|
||||
## Design notes
|
||||
|
||||
### Why RTIC rather than Embassy
|
||||
@@ -324,15 +415,31 @@ The ISR deliberately does no parsing. At 19200 baud a character arrives every
|
||||
~520 µs, roughly 65,000 core cycles, so there is ample slack; keeping the handler
|
||||
to a FIFO drain is what bounds the jitter everything else sees.
|
||||
|
||||
### Three details that are easy to get wrong
|
||||
### Details that are easy to get wrong
|
||||
|
||||
**`RTIM` alone is not a sufficient frame delimiter.** It only fires while the RX
|
||||
FIFO is non-empty, so a frame whose length lands exactly on the FIFO watermark
|
||||
can be drained by the watermark interrupt and never produce a timeout. The
|
||||
authoritative delimiter is therefore a monotonic timer measured from the last
|
||||
received byte; `RTIM` merely makes the common case prompt. `frame_gap` re-reads
|
||||
the arrival timestamp and sleeps again rather than cancelling and respawning a
|
||||
timer per byte.
|
||||
**`RTIM` is not a sufficient frame delimiter, and an interrupt timestamp is not
|
||||
a trustworthy end-of-frame.** Two separate traps here, and the second one bit us
|
||||
for real.
|
||||
|
||||
`RTIM` only fires while the RX FIFO is non-empty, so a frame drained exactly
|
||||
empty by the watermark interrupt never produces a timeout at all.
|
||||
|
||||
Worse: once the watermark interrupt has fired mid-frame — at 16 bytes, with the
|
||||
FIFO 32 deep — the *remaining* bytes sit below the watermark and raise no
|
||||
interrupt whatsoever until the timeout eventually arrives. A gap measured from
|
||||
the timestamp the ISR left behind therefore expires while the tail of the frame
|
||||
is still arriving, and the node parses a prefix whose `LEN` disagrees with its
|
||||
length. Every frame of 16 bytes or more was silently rejected this way; frames
|
||||
under 16 never trip the watermark, so their only interrupt is the timeout, by
|
||||
which point every byte is drained and the timestamp is honest. That is why all
|
||||
five real commands worked and only an oversized `PING` exposed it.
|
||||
|
||||
So `frame_gap` drains the FIFO *itself* before each decision rather than trusting
|
||||
the ISR's timestamp, which lets a byte that has landed push the deadline back.
|
||||
The gap is then measured from when the byte was observed rather than when it
|
||||
arrived, making the reply up to one gap period later than strictly necessary —
|
||||
1.8 ms is a fair price. It re-reads and sleeps again rather than cancelling and
|
||||
respawning a timer per byte.
|
||||
|
||||
**`DE` must be held until the last stop bit is gone.** Releasing it when the TX
|
||||
FIFO empties truncates the final character for every listener — a fault that
|
||||
|
||||
@@ -12,6 +12,10 @@ test = false
|
||||
bench = false
|
||||
|
||||
[dependencies]
|
||||
# Only for the shared line rate; the bridge deliberately knows nothing about
|
||||
# the protocol itself.
|
||||
wiredsensor-core = { workspace = true }
|
||||
|
||||
rp2040-hal = { version = "0.12", features = ["rt", "critical-section-impl"] }
|
||||
rp2040-boot2 = "0.3"
|
||||
|
||||
|
||||
+3
-3
@@ -42,9 +42,9 @@ use usb_device::{
|
||||
device::{StringDescriptors, UsbDeviceBuilder, UsbVidPid},
|
||||
};
|
||||
use usbd_serial::SerialPort;
|
||||
|
||||
/// Line rate. Must match the sensor node's `board::BAUD_RATE`.
|
||||
const BAUD_RATE: u32 = 19_200;
|
||||
// The one thing shared with the node: both ends of the bus must agree on the
|
||||
// line rate, so it is defined once in the core crate rather than copied here.
|
||||
use wiredsensor_core::timing::BAUD_RATE;
|
||||
|
||||
/// Crystal fitted to the board.
|
||||
const XTAL_FREQ_HZ: u32 = 12_000_000;
|
||||
|
||||
+30
-4
@@ -1,12 +1,29 @@
|
||||
//! Wire timing derived from the line rate.
|
||||
//! The line rate, and the wire timing derived from it.
|
||||
//!
|
||||
//! Kept here rather than in the firmware so the arithmetic — which decides
|
||||
//! whether frames are delimited correctly and whether the transceiver is
|
||||
//! released at the right moment — is covered by host tests.
|
||||
//! Kept here rather than in the firmware for two reasons. The arithmetic —
|
||||
//! which decides whether frames are delimited correctly and whether the
|
||||
//! transceiver is released at the right moment — is covered by host tests. And
|
||||
//! [`BAUD_RATE`] is a property of the *segment*, not of any one board: the node
|
||||
//! and every master on the bus must agree on it, so it is defined once here
|
||||
//! rather than copied into each firmware and kept in sync by hand.
|
||||
|
||||
/// Line rate for the whole segment.
|
||||
///
|
||||
/// 19200 8N1 is the Modbus default and a safe starting point for long runs.
|
||||
/// Changing it here changes it for every device built from this workspace,
|
||||
/// which is the point — a mismatch between two ends is silent and looks like
|
||||
/// random CRC failures.
|
||||
pub const BAUD_RATE: u32 = 19_200;
|
||||
|
||||
/// Bits on the wire per character: 1 start + 8 data + 1 stop, i.e. 8N1.
|
||||
pub const BITS_PER_CHAR: u32 = 10;
|
||||
|
||||
/// Inter-frame idle gap at [`BAUD_RATE`], in microseconds.
|
||||
pub const INTER_FRAME_GAP_US: u32 = inter_frame_gap_us(BAUD_RATE);
|
||||
|
||||
/// Time to clock out one character at [`BAUD_RATE`], in microseconds.
|
||||
pub const CHAR_TIME_US: u32 = char_time_us(BAUD_RATE);
|
||||
|
||||
/// Idle time that delimits one frame from the next, in microseconds.
|
||||
///
|
||||
/// Modbus specifies 3.5 character times, but pins the value at a fixed 1750 µs
|
||||
@@ -79,4 +96,13 @@ mod tests {
|
||||
assert_eq!(tx_time_us(1, 19_200), char_time_us(19_200));
|
||||
assert_eq!(tx_time_us(21, 19_200), 21 * 520);
|
||||
}
|
||||
|
||||
/// The derived constants must track whatever [`BAUD_RATE`] is set to, so
|
||||
/// changing the line rate cannot leave a stale gap or character time behind.
|
||||
#[test]
|
||||
fn derived_constants_follow_the_configured_rate() {
|
||||
assert_eq!(INTER_FRAME_GAP_US, inter_frame_gap_us(BAUD_RATE));
|
||||
assert_eq!(CHAR_TIME_US, char_time_us(BAUD_RATE));
|
||||
assert!(INTER_FRAME_GAP_US > CHAR_TIME_US);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -86,15 +86,11 @@ pub const XTAL_FREQ_HZ: u32 = 12_000_000;
|
||||
|
||||
// -------------------------------------------------------------- bus timing ---
|
||||
|
||||
/// RS485 line rate. 19200 8N1 is the Modbus default and a safe starting point
|
||||
/// for long segments; every timing constant below derives from it.
|
||||
pub const BAUD_RATE: u32 = 19_200;
|
||||
|
||||
/// Idle time that delimits one frame from the next, in microseconds.
|
||||
pub const INTER_FRAME_GAP_US: u32 = timing::inter_frame_gap_us(BAUD_RATE);
|
||||
|
||||
/// Time to clock out one character at [`BAUD_RATE`], in microseconds.
|
||||
pub const CHAR_TIME_US: u32 = timing::char_time_us(BAUD_RATE);
|
||||
// Re-exported rather than redefined. The line rate belongs to the segment, not
|
||||
// to this board, so it lives in `wiredsensor_core::timing` where the bridge
|
||||
// firmware reads the same value — a mismatch between two ends of the bus is
|
||||
// silent and presents as random CRC failures. Change it there.
|
||||
pub use timing::{BAUD_RATE, CHAR_TIME_US, INTER_FRAME_GAP_US};
|
||||
|
||||
// ------------------------------------------------------------ sensor timing ---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user