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

Make digital pin traits fallible #97

Closed
wants to merge 17 commits into from
Closed
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
8 changes: 4 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@ language: rust

matrix:
include:
- env: TARGET=x86_64-unknown-linux-gnu
- env: TARGET=x86_64-unknown-linux-gnu FEATURES=unproven
if: (branch = staging OR branch = trying) OR (type = pull_request AND branch = master)

- env: TARGET=thumbv6m-none-eabi
- env: TARGET=thumbv6m-none-eabi FEATURES=unproven
rust: beta
if: (branch = staging OR branch = trying) OR (type = pull_request AND branch = master)

- env: TARGET=thumbv7m-none-eabi
- env: TARGET=thumbv7m-none-eabi FEATURES=unproven
rust: beta
if: (branch = staging OR branch = trying) OR (type = pull_request AND branch = master)

- env: TARGET=x86_64-unknown-linux-gnu
- env: TARGET=x86_64-unknown-linux-gnu FEATURES=unproven
rust: nightly
if: (branch = staging OR branch = trying) OR (type = pull_request AND branch = master)

Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Changed

- [breaking-change] The digital `OutputPin`, `StatefulOutputPin`, `ToggleableOutputPin`
and `InputPin` traits have been replaced with fallible versions.
The methods now return a `Result` type as setting an output pin
and reading an input pin could potentially fail.
See [here](https://github.com/rust-embedded/embedded-hal/issues/95) for more info.

## [v0.2.1] - 2018-05-14

### Changed
Expand Down
4 changes: 2 additions & 2 deletions ci/script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ set -euxo pipefail

main() {
cargo check --target $TARGET
cargo check --target $TARGET --features unproven
cargo check --target $TARGET --features $FEATURES

if [ $TRAVIS_RUST_VERSION = nightly ]; then
cargo test --target $TARGET --features unproven
cargo test --target $TARGET --features $FEATURES
fi
}

Expand Down
69 changes: 43 additions & 26 deletions src/digital.rs
Original file line number Diff line number Diff line change
@@ -1,37 +1,40 @@
//! Digital I/O
//! Digital I/O.

/// Single digital push-pull output pin
/// Single digital push-pull output pin.
pub trait OutputPin {
/// Error type
type Error;

/// Drives the pin low
///
/// *NOTE* the actual electrical state of the pin may not actually be low, e.g. due to external
/// electrical sources
fn set_low(&mut self);
fn set_low(&mut self) -> Result<(), Self::Error>;

/// Drives the pin high
///
/// *NOTE* the actual electrical state of the pin may not actually be high, e.g. due to external
/// electrical sources
fn set_high(&mut self);
fn set_high(&mut self) -> Result<(), Self::Error>;
}

/// Push-pull output pin that can read its output state
///
/// *This trait is available if embedded-hal is built with the `"unproven"` feature.*
#[cfg(feature = "unproven")]
pub trait StatefulOutputPin {
pub trait StatefulOutputPin : OutputPin {
/// Is the pin in drive high mode?
///
/// *NOTE* this does *not* read the electrical state of the pin
fn is_set_high(&self) -> bool;
fn is_set_high(&self) -> Result<bool, Self::Error>;

/// Is the pin in drive low mode?
///
/// *NOTE* this does *not* read the electrical state of the pin
fn is_set_low(&self) -> bool;
fn is_set_low(&self) -> Result<bool, Self::Error>;
}

/// Output pin that can be toggled
/// Output pin that can be toggled.
///
/// *This trait is available if embedded-hal is built with the `"unproven"` feature.*
///
Expand All @@ -41,8 +44,11 @@ pub trait StatefulOutputPin {
/// implemented. Otherwise, implement this using hardware mechanisms.
#[cfg(feature = "unproven")]
pub trait ToggleableOutputPin {
/// Error type
type Error;

/// Toggle pin output.
fn toggle(&mut self);
fn toggle(&mut self) -> Result<(), Self::Error>;
}

/// If you can read **and** write the output state, a pin is
Expand All @@ -58,33 +64,38 @@ pub trait ToggleableOutputPin {
/// }
///
/// impl OutputPin for MyPin {
/// fn set_low(&mut self) {
/// type Error = void::Void;
///
/// fn set_low(&mut self) -> Result<(), Self::Error> {
/// self.state = false;
/// Ok(())
/// }
/// fn set_high(&mut self) {
/// fn set_high(&mut self) -> Result<(), Self::Error> {
/// self.state = true;
/// Ok(())
/// }
/// }
///
/// impl StatefulOutputPin for MyPin {
/// fn is_set_low(&self) -> bool {
/// !self.state
/// fn is_set_low(&self) -> Result<bool, Self::Error> {
/// Ok(!self.state)
/// }
/// fn is_set_high(&self) -> bool {
/// self.state
/// fn is_set_high(&self) -> Result<bool, Self::Error> {
/// Ok(self.state)
/// }
/// }
///
/// /// Opt-in to the software implementation.
/// impl toggleable::Default for MyPin {}
///
/// let mut pin = MyPin { state: false };
/// pin.toggle();
/// assert!(pin.is_set_high());
/// pin.toggle();
/// assert!(pin.is_set_low());
/// pin.toggle().unwrap();
/// assert!(pin.is_set_high().unwrap());
/// pin.toggle().unwrap();
/// assert!(pin.is_set_low().unwrap());
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I'm wondering, if we should simplify the example. I mean, we want people to use the new fallible versions, so why even show the infallible versions here? Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that the docs should prioritise the fallible version, though maybe we could keep the infallible documentation until the opt-out release?

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the fallible versions up. However, the example still looks quite cluttered with #cfg... to me. Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is a bit cluttered, maybe grouping each example would help?
I'm ok with merging this either way.

Copy link
Member Author

Choose a reason for hiding this comment

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

I found no way to group the statements so that I can have fewer #cfgs. Do you have an idea?

/// ```
#[cfg(feature = "unproven")]
#[allow(deprecated)]
pub mod toggleable {
use super::{OutputPin, StatefulOutputPin, ToggleableOutputPin};

Expand All @@ -97,25 +108,31 @@ pub mod toggleable {
where
P: Default,
{
type Error = P::Error;

/// Toggle pin output
fn toggle(&mut self) {
if self.is_set_low() {
self.set_high();
fn toggle(&mut self) -> Result<(), Self::Error> {
if self.is_set_low()? {
self.set_high()
} else {
self.set_low();
self.set_low()
}
}
}
}

/// Single digital input pin
/// Single digital input pin.
///
/// *This trait is available if embedded-hal is built with the `"unproven"` feature.*
#[cfg(feature = "unproven")]
pub trait InputPin {
/// Error type
type Error;

/// Is the input pin high?
fn is_high(&self) -> bool;
fn is_high(&self) -> Result<bool, Self::Error>;

/// Is the input pin low?
fn is_low(&self) -> bool;
fn is_low(&self) -> Result<bool, Self::Error>;
}