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.

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

Make digital pin traits fallible #97

Closed
wants to merge 17 commits into from
Closed

Make digital pin traits fallible #97

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
227468b
Make InputPin trait methods fallible
eldruin Sep 28, 2018
611ec43
Add documentation for InputPin Error type
eldruin Sep 28, 2018
2e90b03
Add FallibleOutputPin trait
eldruin Sep 28, 2018
7c2167b
Make ToggeableOutputPin fallible
eldruin Sep 28, 2018
156c861
Fix unwrapping toggle result in example
eldruin Sep 28, 2018
34cd2b8
Fix spacing
eldruin Sep 28, 2018
db447ce
Add ToggleableOutputPin change to changelog
eldruin Sep 28, 2018
e959f02
Put fallible OutputPin behind use-fallible-digital-traits feature
eldruin Sep 29, 2018
f873a73
Make StatefulOutputPin inherit from OutputPin to inherit its error type
eldruin Sep 29, 2018
2a1d72e
Mark infallible digital traits as deprecated
eldruin Sep 30, 2018
b681162
Update changelog
eldruin Sep 30, 2018
2bad26e
Add missing import
eldruin Sep 30, 2018
ea14b73
Prioritize fallible version
eldruin Sep 30, 2018
623aaf8
Split implementations to leave infallible traits untouched though dep…
eldruin Oct 12, 2018
2ab76fe
Add use-fallible-digital-traits to travis matrix
eldruin Oct 12, 2018
6224d88
Implement fallible digital traits as a clean break
eldruin Oct 16, 2018
3422af2
Remove builds with use-fallible-digital-traits feature
eldruin Oct 16, 2018
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
39 changes: 37 additions & 2 deletions src/digital.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub trait OutputPin {
/// 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")]
#[cfg(all(feature = "unproven", not(feature = "use-fallible-digital-traits")))]
pub trait StatefulOutputPin {
/// Is the pin in drive high mode?
///
Expand All @@ -54,6 +54,24 @@ pub trait StatefulOutputPin {
fn is_set_low(&self) -> bool;
}

/// Push-pull output pin that can read its output state
/// (Fallible version. This will become the default after the next release)
///
/// *This trait is available if embedded-hal is built with the `"unproven"` and
/// `"use-fallible-digital-traits"` features.*
#[cfg(all(feature = "unproven", feature = "use-fallible-digital-traits"))]
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) -> 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) -> Result<bool, Self::Error>;
}

/// Output pin that can be toggled
///
/// *This trait is available if embedded-hal is built with the `"unproven"` feature.*
Expand Down Expand Up @@ -107,6 +125,7 @@ pub trait ToggleableOutputPin {
/// }
/// }
///
/// #[cfg(not(feature = "use-fallible-digital-traits"))]
/// impl StatefulOutputPin for MyPin {
/// fn is_set_low(&self) -> bool {
/// !self.state
Expand All @@ -116,14 +135,30 @@ pub trait ToggleableOutputPin {
/// }
/// }
///
/// #[cfg(feature = "use-fallible-digital-traits")]
/// impl StatefulOutputPin for MyPin {
/// fn is_set_low(&self) -> Result<bool, Self::Error> {
/// Ok(!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().unwrap();
/// #[cfg(not(feature = "use-fallible-digital-traits"))]
/// assert!(pin.is_set_high());
/// #[cfg(feature = "use-fallible-digital-traits")]
/// assert!(pin.is_set_high().unwrap());
/// pin.toggle().unwrap();
/// #[cfg(not(feature = "use-fallible-digital-traits"))]
/// assert!(pin.is_set_low());
/// #[cfg(feature = "use-fallible-digital-traits")]
/// 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")]
pub mod toggleable {
Expand Down Expand Up @@ -161,7 +196,7 @@ pub mod toggleable {

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