diff --git a/ch32/Cargo.toml b/ch32/Cargo.toml index 1ec80fe..13cfc07 100644 --- a/ch32/Cargo.toml +++ b/ch32/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "tinyboot-ch32" description = "tinyboot ch32 implementation" -version = "0.7.1" +version = "0.7.2" edition.workspace = true repository.workspace = true license.workspace = true @@ -52,8 +52,8 @@ repository = "https://github.com/OpenServoCore/tinyboot" license = "MIT OR Apache-2.0" [workspace.dependencies] -tinyboot-core = { version = "0.6.2", path = "../lib/core", default-features = false } -tinyboot-ch32 = { version = "0.7.1", path = ".", default-features = false } +tinyboot-core = { version = "0.6.3", path = "../lib/core", default-features = false } +tinyboot-ch32 = { version = "0.7.2", path = ".", default-features = false } embedded-hal = "1" embedded-io = "0.7" embedded-storage = "0.3" diff --git a/ch32/src/platform/transport/usart/mod.rs b/ch32/src/platform/transport/usart/mod.rs index 3d0cf75..5ba58dc 100644 --- a/ch32/src/platform/transport/usart/mod.rs +++ b/ch32/src/platform/transport/usart/mod.rs @@ -113,12 +113,15 @@ impl Usart { afio::set_usart_remap(usart_n, config.mapping.remap_value()); - let wire = config.wire.init(tx_pin, rx_pin); let tx_en = config.tx_en.init(); let brr = stored_brr(config.pclk, brr(config.pclk, config.baud.hz())); usart::init(regs, brr, W::HALF_DUPLEX); + // After TE: a TX pin switched to its USART function before then + // follows an output that idles low. + let wire = config.wire.init(tx_pin, rx_pin); + Usart { regs, wire, tx_en } } diff --git a/cli/Cargo.lock b/cli/Cargo.lock index 84d30e0..e2991b9 100644 --- a/cli/Cargo.lock +++ b/cli/Cargo.lock @@ -531,7 +531,7 @@ dependencies = [ [[package]] name = "tinyboot-protocol" -version = "0.5.1" +version = "0.5.2" dependencies = [ "bitflags 2.11.1", "embedded-io", diff --git a/docs/shared-bus.md b/docs/shared-bus.md index 7e4a41f..fdc3868 100644 --- a/docs/shared-bus.md +++ b/docs/shared-bus.md @@ -48,6 +48,8 @@ It never answers a frame it can't read (bad CRC, unknown command, payload too bi This means a device sitting in its bootloader never disturbs the bus. Several devices waiting for firmware at once is fine too. Each one stays quiet until the host picks it. +A device hears every frame on the bus, not just its own. A reply from another device is read and dropped without a CRC check, so the device is listening again right after the last byte. A request is different. Its CRC is checked after the last byte arrives, about 3 us per frame byte on a 48 MHz CH32, and the device cannot hear during that time. The host only sends its next request after the reply or after its listen window, so this never overlaps in normal use. If you write your own host and send a request that draws no reply, leave that much time before the next frame. + ## How `Select` works `Select` carries a seed and a prefix of id bits. Every device whose id starts with the prefix becomes selected and replies with its id. Every other device becomes deselected. diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 3badb02..67a6896 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -8,7 +8,7 @@ repository = "https://github.com/OpenServoCore/tinyboot" license = "MIT OR Apache-2.0" [workspace.dependencies] -tinyboot-protocol = { version = "0.5.0", path = "protocol" } +tinyboot-protocol = { version = "0.5.2", path = "protocol" } embedded-io = "0.7" embedded-io-async = "0.7" embedded-storage = "0.3" diff --git a/lib/core/Cargo.toml b/lib/core/Cargo.toml index 8c39eea..37bf2af 100644 --- a/lib/core/Cargo.toml +++ b/lib/core/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "tinyboot-core" description = "Core bootloader library for tinyboot" -version = "0.6.2" +version = "0.6.3" edition.workspace = true repository.workspace = true license.workspace = true diff --git a/lib/core/src/app.rs b/lib/core/src/app.rs index 01931da..c63a551 100644 --- a/lib/core/src/app.rs +++ b/lib/core/src/app.rs @@ -116,7 +116,7 @@ impl App(&mut self, rx: &mut R, tx: &mut W) { - let status = match self.frame.read(rx) { + let status = match self.frame.read_request(rx) { Ok(s) => s, Err(_) => return, }; @@ -132,7 +132,7 @@ impl App s, Err(_) => return, }; @@ -155,6 +155,9 @@ impl App Result<(), ReadError> { - let status = self.frame.read(&mut self.platform.transport)?; + let status = self.frame.read_request(&mut self.platform.transport)?; let reply = if A::ADDRESSABLE { if status != Status::Ok || self.frame.status != Status::Request { @@ -285,6 +285,9 @@ impl< } else { false } + } else if status == Status::Request { + // Another device's reply. + false } else if status != Status::Ok { self.frame.len = 0; self.frame.status = status; diff --git a/lib/core/tests/addressing.rs b/lib/core/tests/addressing.rs index 2468fcb..fd2d584 100644 --- a/lib/core/tests/addressing.rs +++ b/lib/core/tests/addressing.rs @@ -368,6 +368,68 @@ fn unaddressed_bootloader_refuses_select() { ); } +/// Bytes of a frame as another device or the host sends it. +fn frame_bytes(cmd: Cmd, status: Status, data: &[u8]) -> Vec { + let mut t = MockTransport::new(); + load(&mut t, cmd, status, 0, 0, data); + t.rx_buf[..t.rx_len].to_vec() +} + +/// Queue `frames` back to back on `t`'s receive side. +fn queue(t: &mut MockTransport, frames: &[Vec]) { + let all = frames.concat(); + t.rx_buf[..all.len()].copy_from_slice(&all); + t.rx_len = all.len(); + t.rx_pos = 0; + t.tx_pos = 0; + t.waited = None; +} + +#[test] +fn a_heard_reply_then_a_select_for_this_device_is_answered() { + let mut ps = platforms(1); + let mut devs = devices(&mut ps, &UID_FNS[..1]); + let d = &mut devs[0]; + let heard = frame_bytes(Cmd::Info, Status::Ok, &[0x5A; 18]); + let seed = 5; + let sel = frame_bytes( + Cmd::Select, + Status::Request, + &select_data(seed, &bits_of(&UIDS[0])), + ); + queue(&mut d.platform.transport, &[heard.clone(), sel]); + + d.dispatch().unwrap(); + assert_eq!(d.platform.transport.rx_pos, heard.len()); + assert!(reply(&d.platform.transport).is_none()); + + d.dispatch().unwrap(); + let f = reply(&d.platform.transport).expect("no Select reply"); + assert_eq!(uid_of(&f), UIDS[0]); + let slot = (crc16(CRC_INIT, &UIDS[0]) >> seed) as u8 & 15; + assert_eq!(d.platform.transport.waited, Some(slot)); +} + +#[test] +fn an_unaddressed_bootloader_never_answers_a_heard_reply() { + let mut p = platform(BootState::Idle); + let mut d = Dispatcher::<_, _, _, _, TEST_BUF_SIZE>::new(&mut p); + let mut bad = frame_bytes(Cmd::Info, Status::Ok, &[0; 18]); + let n = bad.len(); + bad[n - 1] ^= 1; + let info = frame_bytes(Cmd::Info, Status::Request, &[]); + queue( + &mut d.platform.transport, + &[frame_bytes(Cmd::Write, Status::Ok, &[]), bad, info], + ); + d.dispatch().unwrap(); + assert!(reply(&d.platform.transport).is_none()); + d.dispatch().unwrap(); + assert!(reply(&d.platform.transport).is_none()); + d.dispatch().unwrap(); + assert_eq!(reply(&d.platform.transport).unwrap().status, Status::Ok); +} + static APP_WAITED: std::sync::atomic::AtomicU8 = std::sync::atomic::AtomicU8::new(0xFF); fn app_wait(n: u8) { @@ -415,4 +477,19 @@ fn addressable_app_answers_only_once_selected() { app.poll(&mut rx, &mut tx); let f = reply(&tx).expect("no Info reply"); assert_eq!(unsafe { f.data.info }.mode, 1); + + // Another device's reply, then a request: the app reads past the one + // and answers the other. + tx.tx_pos = 0; + queue( + &mut rx, + &[ + frame_bytes(Cmd::Info, Status::Ok, &[0; 18]), + frame_bytes(Cmd::Info, Status::Request, &[]), + ], + ); + app.poll(&mut rx, &mut tx); + assert!(reply(&tx).is_none()); + app.poll(&mut rx, &mut tx); + assert!(reply(&tx).is_some()); } diff --git a/lib/protocol/Cargo.toml b/lib/protocol/Cargo.toml index 2901ad9..8f5bff5 100644 --- a/lib/protocol/Cargo.toml +++ b/lib/protocol/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "tinyboot-protocol" description = "Protocol implementation for tinyboot bootloader" -version = "0.5.1" +version = "0.5.2" edition.workspace = true repository.workspace = true license.workspace = true diff --git a/lib/protocol/src/frame.rs b/lib/protocol/src/frame.rs index 6ed0a89..2d047af 100644 --- a/lib/protocol/src/frame.rs +++ b/lib/protocol/src/frame.rs @@ -196,6 +196,24 @@ impl Frame { /// Returns `Ok(Status::Ok)` on success, `Ok(Status::*)` for protocol /// errors (CRC, invalid frame, overflow), `Err` only for transport IO. pub fn read(&mut self, r: &mut R) -> Result { + self.read_frame::(r) + } + + /// Read one frame as a device: as [`read`](Self::read), but a frame + /// whose status is not [`Status::Request`] is another device's reply. + /// Its payload and CRC are consumed unchecked and it returns + /// `Ok(Status::Request)`, which draws no reply, so the device listens + /// again at once. A reply whose payload exceeds `N` is left to the sync + /// scan. + pub fn read_request(&mut self, r: &mut R) -> Result { + self.read_frame::(r) + } + + #[inline(always)] + fn read_frame( + &mut self, + r: &mut R, + ) -> Result { self.sync.read(r)?; // Read header fields: cmd(1) + status(1) + addr(4) + len(2) = 8 bytes at offset 2 @@ -210,12 +228,17 @@ impl Frame { return Ok(Status::Unsupported); } + let heard = DEVICE && self.status != Status::Request; let data_len = self.len as usize; if data_len > N { // Same as above — we cannot drain a payload larger than our buffer. // The sync scanner recovers on the next frame. - return Ok(Status::PayloadOverflow); + return Ok(if heard { + Status::Request + } else { + Status::PayloadOverflow + }); } // Read data directly into buffer @@ -227,6 +250,10 @@ impl Frame { // Read CRC directly — [u8; 2] has no alignment constraint r.read_exact(&mut self.crc).map_err(|_| ReadError)?; + if heard { + return Ok(Status::Request); + } + // Validate CRC over body if self.crc != crc16(CRC_INIT, self.as_bytes(0, 10 + data_len)).to_le_bytes() { return Ok(Status::CrcMismatch); @@ -251,6 +278,21 @@ impl Frame { pub async fn read_async( &mut self, r: &mut R, + ) -> Result { + self.read_frame_async::(r).await + } + + /// Async version of [`read_request`](Self::read_request). + pub async fn read_request_async( + &mut self, + r: &mut R, + ) -> Result { + self.read_frame_async::(r).await + } + + async fn read_frame_async( + &mut self, + r: &mut R, ) -> Result { self.sync.read_async(r).await?; @@ -263,11 +305,16 @@ impl Frame { return Ok(Status::Unsupported); } + let heard = DEVICE && self.status != Status::Request; let data_len = self.len as usize; if data_len > N { // See sync read() for rationale on not draining here. - return Ok(Status::PayloadOverflow); + return Ok(if heard { + Status::Request + } else { + Status::PayloadOverflow + }); } if data_len > 0 { @@ -278,6 +325,10 @@ impl Frame { r.read_exact(&mut self.crc).await.map_err(|_| ReadError)?; + if heard { + return Ok(Status::Request); + } + if self.crc != crc16(CRC_INIT, self.as_bytes(0, 10 + data_len)).to_le_bytes() { return Ok(Status::CrcMismatch); } @@ -530,4 +581,89 @@ mod tests { Ok(Status::PayloadOverflow) ); } + + #[test] + fn read_rejects_a_bad_crc() { + let mut f = frame(Cmd::Write, Status::Request, 0, &[0xDE, 0xAD]); + let mut sink = Sink::new(); + f.send(&mut sink).unwrap(); + sink.buf[10] ^= 0x01; + let mut f2 = Frame::::default(); + assert_eq!( + f2.read(&mut MockReader::new(sink.written())), + Ok(Status::CrcMismatch) + ); + let mut f3 = Frame::::default(); + assert_eq!( + f3.read_request(&mut MockReader::new(sink.written())), + Ok(Status::CrcMismatch) + ); + } + + /// Wire bytes of `f`; `damage` flips a payload bit. + fn wire(f: &mut Frame, damage: bool) -> ([u8; 512], usize) { + let mut sink = Sink::new(); + f.send(&mut sink).unwrap(); + if damage { + sink.buf[10] ^= 0x01; + } + (sink.buf, sink.pos) + } + + fn heard_reply_then_request(damage: bool) { + let (r, rn) = wire(&mut frame(Cmd::Info, Status::Ok, 0, &[7; 18]), damage); + let (q, qn) = wire( + &mut frame(Cmd::Erase, Status::Request, 0x40, &[0x40, 0]), + false, + ); + let mut input = [0u8; 1024]; + input[..rn].copy_from_slice(&r[..rn]); + input[rn..rn + qn].copy_from_slice(&q[..qn]); + let mut rd = MockReader::new(&input[..rn + qn]); + + let mut dev = Frame::::default(); + assert_eq!(dev.read_request(&mut rd), Ok(Status::Request)); + assert_eq!(rd.pos, rn, "consumes exactly len + 2 after the header"); + + assert_eq!(dev.read_request(&mut rd), Ok(Status::Ok)); + assert_eq!(dev.cmd, Cmd::Erase); + assert_eq!(dev.addr(), 0x40); + assert_eq!(rd.pos, rn + qn); + } + + #[test] + fn device_read_consumes_a_heard_reply_and_parses_the_next_request() { + heard_reply_then_request(false); + } + + #[test] + fn device_read_never_validates_a_heard_reply() { + heard_reply_then_request(true); + } + + #[test] + fn host_read_still_validates_replies() { + let (r, rn) = wire(&mut frame(Cmd::Info, Status::Ok, 0, &[7; 18]), true); + let mut f = Frame::::default(); + assert_eq!( + f.read(&mut MockReader::new(&r[..rn])), + Ok(Status::CrcMismatch) + ); + } + + #[test] + fn device_read_leaves_an_oversized_reply_to_the_sync_scan() { + let mut f = Frame::<240> { + cmd: Cmd::Info, + status: Status::Ok, + ..Default::default() + }; + f.len = 100; + let mut sink = Sink::new(); + f.send(&mut sink).unwrap(); + let mut rd = MockReader::new(sink.written()); + let mut dev = Frame::::default(); + assert_eq!(dev.read_request(&mut rd), Ok(Status::Request)); + assert_eq!(rd.pos, 10); + } }