From 571945f0933ed74ead9c5bed1d76fe9e8d67a61d Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Sat, 20 Jun 2020 10:29:38 +0200 Subject: [PATCH 1/6] Use a struct to initialise blocking i2c --- src/i2c.rs | 82 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 25 deletions(-) diff --git a/src/i2c.rs b/src/i2c.rs index 37e8d0ea..2e5d7979 100644 --- a/src/i2c.rs +++ b/src/i2c.rs @@ -100,7 +100,7 @@ pub struct I2c { pub struct BlockingI2c { nb: I2c, start_timeout: u32, - start_retries: u8, + start_attempts: u8, addr_timeout: u32, data_timeout: u32, } @@ -123,19 +123,54 @@ impl I2c { } } +/// Timeouts for different stages of the communication +pub struct BlockingTimeouts { + /// How many times to attempt starting the transmission before giving up + pub start_attempts: u8, + pub start_us: u32, + pub addr_us: u32, + pub data_us: u32 +} + +impl Default for BlockingTimeouts { + // TODO: Are these sane defaults + /// Default timeouts, waits 1 ms for all stages, and does not make multiple + /// attempts to start the transmission + fn default() -> Self { + Self { + start_attempts: 1, + start_us: 1000, + addr_us: 1000, + data_us: 1000, + } + } +} + +impl BlockingTimeouts { + pub fn start_timeout_us(self, start_us: u32) -> Self { + Self {start_us, .. self} + } + pub fn addr_timeout_us(self, addr_us: u32) -> Self { + Self {addr_us, .. self} + } + pub fn data_timeout_us(self, data_us: u32) -> Self { + Self {data_us, .. self} + } + pub fn start_attempts(self, start_attempts: u8) -> Self { + Self {start_attempts, .. self} + } +} + impl BlockingI2c { /// Creates a blocking I2C1 object on pins PB6 and PB7 or PB8 and PB9 using the embedded-hal `BlockingI2c` trait. pub fn i2c1( i2c: I2C1, pins: PINS, - mapr: &mut MAPR, mode: Mode, clocks: Clocks, + mapr: &mut MAPR, apb: &mut ::Bus, - start_timeout_us: u32, - start_retries: u8, - addr_timeout_us: u32, - data_timeout_us: u32, + timeouts: BlockingTimeouts ) -> Self where PINS: Pins, @@ -147,10 +182,10 @@ impl BlockingI2c { mode, clocks, apb, - start_timeout_us, - start_retries, - addr_timeout_us, - data_timeout_us, + timeouts.start_us, + timeouts.start_attempts, + timeouts.addr_us, + timeouts.data_us, ) } } @@ -179,10 +214,7 @@ impl BlockingI2c { mode: Mode, clocks: Clocks, apb: &mut ::Bus, - start_timeout_us: u32, - start_retries: u8, - addr_timeout_us: u32, - data_timeout_us: u32, + timeouts: BlockingTimeouts ) -> Self where PINS: Pins, @@ -193,10 +225,10 @@ impl BlockingI2c { mode, clocks, apb, - start_timeout_us, - start_retries, - addr_timeout_us, - data_timeout_us, + timeouts.start_us, + timeouts.start_attempts, + timeouts.addr_us, + timeouts.data_us, ) } } @@ -206,7 +238,7 @@ fn blocking_i2c( i2c: I2c, clocks: Clocks, start_timeout_us: u32, - start_retries: u8, + start_attempts: u8, addr_timeout_us: u32, data_timeout_us: u32, ) -> BlockingI2c { @@ -214,7 +246,7 @@ fn blocking_i2c( BlockingI2c { nb: i2c, start_timeout: start_timeout_us * sysclk_mhz, - start_retries, + start_attempts, addr_timeout: addr_timeout_us * sysclk_mhz, data_timeout: data_timeout_us * sysclk_mhz, } @@ -386,21 +418,21 @@ macro_rules! hal { clocks: Clocks, apb: &mut <$I2CX as RccBus>::Bus, start_timeout_us: u32, - start_retries: u8, + start_attempts: u8, addr_timeout_us: u32, data_timeout_us: u32 ) -> Self { blocking_i2c(I2c::$i2cX(i2c, pins, mode, clocks, apb), - clocks, start_timeout_us, start_retries, + clocks, start_timeout_us, start_attempts, addr_timeout_us, data_timeout_us) } fn send_start_and_wait(&mut self) -> NbResult<(), Error> { // According to http://www.st.com/content/ccc/resource/technical/document/errata_sheet/f5/50/c9/46/56/db/4a/f6/CD00197763.pdf/files/CD00197763.pdf/jcr:content/translations/en.CD00197763.pdf // 2.14.4 Wrong behavior of I2C peripheral in master mode after a misplaced STOP - let mut retries_left = self.start_retries; + let mut attempts_left = self.start_attempts; let mut last_ret: NbResult<(), Error> = Err(WouldBlock); - while retries_left > 0 { + while attempts_left > 0 { self.nb.send_start(); last_ret = busy_wait_cycles!(self.nb.wait_after_sent_start(), self.start_timeout); if last_ret.is_err() { @@ -408,7 +440,7 @@ macro_rules! hal { } else { break; } - retries_left -= 1; + attempts_left -= 1; } last_ret } From 699f61f360683a6c9ee9454cb12cc5bbdeb00962 Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Sat, 20 Jun 2020 10:37:31 +0200 Subject: [PATCH 2/6] Move timeouts before mut refs to registers --- src/i2c.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/i2c.rs b/src/i2c.rs index 2e5d7979..6b6aa2a3 100644 --- a/src/i2c.rs +++ b/src/i2c.rs @@ -168,9 +168,9 @@ impl BlockingI2c { pins: PINS, mode: Mode, clocks: Clocks, + timeouts: BlockingTimeouts, mapr: &mut MAPR, apb: &mut ::Bus, - timeouts: BlockingTimeouts ) -> Self where PINS: Pins, @@ -213,8 +213,8 @@ impl BlockingI2c { pins: PINS, mode: Mode, clocks: Clocks, + timeouts: BlockingTimeouts, apb: &mut ::Bus, - timeouts: BlockingTimeouts ) -> Self where PINS: Pins, From c0cf3c1c8ab9c9e9af5fa6d787ef8a2d895f378d Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Sat, 20 Jun 2020 10:37:47 +0200 Subject: [PATCH 3/6] rustfmt --- src/i2c.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/i2c.rs b/src/i2c.rs index 6b6aa2a3..d2292deb 100644 --- a/src/i2c.rs +++ b/src/i2c.rs @@ -129,7 +129,7 @@ pub struct BlockingTimeouts { pub start_attempts: u8, pub start_us: u32, pub addr_us: u32, - pub data_us: u32 + pub data_us: u32, } impl Default for BlockingTimeouts { @@ -148,16 +148,19 @@ impl Default for BlockingTimeouts { impl BlockingTimeouts { pub fn start_timeout_us(self, start_us: u32) -> Self { - Self {start_us, .. self} + Self { start_us, ..self } } pub fn addr_timeout_us(self, addr_us: u32) -> Self { - Self {addr_us, .. self} + Self { addr_us, ..self } } pub fn data_timeout_us(self, data_us: u32) -> Self { - Self {data_us, .. self} + Self { data_us, ..self } } pub fn start_attempts(self, start_attempts: u8) -> Self { - Self {start_attempts, .. self} + Self { + start_attempts, + ..self + } } } From 60ac07cb2a7080e410ac2f3b885dad5c702b9d73 Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Sat, 20 Jun 2020 11:12:09 +0200 Subject: [PATCH 4/6] Use statically typed time consistently --- src/i2c.rs | 67 +++++++++++++++++++++++++------------------------ src/time.rs | 24 +++++++++--------- src/watchdog.rs | 16 ++++++------ 3 files changed, 54 insertions(+), 53 deletions(-) diff --git a/src/i2c.rs b/src/i2c.rs index d2292deb..825f3886 100644 --- a/src/i2c.rs +++ b/src/i2c.rs @@ -1,16 +1,17 @@ -//! Inter-Integrated Circuit (I2C) bus - +/** +Inter-Integrated Circuit (I2C) bus +*/ // This document describes a correct i2c implementation and is what // parts of this code is based on // https://www.st.com/content/ccc/resource/technical/document/application_note/5d/ae/a3/6f/08/69/4e/9b/CD00209826.pdf/files/CD00209826.pdf/jcr:content/translations/en.CD00209826.pdf - use crate::afio::MAPR; use crate::gpio::gpiob::{PB10, PB11, PB6, PB7, PB8, PB9}; use crate::gpio::{Alternate, OpenDrain}; use crate::hal::blocking::i2c::{Read, Write, WriteRead}; use crate::pac::{DWT, I2C1, I2C2}; +use crate::prelude::*; use crate::rcc::{sealed::RccBus, Clocks, Enable, GetBusFreq, Reset}; -use crate::time::Hertz; +use crate::time::{Hertz, Microseconds}; use nb::Error::{Other, WouldBlock}; use nb::{Error as NbError, Result as NbResult}; @@ -127,9 +128,9 @@ impl I2c { pub struct BlockingTimeouts { /// How many times to attempt starting the transmission before giving up pub start_attempts: u8, - pub start_us: u32, - pub addr_us: u32, - pub data_us: u32, + pub start: Microseconds, + pub addr: Microseconds, + pub data: Microseconds, } impl Default for BlockingTimeouts { @@ -139,22 +140,22 @@ impl Default for BlockingTimeouts { fn default() -> Self { Self { start_attempts: 1, - start_us: 1000, - addr_us: 1000, - data_us: 1000, + start: 1000.us(), + addr: 1000.us(), + data: 1000.us(), } } } impl BlockingTimeouts { - pub fn start_timeout_us(self, start_us: u32) -> Self { - Self { start_us, ..self } + pub fn start_timeout(self, start: Microseconds) -> Self { + Self { start, ..self } } - pub fn addr_timeout_us(self, addr_us: u32) -> Self { - Self { addr_us, ..self } + pub fn addr_timeout(self, addr: Microseconds) -> Self { + Self { addr, ..self } } - pub fn data_timeout_us(self, data_us: u32) -> Self { - Self { data_us, ..self } + pub fn data_timeout(self, data: Microseconds) -> Self { + Self { data, ..self } } pub fn start_attempts(self, start_attempts: u8) -> Self { Self { @@ -185,10 +186,10 @@ impl BlockingI2c { mode, clocks, apb, - timeouts.start_us, + timeouts.start, timeouts.start_attempts, - timeouts.addr_us, - timeouts.data_us, + timeouts.addr, + timeouts.data, ) } } @@ -228,10 +229,10 @@ impl BlockingI2c { mode, clocks, apb, - timeouts.start_us, + timeouts.start, timeouts.start_attempts, - timeouts.addr_us, - timeouts.data_us, + timeouts.addr, + timeouts.data, ) } } @@ -240,18 +241,18 @@ impl BlockingI2c { fn blocking_i2c( i2c: I2c, clocks: Clocks, - start_timeout_us: u32, + start_timeout: Microseconds, start_attempts: u8, - addr_timeout_us: u32, - data_timeout_us: u32, + addr_timeout: Microseconds, + data_timeout: Microseconds, ) -> BlockingI2c { let sysclk_mhz = clocks.sysclk().0 / 1_000_000; BlockingI2c { nb: i2c, - start_timeout: start_timeout_us * sysclk_mhz, + start_timeout: start_timeout.0 * sysclk_mhz, start_attempts, - addr_timeout: addr_timeout_us * sysclk_mhz, - data_timeout: data_timeout_us * sysclk_mhz, + addr_timeout: addr_timeout.0 * sysclk_mhz, + data_timeout: data_timeout.0 * sysclk_mhz, } } @@ -420,14 +421,14 @@ macro_rules! hal { mode: Mode, clocks: Clocks, apb: &mut <$I2CX as RccBus>::Bus, - start_timeout_us: u32, + start_timeout: Microseconds, start_attempts: u8, - addr_timeout_us: u32, - data_timeout_us: u32 + addr_timeout: Microseconds, + data_timeout: Microseconds ) -> Self { blocking_i2c(I2c::$i2cX(i2c, pins, mode, clocks, apb), - clocks, start_timeout_us, start_attempts, - addr_timeout_us, data_timeout_us) + clocks, start_timeout, start_attempts, + addr_timeout, data_timeout) } fn send_start_and_wait(&mut self) -> NbResult<(), Error> { diff --git a/src/time.rs b/src/time.rs index 6d37382b..b7e24b03 100644 --- a/src/time.rs +++ b/src/time.rs @@ -100,10 +100,10 @@ pub struct MegaHertz(pub u32); /// Time unit #[derive(PartialEq, PartialOrd, Clone, Copy)] -pub struct MilliSeconds(pub u32); +pub struct Milliseconds(pub u32); #[derive(PartialEq, PartialOrd, Clone, Copy)] -pub struct MicroSeconds(pub u32); +pub struct Microseconds(pub u32); /// Extension trait that adds convenience methods to the `u32` type pub trait U32Ext { @@ -119,11 +119,11 @@ pub trait U32Ext { /// Wrap in `MegaHertz` fn mhz(self) -> MegaHertz; - /// Wrap in `MilliSeconds` - fn ms(self) -> MilliSeconds; + /// Wrap in `Milliseconds` + fn ms(self) -> Milliseconds; - /// Wrap in `MicroSeconds` - fn us(self) -> MicroSeconds; + /// Wrap in `Microseconds` + fn us(self) -> Microseconds; } impl U32Ext for u32 { @@ -143,12 +143,12 @@ impl U32Ext for u32 { MegaHertz(self) } - fn ms(self) -> MilliSeconds { - MilliSeconds(self) + fn ms(self) -> Milliseconds { + Milliseconds(self) } - fn us(self) -> MicroSeconds { - MicroSeconds(self) + fn us(self) -> Microseconds { + Microseconds(self) } } @@ -170,13 +170,13 @@ impl From for KiloHertz { } } -impl Into for MilliSeconds { +impl Into for Milliseconds { fn into(self) -> Hertz { Hertz(1_000 / self.0) } } -impl Into for MicroSeconds { +impl Into for Microseconds { fn into(self) -> Hertz { Hertz(1_000_000 / self.0) } diff --git a/src/watchdog.rs b/src/watchdog.rs index f1bf3888..84913f28 100644 --- a/src/watchdog.rs +++ b/src/watchdog.rs @@ -3,7 +3,7 @@ use crate::{ hal::watchdog::{Watchdog, WatchdogEnable}, pac::{DBGMCU as DBG, IWDG}, - time::MilliSeconds, + time::Milliseconds, }; /// Wraps the Independent Watchdog (IWDG) peripheral @@ -29,15 +29,15 @@ impl IndependentWatchdog { dbg.cr.modify(|_, w| w.dbg_iwdg_stop().bit(stop)); } - fn setup(&self, timeout_ms: u32) { + fn setup(&self, timeout: Milliseconds) { let mut pr = 0; - while pr < MAX_PR && Self::timeout_period(pr, MAX_RL) < timeout_ms { + while pr < MAX_PR && Self::timeout_period(pr, MAX_RL) < timeout.0 { pr += 1; } let max_period = Self::timeout_period(pr, MAX_RL); let max_rl = u32::from(MAX_RL); - let rl = (timeout_ms * max_rl / max_period).min(max_rl) as u16; + let rl = (timeout.0 * max_rl / max_period).min(max_rl) as u16; self.access_registers(|iwdg| { iwdg.pr.modify(|_, w| w.pr().bits(pr)); @@ -50,13 +50,13 @@ impl IndependentWatchdog { } /// Returns the interval in ms - pub fn interval(&self) -> MilliSeconds { + pub fn interval(&self) -> Milliseconds { while self.is_pr_updating() {} let pr = self.iwdg.pr.read().pr().bits(); let rl = self.iwdg.rlr.read().rl().bits(); let ms = Self::timeout_period(pr, rl); - MilliSeconds(ms) + Milliseconds(ms) } /// pr: Prescaler divider bits, rl: reload value @@ -89,10 +89,10 @@ impl IndependentWatchdog { } impl WatchdogEnable for IndependentWatchdog { - type Time = MilliSeconds; + type Time = Milliseconds; fn start>(&mut self, period: T) { - self.setup(period.into().0); + self.setup(period.into()); self.iwdg.kr.write(|w| unsafe { w.key().bits(KR_START) }); } From 807f6daf853a56e53ae170a46d721afc2c0a16e9 Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Sat, 20 Jun 2020 14:49:53 +0200 Subject: [PATCH 5/6] Revert multiline comment --- src/i2c.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/i2c.rs b/src/i2c.rs index 825f3886..5084c7e1 100644 --- a/src/i2c.rs +++ b/src/i2c.rs @@ -1,6 +1,4 @@ -/** -Inter-Integrated Circuit (I2C) bus -*/ +//! Inter-Integrated Circuit (I2C) bus // This document describes a correct i2c implementation and is what // parts of this code is based on // https://www.st.com/content/ccc/resource/technical/document/application_note/5d/ae/a3/6f/08/69/4e/9b/CD00209826.pdf/files/CD00209826.pdf/jcr:content/translations/en.CD00209826.pdf From 0853d333bd0b8f202dd71a5cdbb1388c51214a9f Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Sat, 20 Jun 2020 19:06:00 +0200 Subject: [PATCH 6/6] Add changelog entry --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a7c0901..28079561 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Breaking changes + +- Use a struct for `BlockingI2C` timeout specification +- Rename `MicroSeconds` and `MilliSeconds` to `Microseconds` and `Milliseconds` +- Replace functions taking `u32` with time units by the corresponding `Microseconds` or `Millisecond` structs + ## [v0.6.0] - 2020-06-06 ### Breaking changes