From 3c5e9894d092ee579fd92ff0a8314afc59c8053c Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Thu, 2 Jan 2025 08:32:46 +0100 Subject: [PATCH 01/12] FIX: Handle Non-Guaranteed IO Trait Read/Write * Return Vec as read() from buffer may contain more than 1 messages (currently up to 2 based on MAX_SIZE) * Store partial messages in struct value until next read() * Change to postcard::to_slice_cobs() to allow individual messages to be identified * Loop read() until at least one complete message received * Loop write() until all bytes confirmed to be sent * Increase SPLIT_MESSAGE_MAX_SIZE to allow for COBS overhead * Handles read() returning < 1 message, > 1 && < 2 messages and up to < 3 messages correctly --- rmk/src/split/driver.rs | 33 +++++++------ rmk/src/split/mod.rs | 2 +- rmk/src/split/serial/mod.rs | 95 +++++++++++++++++++++++++++++-------- 3 files changed, 94 insertions(+), 36 deletions(-) diff --git a/rmk/src/split/driver.rs b/rmk/src/split/driver.rs index 6216dad1..027a9e1d 100644 --- a/rmk/src/split/driver.rs +++ b/rmk/src/split/driver.rs @@ -4,6 +4,7 @@ use crate::keyboard::{key_event_channel, KeyEvent}; use super::SplitMessage; use defmt::{debug, error}; +use heapless::Vec; #[derive(Debug, Clone, Copy, defmt::Format)] pub(crate) enum SplitDriverError { @@ -16,7 +17,7 @@ pub(crate) enum SplitDriverError { /// Split message reader from other split devices pub(crate) trait SplitReader { - async fn read(&mut self) -> Result; + async fn read(&mut self) -> Result, SplitDriverError>; } /// Split message writer to other split devices @@ -62,21 +63,23 @@ impl< pub(crate) async fn run(mut self) -> ! { loop { match self.receiver.read().await { - Ok(received_message) => { - debug!("Received peripheral message: {}", received_message); - if let SplitMessage::Key(e) = received_message { - // Check row/col - if e.row as usize > ROW || e.col as usize > COL { - error!("Invalid peripheral row/col: {} {}", e.row, e.col); - continue; + Ok(received_messages) => { + for received_message in received_messages { + debug!("Received peripheral message: {}", received_message); + if let SplitMessage::Key(e) = received_message { + // Check row/col + if e.row as usize > ROW || e.col as usize > COL { + error!("Invalid peripheral row/col: {} {}", e.row, e.col); + continue; + } + key_event_channel + .send(KeyEvent { + row: e.row + ROW_OFFSET as u8, + col: e.col + COL_OFFSET as u8, + pressed: e.pressed, + }) + .await; } - key_event_channel - .send(KeyEvent { - row: e.row + ROW_OFFSET as u8, - col: e.col + COL_OFFSET as u8, - pressed: e.pressed, - }) - .await; } } Err(e) => error!("Peripheral message read error: {:?}", e), diff --git a/rmk/src/split/mod.rs b/rmk/src/split/mod.rs index 43c5fe70..a0136c1b 100644 --- a/rmk/src/split/mod.rs +++ b/rmk/src/split/mod.rs @@ -13,7 +13,7 @@ pub mod peripheral; pub(crate) mod serial; /// Maximum size of a split message -pub const SPLIT_MESSAGE_MAX_SIZE: usize = SplitMessage::POSTCARD_MAX_SIZE + 4; +pub const SPLIT_MESSAGE_MAX_SIZE: usize = SplitMessage::POSTCARD_MAX_SIZE + 6; /// Message used from central & peripheral communication #[derive(Serialize, Deserialize, Debug, Clone, Copy, MaxSize, defmt::Format)] diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index 798c5284..cbccc84e 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -7,6 +7,7 @@ use crate::split::{ }; use super::driver::SplitDriverError; +use heapless::Vec; // Receive split message from peripheral via serial and process it /// @@ -38,43 +39,97 @@ pub(crate) async fn run_serial_peripheral_monitor< /// Serial driver for BOTH split central and peripheral pub(crate) struct SerialSplitDriver { serial: S, + buffer: [u8; SPLIT_MESSAGE_MAX_SIZE], + n_bytes_part: usize, } impl SerialSplitDriver { pub(crate) fn new(serial: S) -> Self { - Self { serial } + Self { + serial, + buffer: [0_u8; SPLIT_MESSAGE_MAX_SIZE], + n_bytes_part: 0, + } } } impl SplitReader for SerialSplitDriver { - async fn read(&mut self) -> Result { - let mut buf = [0_u8; SPLIT_MESSAGE_MAX_SIZE]; - let n_bytes = self - .serial - .read(&mut buf) - .await - .map_err(|_e| SplitDriverError::SerialError)?; - if n_bytes == 0 { - return Err(SplitDriverError::EmptyMessage); + async fn read(&mut self) -> Result, SplitDriverError> { + const SENTINEL: u8 = 0x00; + let mut messages = Vec::new(); + let mut n_bytes_sum = self.n_bytes_part; + while n_bytes_sum < self.buffer.len() { + let n_bytes = self + .serial + .read(&mut self.buffer[n_bytes_sum..]) + .await + .map_err(|_e| SplitDriverError::SerialError)?; + if n_bytes == 0 { + return Err(SplitDriverError::EmptyMessage); + } + + n_bytes_sum += n_bytes; + if self.buffer[..n_bytes_sum].contains(&SENTINEL) { + break; + } } - let message: SplitMessage = postcard::from_bytes(&buf).map_err(|e| { - error!("Postcard deserialize split message error: {}", e); - SplitDriverError::DeserializeError - })?; - Ok(message) + + let mut start_byte = 0; + let mut end_byte = start_byte; + let mut partial_message = false; + while start_byte < n_bytes_sum { + let value = self.buffer[end_byte]; + if value == SENTINEL { + postcard::from_bytes_cobs(&mut self.buffer[start_byte..=end_byte]).map_or_else( + |e| error!("Postcard deserialize split message error: {}", e), + |message| { + messages + .push(message) + .unwrap_or_else(|_m| error!("Split message vector full")); + }, + ); + start_byte = end_byte + 1; + end_byte = start_byte; + continue; + } else if end_byte + value as usize >= n_bytes_sum { + partial_message = true; + break; + } + // Next Zero Data Byte + end_byte += value as usize; + } + + if partial_message { + // Store Partial Message for Next Read + self.n_bytes_part = n_bytes_sum - start_byte; + self.buffer.copy_within(start_byte..n_bytes_sum, 0); + self.buffer[self.n_bytes_part..].fill(0); + } else { + // Reset Buffer + self.n_bytes_part = 0; + self.buffer.fill(0); + } + + Ok(messages) } } impl SplitWriter for SerialSplitDriver { async fn write(&mut self, message: &SplitMessage) -> Result { let mut buf = [0_u8; SPLIT_MESSAGE_MAX_SIZE]; - let bytes = postcard::to_slice(message, &mut buf).map_err(|e| { + let bytes = postcard::to_slice_cobs(message, &mut buf).map_err(|e| { error!("Postcard serialize split message error: {}", e); SplitDriverError::SerializeError })?; - self.serial - .write(bytes) - .await - .map_err(|_e| SplitDriverError::SerialError) + let mut remaining_bytes = bytes.len(); + while remaining_bytes > 0 { + let sent_bytes = self + .serial + .write(&bytes[bytes.len() - remaining_bytes..]) + .await + .map_err(|_e| SplitDriverError::SerialError)?; + remaining_bytes -= sent_bytes; + } + Ok(bytes.len()) } } From 6f56895b02183e2ace3d4b530d3040a2496b21a6 Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Fri, 3 Jan 2025 18:06:15 +0100 Subject: [PATCH 02/12] UPD: Make Cancel-Safe, Always Store n_bytes_part --- rmk/src/split/serial/mod.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index cbccc84e..4663ccc2 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -57,19 +57,18 @@ impl SplitReader for SerialSplitDriver { async fn read(&mut self) -> Result, SplitDriverError> { const SENTINEL: u8 = 0x00; let mut messages = Vec::new(); - let mut n_bytes_sum = self.n_bytes_part; - while n_bytes_sum < self.buffer.len() { + while self.n_bytes_part < self.buffer.len() { let n_bytes = self .serial - .read(&mut self.buffer[n_bytes_sum..]) + .read(&mut self.buffer[self.n_bytes_part..]) .await .map_err(|_e| SplitDriverError::SerialError)?; if n_bytes == 0 { return Err(SplitDriverError::EmptyMessage); } - n_bytes_sum += n_bytes; - if self.buffer[..n_bytes_sum].contains(&SENTINEL) { + self.n_bytes_part += n_bytes; + if self.buffer[..self.n_bytes_part].contains(&SENTINEL) { break; } } @@ -77,7 +76,7 @@ impl SplitReader for SerialSplitDriver { let mut start_byte = 0; let mut end_byte = start_byte; let mut partial_message = false; - while start_byte < n_bytes_sum { + while start_byte < self.n_bytes_part { let value = self.buffer[end_byte]; if value == SENTINEL { postcard::from_bytes_cobs(&mut self.buffer[start_byte..=end_byte]).map_or_else( @@ -91,7 +90,7 @@ impl SplitReader for SerialSplitDriver { start_byte = end_byte + 1; end_byte = start_byte; continue; - } else if end_byte + value as usize >= n_bytes_sum { + } else if end_byte + value as usize >= self.n_bytes_part { partial_message = true; break; } @@ -101,8 +100,8 @@ impl SplitReader for SerialSplitDriver { if partial_message { // Store Partial Message for Next Read - self.n_bytes_part = n_bytes_sum - start_byte; - self.buffer.copy_within(start_byte..n_bytes_sum, 0); + self.buffer.copy_within(start_byte..self.n_bytes_part, 0); + self.n_bytes_part = self.n_bytes_part - start_byte; self.buffer[self.n_bytes_part..].fill(0); } else { // Reset Buffer From d8091831f7ed7190bf8a73ab534e68920609084a Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Fri, 3 Jan 2025 18:07:40 +0100 Subject: [PATCH 03/12] UPD: Make Cancel-Safe, Always Store n_bytes_part --- rmk/src/split/serial/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index 4663ccc2..2d856596 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -102,11 +102,9 @@ impl SplitReader for SerialSplitDriver { // Store Partial Message for Next Read self.buffer.copy_within(start_byte..self.n_bytes_part, 0); self.n_bytes_part = self.n_bytes_part - start_byte; - self.buffer[self.n_bytes_part..].fill(0); } else { // Reset Buffer self.n_bytes_part = 0; - self.buffer.fill(0); } Ok(messages) From 2662afbe3bc787a38d173590674667a0695b4878 Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Fri, 3 Jan 2025 18:08:26 +0100 Subject: [PATCH 04/12] UPD: Reset Buffer on SerialError --- rmk/src/split/serial/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index 2d856596..d53bac5c 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -62,6 +62,7 @@ impl SplitReader for SerialSplitDriver { .serial .read(&mut self.buffer[self.n_bytes_part..]) .await + .inspect_err(|_e| self.n_bytes_part = 0) .map_err(|_e| SplitDriverError::SerialError)?; if n_bytes == 0 { return Err(SplitDriverError::EmptyMessage); From 68ca8964eec68c9f7c92ebf201f333a59c5673ca Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Fri, 3 Jan 2025 21:24:54 +0100 Subject: [PATCH 05/12] UPD: Receive Driver Message Vector --- rmk/src/split/peripheral.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/rmk/src/split/peripheral.rs b/rmk/src/split/peripheral.rs index 1a1efa00..44a28e88 100644 --- a/rmk/src/split/peripheral.rs +++ b/rmk/src/split/peripheral.rs @@ -147,12 +147,16 @@ impl SplitPeripheral { loop { match select(self.split_driver.read(), key_event_channel.receive()).await { embassy_futures::select::Either::First(m) => match m { - // Currently only handle the central state message - Ok(split_message) => match split_message { - SplitMessage::ConnectionState(state) => { - CONNECTION_STATE.store(state, core::sync::atomic::Ordering::Release); + Ok(split_messages) => { + for split_message in split_messages { + // Currently only handle the central state message + match split_message { + SplitMessage::ConnectionState(state) => { + CONNECTION_STATE.store(state, core::sync::atomic::Ordering::Release); + } + _ => (), + } } - _ => (), }, Err(e) => { error!("Split message read error: {:?}", e); From d05529e51ff5b0b9c1cf2b717b457ac2b50e3577 Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Sat, 4 Jan 2025 09:27:20 +0100 Subject: [PATCH 06/12] FMT: Fix Formatting --- rmk/src/split/driver.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/rmk/src/split/driver.rs b/rmk/src/split/driver.rs index bd9daf7d..1109cb68 100644 --- a/rmk/src/split/driver.rs +++ b/rmk/src/split/driver.rs @@ -75,6 +75,7 @@ impl< error!("SplitDriver write error: {}", e); } loop { + defmt::info!("test"); match select(self.receiver.read(), embassy_time::Timer::after_millis(500)).await { embassy_futures::select::Either::First(read_result) => match read_result { Ok(received_messages) => { @@ -86,16 +87,16 @@ impl< error!("Invalid peripheral row/col: {} {}", e.row, e.col); continue; } - + if CONNECTION_STATE.load(core::sync::atomic::Ordering::Acquire) { - // Only when the connection is established, send the key event. - KEY_EVENT_CHANNEL - .send(KeyEvent { - row: e.row + ROW_OFFSET as u8, - col: e.col + COL_OFFSET as u8, - pressed: e.pressed, - }) - .await; + // Only when the connection is established, send the key event. + KEY_EVENT_CHANNEL + .send(KeyEvent { + row: e.row + ROW_OFFSET as u8, + col: e.col + COL_OFFSET as u8, + pressed: e.pressed, + }) + .await; } } else { warn!("Key event from peripheral is ignored because the connection is not established."); From 7db2732860a39504e5e6400709142c09cbd2286e Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Sun, 5 Jan 2025 08:11:48 +0100 Subject: [PATCH 07/12] FIX: Handle Corrupt Messages, Avoid Overread * If message is corrupt, unlikely to respect COBS encoding so not possible to skip bytes in loop * Avoid possible situation where end_byte is greater than buffer.len() --- rmk/src/split/serial/mod.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index 9d26bf44..aff44d0a 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -80,8 +80,7 @@ impl SplitReader for SerialSplitDriver { let mut start_byte = 0; let mut end_byte = start_byte; - let mut partial_message = false; - while start_byte < self.n_bytes_part { + while end_byte < self.n_bytes_part { let value = self.buffer[end_byte]; if value == SENTINEL { postcard::from_bytes_cobs(&mut self.buffer[start_byte..=end_byte]).map_or_else( @@ -93,17 +92,12 @@ impl SplitReader for SerialSplitDriver { }, ); start_byte = end_byte + 1; - end_byte = start_byte; - continue; - } else if end_byte + value as usize >= self.n_bytes_part { - partial_message = true; - break; } - // Next Zero Data Byte - end_byte += value as usize; + + end_byte += 1; } - if partial_message { + if start_byte != self.n_bytes_part { // Store Partial Message for Next Read self.buffer.copy_within(start_byte..self.n_bytes_part, 0); self.n_bytes_part = self.n_bytes_part - start_byte; From fe03f08bed797e12856e0aeeae2c87d417176321 Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Sun, 5 Jan 2025 08:35:13 +0100 Subject: [PATCH 08/12] FIX: Remove Debug Print --- rmk/src/split/driver.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rmk/src/split/driver.rs b/rmk/src/split/driver.rs index 1109cb68..a7ea1025 100644 --- a/rmk/src/split/driver.rs +++ b/rmk/src/split/driver.rs @@ -75,7 +75,6 @@ impl< error!("SplitDriver write error: {}", e); } loop { - defmt::info!("test"); match select(self.receiver.read(), embassy_time::Timer::after_millis(500)).await { embassy_futures::select::Either::First(read_result) => match read_result { Ok(received_messages) => { From bc03071d8721d2940453744e5988cea64744950d Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Thu, 9 Jan 2025 20:31:53 +0100 Subject: [PATCH 09/12] UPD: Restore to #68b0025 --- rmk/src/split/driver.rs | 39 +++++++++++++++++-------------------- rmk/src/split/mod.rs | 2 +- rmk/src/split/peripheral.rs | 14 +++++-------- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/rmk/src/split/driver.rs b/rmk/src/split/driver.rs index a7ea1025..eb5e2604 100644 --- a/rmk/src/split/driver.rs +++ b/rmk/src/split/driver.rs @@ -7,7 +7,6 @@ use crate::CONNECTION_STATE; use crate::{event::KeyEvent, keyboard::KEY_EVENT_CHANNEL}; use defmt::{debug, error, warn}; use embassy_futures::select::select; -use heapless::Vec; #[derive(Debug, Clone, Copy, defmt::Format)] pub(crate) enum SplitDriverError { @@ -20,7 +19,7 @@ pub(crate) enum SplitDriverError { /// Split message reader from other split devices pub(crate) trait SplitReader { - async fn read(&mut self) -> Result, SplitDriverError>; + async fn read(&mut self) -> Result; } /// Split message writer to other split devices @@ -77,26 +76,24 @@ impl< loop { match select(self.receiver.read(), embassy_time::Timer::after_millis(500)).await { embassy_futures::select::Either::First(read_result) => match read_result { - Ok(received_messages) => { - for received_message in received_messages { - debug!("Received peripheral message: {}", received_message); - if let SplitMessage::Key(e) = received_message { - // Check row/col - if e.row as usize > ROW || e.col as usize > COL { - error!("Invalid peripheral row/col: {} {}", e.row, e.col); - continue; - } + Ok(received_message) => { + debug!("Received peripheral message: {}", received_message); + if let SplitMessage::Key(e) = received_message { + // Check row/col + if e.row as usize > ROW || e.col as usize > COL { + error!("Invalid peripheral row/col: {} {}", e.row, e.col); + continue; + } - if CONNECTION_STATE.load(core::sync::atomic::Ordering::Acquire) { - // Only when the connection is established, send the key event. - KEY_EVENT_CHANNEL - .send(KeyEvent { - row: e.row + ROW_OFFSET as u8, - col: e.col + COL_OFFSET as u8, - pressed: e.pressed, - }) - .await; - } + if CONNECTION_STATE.load(core::sync::atomic::Ordering::Acquire) { + // Only when the connection is established, send the key event. + KEY_EVENT_CHANNEL + .send(KeyEvent { + row: e.row + ROW_OFFSET as u8, + col: e.col + COL_OFFSET as u8, + pressed: e.pressed, + }) + .await; } else { warn!("Key event from peripheral is ignored because the connection is not established."); } diff --git a/rmk/src/split/mod.rs b/rmk/src/split/mod.rs index 8057de6e..e4f1620a 100644 --- a/rmk/src/split/mod.rs +++ b/rmk/src/split/mod.rs @@ -13,7 +13,7 @@ pub mod peripheral; pub mod serial; /// Maximum size of a split message -pub const SPLIT_MESSAGE_MAX_SIZE: usize = SplitMessage::POSTCARD_MAX_SIZE + 6; +pub const SPLIT_MESSAGE_MAX_SIZE: usize = SplitMessage::POSTCARD_MAX_SIZE + 4; /// Message used from central & peripheral communication #[derive(Serialize, Deserialize, Debug, Clone, Copy, MaxSize, defmt::Format)] diff --git a/rmk/src/split/peripheral.rs b/rmk/src/split/peripheral.rs index 454d07ee..619d971e 100644 --- a/rmk/src/split/peripheral.rs +++ b/rmk/src/split/peripheral.rs @@ -147,16 +147,12 @@ impl SplitPeripheral { loop { match select(self.split_driver.read(), KEY_EVENT_CHANNEL.receive()).await { embassy_futures::select::Either::First(m) => match m { - Ok(split_messages) => { - for split_message in split_messages { - // Currently only handle the central state message - match split_message { - SplitMessage::ConnectionState(state) => { - CONNECTION_STATE.store(state, core::sync::atomic::Ordering::Release); - } - _ => (), - } + // Currently only handle the central state message + Ok(split_message) => match split_message { + SplitMessage::ConnectionState(state) => { + CONNECTION_STATE.store(state, core::sync::atomic::Ordering::Release); } + _ => (), }, Err(e) => { error!("Split message read error: {:?}", e); From f6522df5cbc9bb83ef2b779677ebff9232d431c3 Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Fri, 10 Jan 2025 07:53:39 +0100 Subject: [PATCH 10/12] UPD: Return Single Message, Remove Serialize Messages Loop --- rmk/src/split/serial/mod.rs | 54 +++++++++++++++---------------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index aff44d0a..bae26bae 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -11,7 +11,6 @@ use crate::{ }; use super::driver::SplitDriverError; -use heapless::Vec; // Receive split message from peripheral via serial and process it /// @@ -58,55 +57,46 @@ impl SerialSplitDriver { } impl SplitReader for SerialSplitDriver { - async fn read(&mut self) -> Result, SplitDriverError> { + async fn read(&mut self) -> Result { const SENTINEL: u8 = 0x00; - let mut messages = Vec::new(); while self.n_bytes_part < self.buffer.len() { let n_bytes = self .serial .read(&mut self.buffer[self.n_bytes_part..]) .await - .inspect_err(|_e| self.n_bytes_part = 0) - .map_err(|_e| SplitDriverError::SerialError)?; + .map_err(|_e| { + self.n_bytes_part = 0; + SplitDriverError::SerialError + })?; if n_bytes == 0 { return Err(SplitDriverError::EmptyMessage); } - self.n_bytes_part += n_bytes; + self.n_bytes_part = (self.n_bytes_part + n_bytes).min(self.buffer.len()); if self.buffer[..self.n_bytes_part].contains(&SENTINEL) { break; } } - let mut start_byte = 0; - let mut end_byte = start_byte; - while end_byte < self.n_bytes_part { - let value = self.buffer[end_byte]; - if value == SENTINEL { - postcard::from_bytes_cobs(&mut self.buffer[start_byte..=end_byte]).map_or_else( - |e| error!("Postcard deserialize split message error: {}", e), - |message| { - messages - .push(message) - .unwrap_or_else(|_m| error!("Split message vector full")); + let (message, n_bytes_unused) = { + let (message, unused_bytes): (SplitMessage, &mut [u8]) = + postcard::take_from_bytes_cobs(&mut self.buffer[..self.n_bytes_part]).map_err( + |e| { + error!("Postcard deserialize split message error: {}", e); + self.n_bytes_part = 0; + SplitDriverError::SerializeError }, - ); - start_byte = end_byte + 1; - } + )?; + (message, unused_bytes.len()) + }; - end_byte += 1; - } - - if start_byte != self.n_bytes_part { - // Store Partial Message for Next Read - self.buffer.copy_within(start_byte..self.n_bytes_part, 0); - self.n_bytes_part = self.n_bytes_part - start_byte; - } else { - // Reset Buffer - self.n_bytes_part = 0; - } + self.buffer.copy_within( + (self.n_bytes_part - n_bytes_unused).max(0)..self.n_bytes_part, + 0, + ); + self.n_bytes_part = n_bytes_unused; - Ok(messages) + Ok(message) } } From 478d2fe9314429f679c1cbfd26626b53a0e214b3 Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Sat, 11 Jan 2025 16:36:24 +0100 Subject: [PATCH 11/12] FIX: max() not valid on usize, n_bytes_unused <= n_bytes_part --- rmk/src/split/serial/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index 980e11c5..6badb2d5 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -91,7 +91,7 @@ impl SplitReader for SerialSplitDriver { }; self.buffer.copy_within( - (self.n_bytes_part - n_bytes_unused).max(0)..self.n_bytes_part, + self.n_bytes_part-n_bytes_unused..self.n_bytes_part, 0, ); self.n_bytes_part = n_bytes_unused; From a3a5489690852c8d83f5692fb5d20c7f6e939dba Mon Sep 17 00:00:00 2001 From: Stuart Andrews Date: Wed, 15 Jan 2025 12:45:17 +0100 Subject: [PATCH 12/12] UPD: Store Bytes After Sentinel on Error * Pass clone of buffer to take_from_bytes_cobs to avoid buffer being modified on error * Get position of 0x00 sentinel byte in buffer on error, and handle unused bytes as no error --- rmk/src/split/serial/mod.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/rmk/src/split/serial/mod.rs b/rmk/src/split/serial/mod.rs index 6badb2d5..ae103ce8 100644 --- a/rmk/src/split/serial/mod.rs +++ b/rmk/src/split/serial/mod.rs @@ -78,25 +78,25 @@ impl SplitReader for SerialSplitDriver { } } - let (message, n_bytes_unused) = { - let (message, unused_bytes): (SplitMessage, &mut [u8]) = - postcard::take_from_bytes_cobs(&mut self.buffer[..self.n_bytes_part]).map_err( - |e| { - error!("Postcard deserialize split message error: {}", e); - self.n_bytes_part = 0; - SplitDriverError::SerializeError - }, - )?; - (message, unused_bytes.len()) + let (result, n_bytes_unused) = match postcard::take_from_bytes_cobs::( + &mut self.buffer.clone()[..self.n_bytes_part], + ) { + Ok((message, unused_bytes)) => (Ok(message), unused_bytes.len()), + Err(e) => { + error!("Postcard deserialize split message error: {}", e); + let n_bytes_unused = self.buffer[..self.n_bytes_part] + .iter() + .position(|&x| x == SENTINEL) + .map_or(0, |index| self.n_bytes_part - index - 1); + (Err(SplitDriverError::SerializeError), n_bytes_unused) + } }; - self.buffer.copy_within( - self.n_bytes_part-n_bytes_unused..self.n_bytes_part, - 0, - ); + self.buffer + .copy_within(self.n_bytes_part - n_bytes_unused..self.n_bytes_part, 0); self.n_bytes_part = n_bytes_unused; - Ok(message) + result } }