Skip to content
This repository was archived by the owner on Jan 9, 2023. It is now read-only.

Commit

Permalink
use streamiterator instead of std iterator (#90)
Browse files Browse the repository at this point in the history
* added streamiter dep

* bump rust edition version

* use streamiter trait in transport

* impl streamiter for canIter

* updated to streamiter

* updated basic example
  • Loading branch information
teamplayer3 authored Dec 11, 2021
1 parent 872bbda commit 5d37215
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 42 deletions.
1 change: 1 addition & 0 deletions examples/basic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ embedded-time = "0.12.0"

# TODO: if new embedded-hal version releases, this can be changed to crates.io
embedded-hal = {version = "0.2.6", git = "https://github.com/rust-embedded/embedded-hal/", branch = "v0.2.x"}
streaming-iterator = "0.1.5"

[dependencies.uavcan]
path = "../../uavcan"
Expand Down
10 changes: 5 additions & 5 deletions examples/basic/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use embedded_hal::can::ExtendedId;
use embedded_time::duration::Milliseconds;
use embedded_time::Clock;
use streaming_iterator::StreamingIterator;
use uavcan::session::StdVecSessionManager;
use uavcan::time::StdClock;
use uavcan::transport::can::{Can, CanFrame as UavcanFrame, CanMetadata};
Expand Down Expand Up @@ -104,11 +105,10 @@ fn main() {
// unsafe { transfer_id.unchecked_add(1); }
transfer_id = (std::num::Wrapping(transfer_id) + std::num::Wrapping(1)).0;

for frame in node.transmit(&transfer).unwrap() {
sock.write_frame(
&CANFrame::new(frame.id.as_raw(), &frame.payload, false, false).unwrap(),
)
.unwrap();
let mut frame_iter = node.transmit(&transfer).unwrap();
while let Some(frame) = frame_iter.next() {
sock.write_frame(&CANFrame::new(frame.id, &frame.payload, false, false).unwrap())
.unwrap();

//print!("Can frame {}: ", i);
//for byte in &frame.payload {
Expand Down
3 changes: 2 additions & 1 deletion uavcan/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "uavcan"
authors = ["David Lenfesty <[email protected]>"]
version = "0.2.0-preview0"
edition = "2018"
edition = "2021"

description = "Full functionality reference implementation of UAVCAN/CAN in Rust"

Expand All @@ -23,6 +23,7 @@ bitfield = "0.13"
# TODO: if new embedded-hal version releases, this can be changed to crates.io
embedded-hal = {version = "0.2.6", git = "https://github.com/rust-embedded/embedded-hal/", branch = "v0.2.x"}
embedded-time = "0.12.0"
streaming-iterator = "0.1.5"

# should only in no_std, so if feature std not set - ref: https://github.com/rust-lang/cargo/issues/1839
heapless = "0.7.7"
Expand Down
56 changes: 37 additions & 19 deletions uavcan/src/transport/can/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use arrayvec::ArrayVec;
use embedded_hal::can::ExtendedId;
use embedded_time::Clock;
use num_traits::FromPrimitive;
use streaming_iterator::StreamingIterator;

use crate::time::Timestamp;
use crate::Priority;
Expand Down Expand Up @@ -152,6 +153,7 @@ pub struct CanIter<'a, C: embedded_time::Clock> {
crc: Crc16,
toggle: bool,
is_start: bool,
can_frame: Option<CanFrame<C>>,
}

impl<'a, C: embedded_time::Clock> CanIter<'a, C> {
Expand Down Expand Up @@ -205,48 +207,56 @@ impl<'a, C: embedded_time::Clock> CanIter<'a, C> {
crc: Crc16::init(),
toggle: true,
is_start: true,
can_frame: None,
})
}
}

impl<'a, C: Clock> Iterator for CanIter<'a, C> {
impl<'a, C: Clock> StreamingIterator for CanIter<'a, C> {
type Item = CanFrame<C>;

// I'm sure I could take an optimization pass at the logic here
fn next(&mut self) -> Option<Self::Item> {
let mut frame = CanFrame {
// TODO enough to use the transfer timestamp, or need actual timestamp
timestamp: self.transfer.timestamp,
id: self.frame_id,
payload: ArrayVec::new(),
};
fn get(&self) -> Option<&Self::Item> {
self.can_frame.as_ref()
}

// I'm sure I could take an optimization pass at the logic here
fn advance(&mut self) {
let bytes_left = self.transfer.payload.len() - self.payload_offset;

// Nothing left to transmit, we are done
if bytes_left == 0 && self.crc_left == 0 {
let _ = self.can_frame.take();
return;
}

let is_end = bytes_left <= 7;
let copy_len = core::cmp::min(bytes_left, 7);

// TODO enough to use the transfer timestamp, or need actual timestamp
let frame = self
.can_frame
.get_or_insert_with(|| CanFrame::new(self.transfer.timestamp, self.frame_id.as_raw()));

frame.payload.clear();

if self.is_start && is_end {
// Single frame transfer, no CRC
frame
.payload
.extend(self.transfer.payload[0..copy_len].iter().copied());
self.payload_offset += bytes_left;
self.crc_left = 0;
unsafe {
frame.payload.push_unchecked(
TailByte::new(true, true, true, self.transfer.transfer_id).0
)
}
} else {
// Nothing left to transmit, we are done
if bytes_left == 0 && self.crc_left == 0 {
return None;
}

// Handle CRC
let out_data =
&self.transfer.payload[self.payload_offset..self.payload_offset + copy_len];
self.crc.digest(out_data);
frame.payload.extend(out_data.iter().copied());
frame.payload.extend(out_data.into_iter().copied());

// Increment offset
self.payload_offset += copy_len;
Expand All @@ -261,8 +271,7 @@ impl<'a, C: Clock> Iterator for CanIter<'a, C> {
if 7 - bytes_left >= 2 {
// Iter doesn't work. Internal type is &u8 but extend
// expects u8
frame.payload.push(crc[0]);
frame.payload.push(crc[1]);
frame.payload.extend(crc.into_iter());
self.crc_left = 0;
} else {
// SAFETY: only written if we have enough space
Expand Down Expand Up @@ -295,8 +304,6 @@ impl<'a, C: Clock> Iterator for CanIter<'a, C> {
}

self.is_start = false;

Some(frame)
}

fn size_hint(&self) -> (usize, Option<usize>) {
Expand Down Expand Up @@ -327,6 +334,17 @@ pub struct CanFrame<C: embedded_time::Clock> {
pub payload: ArrayVec<[u8; 8]>,
}

impl<C: embedded_time::Clock> CanFrame<C> {
fn new(timestamp: Timestamp<C>, id: u32) -> Self {
Self {
timestamp,
// TODO get rid of this expect, it probably isn't necessary, just added quickly
id: ExtendedId::new(id).expect("invalid ID"),
payload: ArrayVec::<[u8; 8]>::new(),
}
}
}

/// Keeps track of toggle bit and CRC during frame processing.
#[derive(Debug)]
pub struct CanMetadata {
Expand Down
30 changes: 14 additions & 16 deletions uavcan/src/transport/can/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn receive_anon_frame() {
let mut frame = CanFrame {
timestamp: clock.try_now().unwrap(),
id: CanMessageId::new(Priority::Nominal, 0, None),
payload: ArrayVec::new(),
payload: arrayvec::ArrayVec::<[u8; 8]>::new(),
};

frame.payload.extend(0..5);
Expand All @@ -53,7 +53,7 @@ fn receive_message_frame() {
let mut frame = CanFrame {
timestamp: clock.try_now().unwrap(),
id: CanMessageId::new(Priority::Nominal, 0, Some(41)),
payload: ArrayVec::new(),
payload: arrayvec::ArrayVec::<[u8; 8]>::new(),
};

frame.payload.push(TailByte::new(true, true, true, 0).0);
Expand All @@ -71,7 +71,7 @@ fn receive_service_frame() {
let mut frame = CanFrame {
timestamp: clock.try_now().unwrap(),
id: CanServiceId::new(Priority::Nominal, false, 0, 42, 41),
payload: ArrayVec::new(),
payload: arrayvec::ArrayVec::<[u8; 8]>::new(),
};

frame.payload.push(TailByte::new(true, true, true, 0).0);
Expand Down Expand Up @@ -112,7 +112,7 @@ fn discard_anon_multi_frame() {
let mut frame = CanFrame {
timestamp: clock.try_now().unwrap(),
id: CanMessageId::new(Priority::Nominal, 0, None),
payload: ArrayVec::new(),
payload: arrayvec::ArrayVec::<[u8; 8]>::new(),
};

// Need to fill frame to avoid tail_byte_checks() cases
Expand Down Expand Up @@ -141,7 +141,7 @@ fn discard_misguided_service_frames() {
let mut frame = CanFrame {
timestamp: clock.try_now().unwrap(),
id: CanServiceId::new(Priority::Nominal, true, 0, 31, 41),
payload: ArrayVec::new(),
payload: arrayvec::ArrayVec::<[u8; 8]>::new(),
};

// Request
Expand Down Expand Up @@ -186,7 +186,7 @@ fn tail_byte_checks() {
let mut frame = CanFrame {
timestamp: clock.try_now().unwrap(),
id: CanMessageId::new(Priority::Nominal, 0, None),
payload: ArrayVec::new(),
payload: arrayvec::ArrayVec::<[u8; 8]>::new(),
};

frame.payload.push(TailByte::new(true, true, false, 0).0);
Expand Down Expand Up @@ -230,20 +230,18 @@ fn transfer_valid_ids() {
// but this is the most ergonomic entry point for this test.

// Anonymous message
let frame: CanFrame<TestClock<u32>> = CanIter::new(&transfer, None)
.unwrap()
.next()
.expect("Failed to create iter");
let id = CanMessageId::from(frame.id);
let mut can_iter = CanIter::new(&transfer, None).unwrap();
let frame: &CanFrame<TestClock<u32>> = can_iter.next().expect("Failed to create iter");
let id = CanMessageId(frame.id.as_raw());
assert!(id.is_message());
assert!(id.is_anon());
assert!(id.subject_id() == 0);
assert!(id.priority() == Priority::Nominal as u8);
// Source ID should be random, not sure how to handle this...

let frame: CanFrame<TestClock<u32>> =
CanIter::new(&transfer, Some(12)).unwrap().next().expect("");
let id = CanMessageId::from(frame.id);
let mut can_iter = CanIter::new(&transfer, Some(12)).unwrap();
let frame: &CanFrame<TestClock<u32>> = can_iter.next().expect("");
let id = CanMessageId(frame.id.as_raw());
assert!(id.is_message());
assert!(!id.is_anon());
assert!(id.subject_id() == 0);
Expand All @@ -257,8 +255,8 @@ fn transfer_valid_ids() {
}

/// Checks that the iterator produces the expected number of frames.
fn assert_frame_count(iter: CanIter<TestClock>, mut expected: usize) {
for _frame in iter {
fn assert_frame_count(mut iter: CanIter<TestClock>, mut expected: usize) {
while let Some(_frame) = iter.next() {
assert!(expected > 0);
expected -= 1;
}
Expand Down
4 changes: 3 additions & 1 deletion uavcan/src/transport/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
// Declaring all of the sub transport modules here.
pub mod can;

use streaming_iterator::StreamingIterator;

use crate::internal::InternalRxFrame;
use crate::NodeId;
use crate::{RxError, TxError};
Expand Down Expand Up @@ -37,7 +39,7 @@ pub trait Transport<C: embedded_time::Clock> {
type Frame;
// TODO does this properly describe the lifetime semantics of this type?
// I implemented this as a quick fix to get the PR tests going - David
type FrameIter<'a>: Iterator where C: 'a;
type FrameIter<'a>: StreamingIterator where C: 'a;

/// Process a frame, returning the internal transport-independant representation,
/// or errors if invalid.
Expand Down

0 comments on commit 5d37215

Please sign in to comment.