Skip to content

Fix various errors in examples #19

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.

Sign up for GitHub

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

Merged
homunkulus merged 7 commits into from
Sep 22, 2017
Merged

Fix various errors in examples #19

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3901140
Fix STM32F103xx implementation example
hannobraun Sep 14, 2017
6cb27ab
Improve `Mutex` example
hannobraun Sep 14, 2017
24c8607
Fix some issues with the futures example
hannobraun Sep 14, 2017
6d94424
Fix "blocking whole-buffer write" example
hannobraun Sep 14, 2017
3359363
Fix "blocking read with timeout" example
hannobraun Sep 15, 2017
81abc9c
Improve "serial with periodic flushing" example
hannobraun Sep 15, 2017
dbafdc7
Address review comment
hannobraun Sep 22, 2017
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
106 changes: 68 additions & 38 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
//! ``` ignore
//! //! An implementation of the `embedded-hal` for STM32F103xx microcontrollers
//!
//! extern crate core;
//! extern crate embedded_hal as hal;
//! extern crate nb;
//!
Expand All @@ -130,40 +131,50 @@
//! // and USART3
//! pub struct Serial<'a, U>(pub &'a U)
//! where
//! U: Deref<stm32f103xx::usart1::RegisterBlock> + 'static;
//! U: Deref<Target=stm32f103xx::usart1::RegisterBlock> + 'static;
//!
//! /// Serial interface error
//! pub enum Error {
//! /// Buffer overrun
//! Overrun,
//! ..
//! // add more error variants here
//! }
//!
//! impl<'a, U> hal::Serial for Serial<'a, U> {
//! impl<'a, U> hal::serial::Read<u8> for Serial<'a, U>
//! where
//! U: Deref<Target=stm32f103xx::usart1::RegisterBlock> + 'static
//! {
//! type Error = Error;
//!
//! fn read(&self) -> nb::Result<u8, Error> {
//! // read the status register
//! let sr = self.sr.read();
//! let sr = self.0.sr.read();
//!
//! if sr.ovr().bit_is_set() {
//! if sr.ore().bit_is_set() {
//! // Error: Buffer overrun
//! Err(nb::Error::Other(Error::Overrun))
//! } else if .. {
//! // Other error conditions
//! ..
//! } else if sr.rxne().bit_is_set() {
//! }
//! // Add additional `else if` statements to check for other errors
//! else if sr.rxne().bit_is_set() {
//! // Data available: read the data register
//! Ok(self.dr.read())
//! } else {
//! Ok(self.0.dr.read().bits() as u8)
//! }
//! else {
//! // No data available yet
//! Err(nb::Error::WouldBlock)
//! }
//! }
//! }
//!
//! impl<'a, U> hal::serial::Write<u8> for Serial<'a, U>
//! where
//! U: Deref<Target=stm32f103xx::usart1::RegisterBlock> + 'static
//! {
//! type Error = Error;
//!
//! fn write(&self, byte: u8) -> nb::Result<(), Error> {
//! // Very similar to the above implementation
//! ..
//! Ok(())
//! }
//! }
//! ```
Expand All @@ -179,16 +190,21 @@
//! implementation:
//!
//! ``` ignore
//! extern crate embedded_hal as hal;
//! extern crate nb;
//!
//! /// A synchronized serial interface
//! // NOTE This is a global singleton
//! pub struct Serial1;
//! pub struct Serial1(Mutex<..>);
Copy link
Member

@japaric japaric Sep 22, 2017

Choose a reason for hiding this comment

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

If this contains a field then it's no longer a global singleton. It's also not required to have this field. See below

//!
//! // NOTE private
//! static USART1: Mutex<_> = Mutex::new(..);
//!
//! impl hal::Serial for Serial {
//! fn read(&self) -> Result<u8, nb::Error<Error>> {
//! hal::Serial::read(&Serial(USART1.lock()))
//! impl hal::serial::Read<u8> for Serial1 {
//! type Error = !;
//!
//! fn read(&self) -> Result<u8, nb::Error<Self::Error>> {
//! hal::serial::Read::read(&Serial1(USART1.lock()))
Copy link
Member

@japaric japaric Sep 22, 2017

Choose a reason for hiding this comment

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

This can use the Serial struct from before like this hal::serial::Read::read(&Serial(&*USART1.lock()))

Copy link
Member Author

@hannobraun hannobraun Sep 22, 2017

Choose a reason for hiding this comment

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

Ah, got it. I missed the point of that example before.

//! }
//! }
//! ```
Expand Down Expand Up @@ -242,40 +258,46 @@
//! extern crate nb;
//!
//! use hal::prelude::*;
//! use futures::{
//! future,
//! Async,
//! Future,
//! };
//! use futures::future::Loop;
//! use stm32f103xx_hal_impl::{Serial, Timer};
//!
//! /// `futures` version of `Timer.wait`
//! ///
//! /// This returns a future that must be polled to completion
//! fn wait<T>(timer: &T) -> impl Future<Item = (), Error = !>
//! fn wait<T>(timer: T) -> impl Future<Item = (), Error = !>
Copy link
Member

@japaric japaric Sep 22, 2017

Choose a reason for hiding this comment

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

With this signature the function will destroy the timer object. The pattern that the futures ecosystem uses to work around this issue is to return the input by value in Item. Thus the return type should be impl Future<Item = T, Error = !>.

Copy link
Member Author

@hannobraun hannobraun Sep 22, 2017

Choose a reason for hiding this comment

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

I don't think this pattern will work with poll_fn. Here's what I got now:

fn wait<T>(timer: T) -> impl Future<Item = T, Error = !>
where
    T: hal::Timer,
{
    future::poll_fn(|| {
        try_nb!(timer.wait());
        Ok(Async::Ready(timer))
    })
}

And here's how it fails:

error[E0507]: cannot move out of captured outer variable in an `FnMut` closure
  --> src/lib.rs:29:25
   |
23 | fn wait<T>(timer: T) -> impl Future<Item = T, Error = !>
   |            ----- captured outer variable
...
29 |         Ok(Async::Ready(timer))
   |                         ^^^^^ cannot move out of captured outer variable in an `FnMut` closure

Which makes sense, as poll_fn takes an FnMut closure.
Of course I could re-create poll_fn/PollFn, but this seems a bit to involved for an example like this. What do you think?

Copy link
Member

@japaric japaric Sep 22, 2017

Choose a reason for hiding this comment

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

Yeah, I think you'll have to use loop_fn instead.

Copy link
Member Author

@hannobraun hannobraun Sep 22, 2017

Choose a reason for hiding this comment

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

Thanks for the help! I think I got it now.

//! where
//! T: hal::Timer,
//! {
//! future::poll_fn(|| {
//! future::poll_fn(move || {
//! Ok(Async::Ready(try_nb!(timer.wait())))
//! })
//! }
//!
//! /// `futures` version of `Serial.read`
//! ///
//! /// This returns a future that must be polled to completion
//! fn read<S>(serial: &S) -> impl Future<Item = u8, Error = Error>
//! fn read<S>(serial: S) -> impl Future<Item = u8, Error = S::Error>
Copy link
Member

@japaric japaric Sep 22, 2017

Choose a reason for hiding this comment

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

same comment as above

//! where
//! S: hal::Serial,
//! S: hal::serial::Read<u8>,
//! {
//! future::poll_fn(|| {
//! future::poll_fn(move || {
//! Ok(Async::Ready(try_nb!(serial.read())))
//! })
//! }
//!
//! /// `futures` version of `Serial.write`
//! ///
//! /// This returns a future that must be polled to completion
//! fn write<S>(byte: u8) -> impl Future<Item = (), Error = Error>
//! fn write<S>(serial: S, byte: u8) -> impl Future<Item = (), Error = S::Error>
Copy link
Member

@japaric japaric Sep 22, 2017

Choose a reason for hiding this comment

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

same comment as above

//! where
//! S: hal::Serial,
//! S: hal::serial::Write<u8>,
//! {
//! future::poll_fn(|| {
//! future::poll_fn(move || {
//! Ok(Async::Ready(try_nb!(serial.write(byte))))
//! })
//! }
Expand All @@ -285,7 +307,7 @@
//! let serial = Serial(usart1);
//!
//! // Tasks
//! let mut blinky = future::loop_fn(true, |_| {
//! let mut blinky = future::loop_fn(true, |state| {
//! wait(timer).map(|_| {
//! if state {
//! Led.on();
Expand Down Expand Up @@ -381,14 +403,16 @@
//!
//! ``` ignore
//! extern crate embedded_hal as hal;
//! #[macro_use]
//! extern crate nb;
//!
//! use hal::prelude::*;
//!
//! fn write_all<S>(serial: &S, buffer: &[u8]) -> Result<(), S::Error>
//! where
//! S: hal::Serial
//! S: hal::serial::Write<u8>
//! {
//! for byte in buffer {
//! for &byte in buffer {
//! block!(serial.write(byte))?;
//! }
//!
Expand All @@ -400,23 +424,24 @@
//!
//! ``` ignore
//! extern crate embedded_hal as hal;
//! extern crate nb;
//!
//! use hal::prelude::*;
//!
//! enum Error<E> {
//! /// Serial interface error
//! Serial(E)
//! Serial(E),
//! TimedOut,
//! }
//!
//! fn read_with_timeout(
//! fn read_with_timeout<S, T>(
//! serial: &S,
//! timer: &T,
//! timeout: T::Ticks,
//! timeout: T::Time,
//! ) -> Result<u8, Error<S::Error>>
//! where
//! T: hal::Timer,
//! S: hal::Serial,
//! S: hal::serial::Read<u8>,
//! {
//! timer.pause();
//! timer.restart();
Expand All @@ -433,6 +458,12 @@
//! }
//!
//! match timer.wait() {
//! Err(nb::Error::Other(e)) => {
//! // The error type specified by `timer.wait()` is `!`, which
//! // means no error can actually occur. The Rust compiler
//! // still forces us to provide this match arm, though.
//! e
//! },
//! Err(nb::Error::WouldBlock) => continue,
//! Ok(()) => {
//! timer.pause();
Expand Down Expand Up @@ -477,9 +508,9 @@
//!
//! use hal::prelude::*;
//!
//! fn flush(serial: &S, cb: &mut CircularBuffer) -> Result<(), S::Error>
//! fn flush<S>(serial: &S, cb: &mut CircularBuffer) -> Result<(), S::Error>
//! where
//! S: hal::Serial,
//! S: hal::serial::Write<u8>,
//! {
//! loop {
//! if let Some(byte) = cb.peek() {
Expand All @@ -501,23 +532,22 @@
//!
//! // NOTE private
//! static BUFFER: Mutex<CircularBuffer> = ..;
//! static SERIAL: Mutex<impl hal::Serial> = ..;
//! static SERIAL: Mutex<impl hal::serial::Write<u8>> = ..;
//!
//! impl BufferedSerial {
//! pub fn write(&self, byte: u8) {
//! pub fn write(&self, bytes: &[u8]) {
//! let mut buffer = BUFFER.lock();
//! for byte in byte {
//! for byte in bytes {
//! buffer.push(*byte).unwrap();
//! }
//! }
//!
//! pub fn write_all(&self, bytes: &[u8]) {
//! let mut buffer = BUFFER.lock();
//! for byte in bytes {
//! cb.push(*byte).unwrap();
//! buffer.push(*byte).unwrap();
//! }
//! }
//!
//! }
//!
//! fn interrupt_handler() {
Expand Down