Skip to content

Commit

Permalink
Remove bincode-core and serde dependencies
Browse files Browse the repository at this point in the history 
- Manual serialization
  * Faster
  * Very simple in this case as all of the hard work splitting fields
    into bytes was already done
  * Serdes and bincode can be annoying to integrate with no_std
- Ended up being far easier than porting to bincode2
- No need to pad the serialization_len by 1 byte anymore (for the
  serialization length)
  • Loading branch information
@haata
haata committed Nov 12, 2021
1 parent d34ef8c commit 8d18fe9
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 103 deletions.
12 changes: 5 additions & 7 deletions hid-io-protocol/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,18 @@ device = []
# server feature is intended for use with full user-space applications
# with access to std for loggging messages
# Mostly no_std with some minor exceptions
server = ["log", "bincode_core/std"]
server = ["log"]

# Adds defmt support to useful enums and structs
defmt-impl = ["defmt", "heapless/defmt-impl"]


[dependencies]
arraydeque = { version = "^0.4", default-features = false }
bincode_core = { git = "https://github.com/bincode-org/bincode-core.git", version = "0.1.0" }
defmt = { version = "0.2", optional = true }
arraydeque = { version = "0.4", default-features = false }
defmt = { version = "0.3", optional = true }
heapless = { version = "0.7" }
log = { version = "^0.4", default-features = false, optional = true }
num_enum = { version = "^0.5", default-features = false }
serde = { version = "^1.0", default-features = false }
log = { version = "0.4", default-features = false, optional = true }
num_enum = { version = "0.5", default-features = false }


[dev-dependencies]
Expand Down
137 changes: 50 additions & 87 deletions hid-io-protocol/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@ pub mod test;

// ----- Crates -----

use bincode_core::{serialize, BufferWriter};
use core::convert::TryFrom;
use core::fmt;
use heapless::Vec;
use num_enum::{IntoPrimitive, TryFromPrimitive};
use serde::ser::{self, Serialize, SerializeSeq, Serializer};

#[cfg(feature = "server")]
use log::{error, warn};
Expand Down Expand Up @@ -136,6 +134,8 @@ pub enum HidIoCommandId {
#[derive(Debug)]
#[cfg_attr(feature = "defmt-impl", derive(defmt::Format))]
pub enum HidIoParseError {
BufferNotReady,
BufferDataTooSmall(usize),
InvalidContinuedIdByte(u8),
InvalidHidIoCommandId(u32),
InvalidPacketIdWidth(u8),
Expand Down Expand Up @@ -515,9 +515,9 @@ impl<const H: usize> HidIoPacketBuffer<H> {
/// Returns the currently computed serialized length of the
/// buffer. Can change based on the struct fields.
pub fn serialized_len(&self) -> u32 {
// Sync packets have a serialized length of 1 (+1 for length)
// Sync packets have a serialized length of 1
if self.ptype == HidIoPacketType::Sync {
return 1 + 1;
return 1;
}

let hdr_len = self.hdr_len();
Expand All @@ -531,8 +531,7 @@ impl<const H: usize> HidIoPacketBuffer<H> {
0
};

// Extra byte is a type field from the serializer
fullpackets + partialpacket + 1
fullpackets + partialpacket
}

/// Append payload data
Expand Down Expand Up @@ -674,48 +673,6 @@ impl<const H: usize> HidIoPacketBuffer<H> {
/// Serialize HidIoPacketBuffer
///
/// # Remarks
/// Provides a raw data vector to the serialized data.
/// Removes some of the header that Serialize from serde prepends.
pub fn serialize_buffer<'a>(
&mut self,
data: &'a mut [u8],
) -> Result<&'a [u8], HidIoParseError> {
let options = bincode_core::config::DefaultOptions::new();
let mut writer = BufferWriter::new(data);
let len;

// Serialize
match serialize(&self, &mut writer, options) {
Ok(_) => {}
Err(_e) => {
error!("Parse error: {:?}", _e);
return Err(HidIoParseError::SerializationError);
}
};

// Make sure serialization worked
len = writer.written_len();
if self.ptype == HidIoPacketType::Sync && len < 2
|| self.ptype != HidIoPacketType::Sync && len < 5
{
error!(
"Serialization too small: {} -> {:02X?}",
len,
writer.written_buffer()
);
return Err(HidIoParseError::SerializationFailedResultTooSmall(len));
}

// Slice off the first byte (type) header bytes from serde
let slice = &data[1..len as usize];
Ok(slice)
}
}

impl<const H: usize> Serialize for HidIoPacketBuffer<H> {
/// Serializer for HidIoPacketBuffer
///
/// # Remarks
/// Determine cont, width, upper_len and len fields
/// According to this C-Struct:
///
Expand All @@ -730,17 +687,17 @@ impl<const H: usize> Serialize for HidIoPacketBuffer<H> {
/// uint8_t data[0]; // Start of data payload (may start with Id)
/// };
/// ```
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
pub fn serialize_buffer<'a>(&self, data: &'a mut [u8]) -> Result<&'a [u8], HidIoParseError> {
// Check if buffer is ready to serialize
if !self.done {
return Err(ser::Error::custom("HidIoPacketBuffer is not 'done'"));
return Err(HidIoParseError::BufferNotReady);
}

// --- First Packet ---

// Keep track of the serialization buffer position
let mut pos = 0;

// Determine id_width
let id_width = self.id_width();

Expand Down Expand Up @@ -785,9 +742,7 @@ impl<const H: usize> Serialize for HidIoPacketBuffer<H> {
for idx in 0..id_width_len {
let id = (self.id as u32 >> (idx * 8)) as u8;
if id_vec.push(id).is_err() {
return Err(ser::Error::custom(
"HidIoPacketBuffer failed to convert Id into bytes, vec add failed.",
));
return Err(HidIoParseError::VecAddFailed);
}
}

Expand All @@ -804,31 +759,19 @@ impl<const H: usize> Serialize for HidIoPacketBuffer<H> {
// upper_len - 2 bits
(upper_len & 0x3);

// Header byte is always serialized
pos = serialize_byte(data, hdr_byte, pos)?;

// Determine if this is a sync packet (much simpler serialization)
if self.ptype == HidIoPacketType::Sync {
let mut state = serializer.serialize_seq(Some(0))?;
state.serialize_element(&hdr_byte)?;
return state.end();
return Ok(data);
}

// Serialize as a sequence
let mut state = serializer.serialize_seq(Some(0))?;

// Serialize header
state.serialize_element(&hdr_byte)?;

// Serialize length
state.serialize_element(&len)?;

// If SYNC packet
if self.ptype == HidIoPacketType::Sync {
return state.end();
}
pos = serialize_byte(data, len, pos)?;

// Serialize id
for id_byte in &id_vec {
state.serialize_element(id_byte)?;
}
pos = serialize_bytes(data, &id_vec[..id_width_len as usize], pos)?;

// Serialize payload data
// We can't just serialize directly (extra info is included), serialize each element of vector separately
Expand All @@ -839,13 +782,11 @@ impl<const H: usize> Serialize for HidIoPacketBuffer<H> {
// Payload that's available
&self.data[0..data_len as usize]
};
for elem in slice {
state.serialize_element(elem)?;
}
pos = serialize_bytes(data, slice, pos)?;

// Finish serialization if no more payload left
if !cont {
return state.end();
return Ok(data);
}

// Determine how much payload is left
Expand Down Expand Up @@ -897,15 +838,13 @@ impl<const H: usize> Serialize for HidIoPacketBuffer<H> {
(upper_len & 0x3);

// Serialize header
state.serialize_element(&hdr_byte)?;
pos = serialize_byte(data, hdr_byte, pos)?;

// Serialize length
state.serialize_element(&len)?;
pos = serialize_byte(data, len, pos)?;

// Serialize id
for id_byte in &id_vec {
state.serialize_element(id_byte)?;
}
pos = serialize_bytes(data, &id_vec[..id_width_len as usize], pos)?;

// Serialize payload data
// We can't just serialize directly (extra info is included), serialize each element of vector separately
Expand All @@ -917,18 +856,42 @@ impl<const H: usize> Serialize for HidIoPacketBuffer<H> {
data_len as usize
};
let slice = &self.data[last_slice_index..slice_end];
for elem in slice {
state.serialize_element(elem)?;
}
pos = serialize_bytes(data, slice, pos)?;

// Recalculate how much payload is left
payload_left -= (slice_end - last_slice_index) as u32;
last_slice_index += payload_len as usize;
}

// --- Finish serialization ---
state.end()
Ok(data)
}
}

/// Very simple error checking function for byte serialization
/// Returns the new serialization index pointer
fn serialize_byte(data: &mut [u8], byte: u8, pos: usize) -> Result<usize, HidIoParseError> {
// Make sure buffer is large enough
if pos >= data.len() {
return Err(HidIoParseError::BufferDataTooSmall(pos - 1));
}

data[pos] = byte;
Ok(pos + 1)
}

/// Very simple error checking function for byte array serialization
/// Returns the new serialization index pointer
fn serialize_bytes(data: &mut [u8], bytes: &[u8], pos: usize) -> Result<usize, HidIoParseError> {
// Make sure buffer is large enough
if pos + bytes.len() > data.len() && !bytes.is_empty() {
return Err(HidIoParseError::BufferDataTooSmall(pos - 1));
}

for (i, byte) in bytes.iter().enumerate() {
data[pos + i] = *byte;
}
Ok(pos + bytes.len())
}

impl fmt::Display for HidIoPacketType {
Expand Down
16 changes: 8 additions & 8 deletions hid-io-protocol/src/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn setup_logging_lite() -> Result<(), LogError> {

/// Loopback helper
/// Serializes, deserializes, then checks if same as original
fn loopback_serializer<const H: usize>(mut buffer: HidIoPacketBuffer<H>, data: &mut [u8]) {
fn loopback_serializer<const H: usize>(buffer: HidIoPacketBuffer<H>, data: &mut [u8]) {
// Serialize
let data = match buffer.serialize_buffer(data) {
Ok(data) => data,
Expand Down Expand Up @@ -119,7 +119,7 @@ fn sync_payload_test() {
};

// Run loopback serializer, handles all test validation
let mut data = [0u8; 2];
let mut data = [0u8; 1];
loopback_serializer(buffer, &mut data);
}

Expand All @@ -146,7 +146,7 @@ fn no_payload_test() {
};

// Run loopback serializer, handles all test validation
let mut data = [0u8; 5];
let mut data = [0u8; 4];
loopback_serializer(buffer, &mut data);
}

Expand All @@ -171,7 +171,7 @@ fn single_byte_payload_test() {
};

// Run loopback serializer, handles all test validation
let mut data = [0u8; 6];
let mut data = [0u8; 5];
loopback_serializer(buffer, &mut data);
}

Expand All @@ -196,7 +196,7 @@ fn full_packet_payload_test() {
};

// Run loopback serializer, handles all test validation
let mut data = [0u8; 65];
let mut data = [0u8; 64];
loopback_serializer(buffer, &mut data);
}

Expand All @@ -221,7 +221,7 @@ fn two_packet_continued_payload_test() {
};

// Run loopback serializer, handles all test validation
let mut data = [0u8; 128];
let mut data = [0u8; 118];
loopback_serializer(buffer, &mut data);
}

Expand All @@ -246,7 +246,7 @@ fn three_packet_continued_payload_test() {
};

// Run loopback serializer, handles all test validation
let mut data = [0u8; 200];
let mut data = [0u8; 182];
loopback_serializer(buffer, &mut data);
}

Expand All @@ -270,7 +270,7 @@ fn four_packet_continued_payload_test() {
};

// Run loopback serializer, handles all test validation
let mut data = [0u8; 257];
let mut data = [0u8; 256];
loopback_serializer(buffer, &mut data);
}

Expand Down
2 changes: 1 addition & 1 deletion src/device/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl HidIoEndpoint {

pub fn send_packet(
&mut self,
mut packet: mailbox::HidIoPacketBuffer,
packet: mailbox::HidIoPacketBuffer,
) -> Result<(), std::io::Error> {
debug!(
"Sending {:x?} len:{} chunk:{}",
Expand Down

0 comments on commit 8d18fe9

Please sign in to comment.