Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use statically typed time consistently #231

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
110 changes: 72 additions & 38 deletions src/i2c.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
//! Inter-Integrated Circuit (I2C) bus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for the comment style change? C++ comments are not so typical in the Rust world.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I think I started adding a larger doc chunk, but then changed my mind about what I wanted to do. I'll revert it until I get around to adding i2c docs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even then it should be possible to use Rust style comments. ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but I kind of dislike that for large blocks, especially inserting new lines


// 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};

Expand Down Expand Up @@ -100,7 +99,7 @@ pub struct I2c<I2C, PINS> {
pub struct BlockingI2c<I2C, PINS> {
nb: I2c<I2C, PINS>,
start_timeout: u32,
start_retries: u8,
start_attempts: u8,
addr_timeout: u32,
data_timeout: u32,
}
Expand All @@ -123,19 +122,57 @@ impl<PINS> I2c<I2C1, PINS> {
}
}

/// 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: Microseconds,
pub addr: Microseconds,
pub data: Microseconds,
}

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: 1000.us(),
addr: 1000.us(),
data: 1000.us(),
}
}
}

impl BlockingTimeouts {
pub fn start_timeout(self, start: Microseconds) -> Self {
Self { start, ..self }
}
pub fn addr_timeout(self, addr: Microseconds) -> Self {
Self { addr, ..self }
}
pub fn data_timeout(self, data: Microseconds) -> Self {
Self { data, ..self }
}
pub fn start_attempts(self, start_attempts: u8) -> Self {
Self {
start_attempts,
..self
}
}
}

impl<PINS> BlockingI2c<I2C1, PINS> {
/// 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,
timeouts: BlockingTimeouts,
mapr: &mut MAPR,
apb: &mut <I2C1 as RccBus>::Bus,
start_timeout_us: u32,
start_retries: u8,
addr_timeout_us: u32,
data_timeout_us: u32,
) -> Self
where
PINS: Pins<I2C1>,
Expand All @@ -147,10 +184,10 @@ impl<PINS> BlockingI2c<I2C1, PINS> {
mode,
clocks,
apb,
start_timeout_us,
start_retries,
addr_timeout_us,
data_timeout_us,
timeouts.start,
timeouts.start_attempts,
timeouts.addr,
timeouts.data,
)
}
}
Expand Down Expand Up @@ -178,11 +215,8 @@ impl<PINS> BlockingI2c<I2C2, PINS> {
pins: PINS,
mode: Mode,
clocks: Clocks,
timeouts: BlockingTimeouts,
apb: &mut <I2C2 as RccBus>::Bus,
start_timeout_us: u32,
start_retries: u8,
addr_timeout_us: u32,
data_timeout_us: u32,
) -> Self
where
PINS: Pins<I2C2>,
Expand All @@ -193,10 +227,10 @@ impl<PINS> BlockingI2c<I2C2, PINS> {
mode,
clocks,
apb,
start_timeout_us,
start_retries,
addr_timeout_us,
data_timeout_us,
timeouts.start,
timeouts.start_attempts,
timeouts.addr,
timeouts.data,
)
}
}
Expand All @@ -205,18 +239,18 @@ impl<PINS> BlockingI2c<I2C2, PINS> {
fn blocking_i2c<I2C, PINS>(
i2c: I2c<I2C, PINS>,
clocks: Clocks,
start_timeout_us: u32,
start_retries: u8,
addr_timeout_us: u32,
data_timeout_us: u32,
start_timeout: Microseconds,
start_attempts: u8,
addr_timeout: Microseconds,
data_timeout: Microseconds,
) -> BlockingI2c<I2C, PINS> {
let sysclk_mhz = clocks.sysclk().0 / 1_000_000;
BlockingI2c {
nb: i2c,
start_timeout: start_timeout_us * sysclk_mhz,
start_retries,
addr_timeout: addr_timeout_us * sysclk_mhz,
data_timeout: data_timeout_us * sysclk_mhz,
start_timeout: start_timeout.0 * sysclk_mhz,
start_attempts,
addr_timeout: addr_timeout.0 * sysclk_mhz,
data_timeout: data_timeout.0 * sysclk_mhz,
}
}

Expand Down Expand Up @@ -385,30 +419,30 @@ macro_rules! hal {
mode: Mode,
clocks: Clocks,
apb: &mut <$I2CX as RccBus>::Bus,
start_timeout_us: u32,
start_retries: u8,
addr_timeout_us: u32,
data_timeout_us: u32
start_timeout: Microseconds,
start_attempts: u8,
addr_timeout: Microseconds,
data_timeout: Microseconds
) -> Self {
blocking_i2c(I2c::$i2cX(i2c, pins, mode, clocks, apb),
clocks, start_timeout_us, start_retries,
addr_timeout_us, data_timeout_us)
clocks, start_timeout, start_attempts,
addr_timeout, data_timeout)
}

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() {
self.nb.reset();
} else {
break;
}
retries_left -= 1;
attempts_left -= 1;
}
last_ret
}
Expand Down
24 changes: 12 additions & 12 deletions src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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)
}
}

Expand All @@ -170,13 +170,13 @@ impl From<MegaHertz> for KiloHertz {
}
}

impl Into<Hertz> for MilliSeconds {
impl Into<Hertz> for Milliseconds {
fn into(self) -> Hertz {
Hertz(1_000 / self.0)
}
}

impl Into<Hertz> for MicroSeconds {
impl Into<Hertz> for Microseconds {
fn into(self) -> Hertz {
Hertz(1_000_000 / self.0)
}
Expand Down
16 changes: 8 additions & 8 deletions src/watchdog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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));
Expand All @@ -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
Expand Down Expand Up @@ -89,10 +89,10 @@ impl IndependentWatchdog {
}

impl WatchdogEnable for IndependentWatchdog {
type Time = MilliSeconds;
type Time = Milliseconds;

fn start<T: Into<Self::Time>>(&mut self, period: T) {
self.setup(period.into().0);
self.setup(period.into());

self.iwdg.kr.write(|w| unsafe { w.key().bits(KR_START) });
}
Expand Down