From aaeefc9faf022c3ef155ea688e4b6a3677829af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20H=C3=BCbener?= <50232924+teamplayer3@users.noreply.github.com> Date: Mon, 20 Dec 2021 14:11:28 +0100 Subject: [PATCH] switch from generic duration to defined type (#98) * switch to duration type from time.rs * fixes for duration type * export streaming_iter trait for now * bumped versions of cortex crates * unused marked * clean up * removed streaming iter dep * cleanup * add deny warnings * removed generic duration * add deny warnings * fixed clippy errors * added default constructor --- examples/basic/Cargo.toml | 1 - examples/basic/src/main.rs | 26 +++++++++------ examples/embedded/Cargo.toml | 4 +-- examples/embedded/src/allocator.rs | 6 ++-- examples/embedded/src/logging.rs | 1 + examples/embedded/src/main.rs | 33 +++++++------------ examples/embedded/src/util.rs | 16 ++++----- uavcan/src/crc16.rs | 9 ++--- uavcan/src/lib.rs | 20 ++++++++---- uavcan/src/session/heap_based.rs | 47 ++++++++++++++------------- uavcan/src/session/mod.rs | 5 +-- uavcan/src/session/std_vec.rs | 36 +++++++------------- uavcan/src/time.rs | 3 +- uavcan/src/transport/can/bitfields.rs | 2 ++ uavcan/src/transport/can/mod.rs | 23 +++++++------ 15 files changed, 115 insertions(+), 117 deletions(-) diff --git a/examples/basic/Cargo.toml b/examples/basic/Cargo.toml index b5638789..9ec49a43 100644 --- a/examples/basic/Cargo.toml +++ b/examples/basic/Cargo.toml @@ -13,7 +13,6 @@ 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" diff --git a/examples/basic/src/main.rs b/examples/basic/src/main.rs index 926ac94e..d38d245a 100644 --- a/examples/basic/src/main.rs +++ b/examples/basic/src/main.rs @@ -1,18 +1,22 @@ +#![deny(warnings)] + 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}; -use uavcan::{transfer::Transfer, types::TransferId, Node, Priority, Subscription, TransferKind}; +use uavcan::{ + session::StdVecSessionManager, + time::StdClock, + transfer::Transfer, + transport::can::{Can, CanFrame as UavcanFrame, CanMetadata}, + types::TransferId, + Node, Priority, StreamingIterator, Subscription, TransferKind, +}; use arrayvec::ArrayVec; use socketcan::{CANFrame, CANSocket}; fn main() { let clock = StdClock::new(); - let mut session_manager = StdVecSessionManager::::new(); + let mut session_manager = StdVecSessionManager::::new(); session_manager .subscribe(Subscription::new( TransferKind::Message, @@ -71,7 +75,7 @@ fn main() { for byte in xfer.payload { print!("0x{:02x} ", byte); } - println!(""); + println!(); } TransferKind::Request => { println!("Request Received!"); @@ -107,8 +111,10 @@ fn main() { 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(); + sock.write_frame( + &CANFrame::new(frame.id.as_raw(), &frame.payload, false, false).unwrap(), + ) + .unwrap(); //print!("Can frame {}: ", i); //for byte in &frame.payload { diff --git a/examples/embedded/Cargo.toml b/examples/embedded/Cargo.toml index c6e1351f..f13dd8ac 100644 --- a/examples/embedded/Cargo.toml +++ b/examples/embedded/Cargo.toml @@ -7,8 +7,8 @@ edition = "2021" [dependencies] -cortex-m = "0.6.0" -cortex-m-rt = "0.6.10" +cortex-m = "0.7.1" +cortex-m-rt = "0.7.1" stm32g4xx-hal = {version = "0.0.0", git = "https://github.com/stm32-rs/stm32g4xx-hal", features = ["rt", "stm32g431"] } alloc-cortex-m = "0.4.1" diff --git a/examples/embedded/src/allocator.rs b/examples/embedded/src/allocator.rs index 1f89e814..268a74ab 100644 --- a/examples/embedded/src/allocator.rs +++ b/examples/embedded/src/allocator.rs @@ -7,9 +7,11 @@ extern crate alloc; pub struct MyAllocator(Mutex>>); impl MyAllocator { - pub const INIT: MyAllocator = MyAllocator(Mutex::new(RefCell::new(Tlsf::INIT))); + pub const fn init() -> Self { + Self(Mutex::new(RefCell::new(Tlsf::INIT))) + } - pub unsafe fn init(&self, pool: *mut u8, len: usize) -> usize { + pub unsafe fn set_pool(&self, pool: *mut u8, len: usize) -> usize { cortex_m::interrupt::free(|cs| { let mut tlsf = self.0.borrow(cs).borrow_mut(); diff --git a/examples/embedded/src/logging.rs b/examples/embedded/src/logging.rs index f5592342..e02990ce 100644 --- a/examples/embedded/src/logging.rs +++ b/examples/embedded/src/logging.rs @@ -20,6 +20,7 @@ defmt::timestamp!("{=usize}", { }); /// Terminates the application and makes `probe-run` exit with exit-code = 0 +#[allow(unused)] pub fn exit() -> ! { loop { cortex_m::asm::bkpt(); diff --git a/examples/embedded/src/main.rs b/examples/embedded/src/main.rs index 233e257d..259c6da9 100644 --- a/examples/embedded/src/main.rs +++ b/examples/embedded/src/main.rs @@ -1,5 +1,6 @@ #![no_main] #![no_std] +#![deny(warnings)] // to use the global allocator #![feature(alloc_error_handler)] @@ -7,19 +8,11 @@ #[macro_use] extern crate std; -#[cfg(feature = "logging")] -mod logging; -#[cfg(feature = "logging")] -use defmt::info; - mod allocator; mod clock; +mod logging; mod util; - -#[cfg(not(feature = "logging"))] -use panic_halt as _; - use core::{ alloc::Layout, mem::MaybeUninit, @@ -29,10 +22,9 @@ use core::{ use allocator::MyAllocator; use clock::StmClock; -use cortex_m as _; use cortex_m_rt::entry; -use embedded_time::{duration::Milliseconds, Clock}; +use embedded_time::Clock; use hal::{ delay::SYSTDelayExt, fdcan::{ @@ -48,7 +40,6 @@ use hal::{ prelude::*, rcc::{Config, PLLSrc, PllConfig, Rcc, RccExt, SysClockSrc}, stm32::{Peripherals, FDCAN1}, - timer::MonoTimer, }; use stm32g4xx_hal as hal; @@ -56,7 +47,7 @@ use uavcan::{ session::HeapSessionManager, transfer::Transfer, transport::can::{Can, CanMetadata}, - Node, Priority, Subscription, TransferKind, + Node, Priority, StreamingIterator, Subscription, TransferKind, }; use util::insert_u8_array_in_u32_array; @@ -64,14 +55,14 @@ use util::insert_u8_array_in_u32_array; static mut POOL: MaybeUninit<[u8; 1024]> = MaybeUninit::uninit(); #[global_allocator] -static ALLOCATOR: MyAllocator = MyAllocator::INIT; +static ALLOCATOR: MyAllocator = MyAllocator::init(); #[entry] fn main() -> ! { // init heap let cursor = unsafe { POOL.as_mut_ptr() } as *mut u8; let size = 1024; - unsafe { ALLOCATOR.init(cursor, size) }; + unsafe { ALLOCATOR.set_pool(cursor, size) }; // define peripherals of the board let dp = Peripherals::take().unwrap(); @@ -119,12 +110,10 @@ fn main() -> ! { can.into_normal() }; - let measure_clock = MonoTimer::new(cp.DWT, cp.DCB, &rcc.clocks); - // init clock let clock = StmClock::new(dp.TIM7, &rcc.clocks); - let mut session_manager = HeapSessionManager::::new(); + let mut session_manager = HeapSessionManager::::new(); session_manager .subscribe(Subscription::new( TransferKind::Message, @@ -175,15 +164,16 @@ fn main() -> ! { } pub fn publish( - node: &mut Node, StmClock>, Can, StmClock>, + node: &mut Node, Can, StmClock>, transfer: Transfer, can: &mut FdCan, ) { - for frame in node.transmit(&transfer).unwrap() { + let mut iter = node.transmit(&transfer).unwrap(); + while let Some(frame) = iter.next() { let header = TxFrameHeader { bit_rate_switching: false, frame_format: hal::fdcan::frame::FrameFormat::Standard, - id: Id::Extended(ExtendedId::new(frame.id).unwrap()), + id: Id::Extended(ExtendedId::new(frame.id.as_raw()).unwrap()), len: frame.payload.len() as u8, marker: None, }; @@ -210,6 +200,7 @@ fn config_rcc(rcc: Rcc) -> Rcc { ) } +#[allow(clippy::empty_loop)] #[alloc_error_handler] fn oom(_: Layout) -> ! { loop {} diff --git a/examples/embedded/src/util.rs b/examples/embedded/src/util.rs index 831c1b26..aa6419e6 100644 --- a/examples/embedded/src/util.rs +++ b/examples/embedded/src/util.rs @@ -4,10 +4,10 @@ pub fn insert_u8_array_in_u32_array(u8_array: &[u8], u32_array: &mut [u32]) { let u32_final_len = (u8_array.len() + 3) / 4; // only iterate over n - 1 - for i in 0..u32_final_len - 1 { - let index = i * 4; + for (index, item) in u32_array.iter_mut().enumerate().take(u32_final_len - 1) { + let index = index * 4; let val = slice_into_u32(&u8_array[index..index + 4]); - u32_array[i] = val; + *item = val; } let remaining = match u8_array.len() % 4 { 0 => 4, @@ -25,16 +25,16 @@ pub fn insert_u8_array_in_u32_array(u8_array: &[u8], u32_array: &mut [u32]) { fn slice_into_u32(slice: &[u8]) -> u32 { // TODO nicer algorithm match slice.len() { - 1 => return (slice[0] as u32) << 24, - 2 => return (slice[0] as u32) << 24 | (slice[1] as u32) << 16, - 3 => return (slice[0] as u32) << 24 | (slice[1] as u32) << 16 | (slice[2] as u32) << 8, + 1 => (slice[0] as u32) << 24, + 2 => (slice[0] as u32) << 24 | (slice[1] as u32) << 16, + 3 => (slice[0] as u32) << 24 | (slice[1] as u32) << 16 | (slice[2] as u32) << 8, 4 => { - return (slice[0] as u32) << 24 + (slice[0] as u32) << 24 | (slice[1] as u32) << 16 | (slice[2] as u32) << 8 | (slice[3] as u32) } - _ => return 0, + _ => 0, } } diff --git a/uavcan/src/crc16.rs b/uavcan/src/crc16.rs index cf2406a3..e5f84dcc 100644 --- a/uavcan/src/crc16.rs +++ b/uavcan/src/crc16.rs @@ -53,7 +53,7 @@ impl Crc16 { /// Process the current crc sum further with the supplied data. pub fn digest>(&mut self, data: &T) { for n in data.as_ref().iter().copied() { - let index = ((self.0 >> u16::from(Self::BITS - 8)) as u8 ^ n) as usize; + let index = ((self.0 >> (Self::BITS - 8)) as u8 ^ n) as usize; self.0 = (self.0 << 8) ^ NO_REF_16_1021[index]; } } @@ -62,15 +62,12 @@ impl Crc16 { pub fn get_crc(&self) -> u16 { let final_xor = 0x0000; - let sum = (self.0 ^ final_xor) & Crc16::mask(); - - sum + (self.0 ^ final_xor) & Crc16::mask() } const fn mask() -> u16 { let high_bit = 1 << (Self::BITS - 1); - let mask = ((high_bit - 1) << 1) | 1; - mask + ((high_bit - 1) << 1) | 1 } } diff --git a/uavcan/src/lib.rs b/uavcan/src/lib.rs index 86236233..e6f29b91 100644 --- a/uavcan/src/lib.rs +++ b/uavcan/src/lib.rs @@ -22,6 +22,7 @@ //! this. I can see issues with this running into issues in multi-threaded //! environments, but I'll get to those when I get to them. #![no_std] +#![deny(warnings)] #![feature(generic_associated_types)] #![feature(test)] @@ -45,10 +46,12 @@ pub mod transfer; pub mod transport; pub mod types; -use embedded_time::fixed_point::FixedPoint; pub use node::Node; +use time::Duration; pub use transfer::TransferKind; +pub use streaming_iterator::StreamingIterator; + mod internal; mod node; pub mod session; @@ -107,15 +110,20 @@ pub enum Priority { } /// Simple subscription type to -pub struct Subscription { +pub struct Subscription { transfer_kind: TransferKind, port_id: PortId, extent: usize, - timeout: D, + timeout: Duration, } -impl Subscription { - pub fn new(transfer_kind: TransferKind, port_id: PortId, extent: usize, timeout: D) -> Self { +impl Subscription { + pub fn new( + transfer_kind: TransferKind, + port_id: PortId, + extent: usize, + timeout: Duration, + ) -> Self { Self { transfer_kind, port_id, @@ -125,7 +133,7 @@ impl Subscription { } } -impl PartialEq for Subscription { +impl PartialEq for Subscription { fn eq(&self, other: &Self) -> bool { self.transfer_kind == other.transfer_kind && self.port_id == other.port_id } diff --git a/uavcan/src/session/heap_based.rs b/uavcan/src/session/heap_based.rs index 426cf920..8923809a 100644 --- a/uavcan/src/session/heap_based.rs +++ b/uavcan/src/session/heap_based.rs @@ -3,9 +3,7 @@ //! This is intended to be the lowest-friction interface to get //! started, both for library development and eventually for using the library. -extern crate alloc; - -use embedded_time::{duration::Duration, fixed_point::FixedPoint, Clock}; +use embedded_time::Clock; use crate::session::*; use crate::types::NodeId; @@ -54,24 +52,21 @@ where } /// Internal subscription object. Contains hash map of sessions. -struct Subscription +struct Subscription where T: crate::transport::SessionMetadata, - D: Duration + FixedPoint, C: Clock, { - sub: crate::Subscription, + sub: crate::Subscription, sessions: BTreeMap>, } -impl Subscription +impl Subscription where T: crate::transport::SessionMetadata, - D: Duration + FixedPoint, C: Clock, - ::T: From<::T>, { - pub fn new(sub: crate::Subscription) -> Self { + pub fn new(sub: crate::Subscription) -> Self { Self { sub, sessions: BTreeMap::new(), @@ -123,6 +118,7 @@ where if let Some(len) = session.md.update(&frame) { // Truncate payload if subscription extent is less than the incoming data + // TODO not working. let payload_to_copy = if session.payload.len() + len > self.sub.extent { session.payload.len() + len - self.sub.extent } else { @@ -152,21 +148,18 @@ where /// SessionManager based on full std support. Meant to be lowest /// barrier to entry and greatest flexibility at the cost of resource usage /// and not being no_std. -pub struct HeapSessionManager +pub struct HeapSessionManager where T: crate::transport::SessionMetadata, - D: Duration + FixedPoint, C: Clock, { - subscriptions: Vec>, + subscriptions: Vec>, } -impl HeapSessionManager +impl HeapSessionManager where T: crate::transport::SessionMetadata, C: Clock, - D: embedded_time::duration::Duration + FixedPoint, - ::T: From<::T>, { pub fn new() -> Self { Self { @@ -177,7 +170,7 @@ where /// Add a subscription pub fn subscribe( &mut self, - subscription: crate::Subscription, + subscription: crate::Subscription, ) -> Result<(), SubscriptionError> { if self.subscriptions.iter().any(|s| s.sub == subscription) { return Err(SubscriptionError::SubscriptionExists); @@ -190,7 +183,7 @@ where /// Modify subscription in place, creating a new one if not found. pub fn edit_subscription( &mut self, - subscription: crate::Subscription, + subscription: crate::Subscription, ) -> Result<(), SubscriptionError> { match self .subscriptions @@ -208,7 +201,7 @@ where /// Removes a subscription from the list. pub fn unsubscribe( &mut self, - subscription: crate::Subscription, + subscription: crate::Subscription, ) -> Result<(), SubscriptionError> { match self .subscriptions @@ -224,12 +217,22 @@ where } } -impl SessionManager for HeapSessionManager +impl Default for HeapSessionManager +where + T: crate::transport::SessionMetadata, + C: Clock, +{ + fn default() -> Self { + Self { + subscriptions: Default::default(), + } + } +} + +impl SessionManager for HeapSessionManager where T: crate::transport::SessionMetadata, C: Clock, - D: embedded_time::duration::Duration + FixedPoint, - ::T: From<::T>, { fn ingest(&mut self, frame: InternalRxFrame) -> Result>, SessionError> { match self diff --git a/uavcan/src/session/mod.rs b/uavcan/src/session/mod.rs index baf37c0b..2c6947ec 100644 --- a/uavcan/src/session/mod.rs +++ b/uavcan/src/session/mod.rs @@ -64,10 +64,7 @@ pub trait SessionManager { /// /// It's not necessary to use this in your implementation, you may have /// a more efficient way to check, but this is a spec-compliant function. - fn matches_sub( - subscription: &crate::Subscription, - frame: &InternalRxFrame, - ) -> bool { + fn matches_sub(subscription: &crate::Subscription, frame: &InternalRxFrame) -> bool { // Order is chosen to short circuit the most common inconsistencies. if frame.port_id != subscription.port_id { return false; diff --git a/uavcan/src/session/std_vec.rs b/uavcan/src/session/std_vec.rs index 19d7dcbe..eb385a3b 100644 --- a/uavcan/src/session/std_vec.rs +++ b/uavcan/src/session/std_vec.rs @@ -3,8 +3,6 @@ //! This is intended to be the lowest-friction interface to get //! started, both for library development and eventually for using the library. -use embedded_time::fixed_point::FixedPoint; - use crate::session::*; use crate::time::Timestamp; use crate::types::NodeId; @@ -43,24 +41,21 @@ where } /// Internal subscription object. Contains hash map of sessions. -struct Subscription +struct Subscription where T: crate::transport::SessionMetadata, - D: embedded_time::duration::Duration + FixedPoint, C: embedded_time::Clock, { - sub: crate::Subscription, + sub: crate::Subscription, sessions: HashMap>, } -impl Subscription +impl Subscription where T: crate::transport::SessionMetadata, - D: embedded_time::duration::Duration + FixedPoint, C: embedded_time::Clock, - ::T: From<::T>, { - pub fn new(sub: crate::Subscription) -> Self { + pub fn new(sub: crate::Subscription) -> Self { Self { sub, sessions: HashMap::new(), @@ -144,21 +139,18 @@ where /// SessionManager based on full std support. Meant to be lowest /// barrier to entry and greatest flexibility at the cost of resource usage /// and not being no_std. -pub struct StdVecSessionManager +pub struct StdVecSessionManager where T: crate::transport::SessionMetadata, - D: embedded_time::duration::Duration + FixedPoint, C: embedded_time::Clock, { - subscriptions: Vec>, + subscriptions: Vec>, } -impl StdVecSessionManager +impl StdVecSessionManager where T: crate::transport::SessionMetadata, - D: embedded_time::duration::Duration + FixedPoint, C: embedded_time::Clock, - ::T: From<::T>, { pub fn new() -> Self { Self { @@ -169,7 +161,7 @@ where /// Add a subscription pub fn subscribe( &mut self, - subscription: crate::Subscription, + subscription: crate::Subscription, ) -> Result<(), SubscriptionError> { if self.subscriptions.iter().any(|s| s.sub == subscription) { return Err(SubscriptionError::SubscriptionExists); @@ -182,7 +174,7 @@ where /// Modify subscription in place, creating a new one if not found. pub fn edit_subscription( &mut self, - subscription: crate::Subscription, + subscription: crate::Subscription, ) -> Result<(), SubscriptionError> { match self .subscriptions @@ -200,7 +192,7 @@ where /// Removes a subscription from the list. pub fn unsubscribe( &mut self, - subscription: crate::Subscription, + subscription: crate::Subscription, ) -> Result<(), SubscriptionError> { match self .subscriptions @@ -216,24 +208,20 @@ where } } -impl Default for StdVecSessionManager +impl Default for StdVecSessionManager where T: crate::transport::SessionMetadata, - D: embedded_time::duration::Duration + FixedPoint, C: embedded_time::Clock, - ::T: From<::T>, { fn default() -> Self { Self::new() } } -impl SessionManager for StdVecSessionManager +impl SessionManager for StdVecSessionManager where T: crate::transport::SessionMetadata, - D: embedded_time::duration::Duration + FixedPoint, C: embedded_time::Clock, - ::T: From<::T>, { fn ingest(&mut self, frame: InternalRxFrame) -> Result>, SessionError> { match self diff --git a/uavcan/src/time.rs b/uavcan/src/time.rs index f77f588f..c86bad7f 100644 --- a/uavcan/src/time.rs +++ b/uavcan/src/time.rs @@ -1,4 +1,5 @@ pub type Timestamp = embedded_time::Instant; +pub type Duration = embedded_time::duration::Milliseconds; #[cfg(test)] pub use test_clock::TestClock; @@ -185,7 +186,7 @@ mod std_clock { impl Default for StdClock { fn default() -> Self { - Self::new() + Self(Rc::new(RefCell::new(Instant::now()))) } } diff --git a/uavcan/src/transport/can/bitfields.rs b/uavcan/src/transport/can/bitfields.rs index 4fbf305e..381f48c9 100644 --- a/uavcan/src/transport/can/bitfields.rs +++ b/uavcan/src/transport/can/bitfields.rs @@ -37,6 +37,7 @@ bitfield! { impl CanMessageId { // TODO bounds checks (can these be auto-implemented?) + #[allow(clippy::new_ret_no_self)] pub fn new(priority: Priority, subject_id: PortId, source_id: Option) -> ExtendedId { let is_anon = source_id.is_none(); // TODO do better than XKCD 221 @@ -101,6 +102,7 @@ bitfield! { } impl CanServiceId { + #[allow(clippy::new_ret_no_self)] pub fn new( priority: Priority, is_request: bool, diff --git a/uavcan/src/transport/can/mod.rs b/uavcan/src/transport/can/mod.rs index e958ec3e..24b067fc 100644 --- a/uavcan/src/transport/can/mod.rs +++ b/uavcan/src/transport/can/mod.rs @@ -247,16 +247,16 @@ impl<'a, C: Clock> StreamingIterator for CanIter<'a, C> { self.payload_offset += bytes_left; self.crc_left = 0; unsafe { - frame.payload.push_unchecked( - TailByte::new(true, true, true, self.transfer.transfer_id).0 - ) + frame + .payload + .push_unchecked(TailByte::new(true, true, true, self.transfer.transfer_id).0) } } else { // 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.into_iter().copied()); + frame.payload.extend(out_data.iter().copied()); // Increment offset self.payload_offset += copy_len; @@ -291,12 +291,15 @@ impl<'a, C: Clock> StreamingIterator for CanIter<'a, C> { // SAFETY: should only copy at most 7 elements prior to here unsafe { - frame.payload.push_unchecked(TailByte::new( - self.is_start, - is_end, - self.toggle, - self.transfer.transfer_id, - ).0); + frame.payload.push_unchecked( + TailByte::new( + self.is_start, + is_end, + self.toggle, + self.transfer.transfer_id, + ) + .0, + ); } // Advance state of iter