Skip to content

Input/Output pin trait proposal #151

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

Closed
wants to merge 2 commits into from
Closed

Input/Output pin trait proposal #151

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f525b1d
Input/Output pin trait proposal
rumatoest Sep 30, 2019
c074a0e
Quick fix for associated types inheritance
rumatoest Sep 30, 2019
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
26 changes: 25 additions & 1 deletion src/digital/v2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub trait OutputPin {
///
/// *This trait is available if embedded-hal is built with the `"unproven"` feature.*
#[cfg(feature = "unproven")]
pub trait StatefulOutputPin : OutputPin {
pub trait StatefulOutputPin: OutputPin {
/// Is the pin in drive high mode?
///
/// *NOTE* this does *not* read the electrical state of the pin
Expand Down Expand Up @@ -136,3 +136,27 @@ pub trait InputPin {
/// Is the input pin low?
fn is_low(&self) -> Result<bool, Self::Error>;
}

/// Describe pin that can switch its input/ouput mode on fly.
/// Should be used to communicate with custom protocols based hardware.
///
/// *This trait is available if embedded-hal is built with the `"unproven"` feature.*
#[cfg(feature = "unproven")]
pub trait InputOutputPin<Error>: InputPin<Error = Error> + OutputPin<Error = Error> {
/// Switch pin into output mode,
///
/// Result::Ok value is true if pin mode was changed or false if pin already was in output mode.
///
/// Expected behaviour after success result:
/// - OutputPin functions should work as for ouptut pin
/// - InputPin functions should return errors
Comment on lines +151 to +152
Copy link
Contributor

Choose a reason for hiding this comment

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

This cannot work because an object implementing InputPin cannot know that it also is implementing InputOutputPin and needs to behave differently if it is in output mode and vice versa. That's what I mean by "this is not composable".

Copy link
Author

Choose a reason for hiding this comment

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

But there can be some kind of structure which does know about current pin mode and can use one or another nested pin structure (via enum for example) to provide required functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

You cannot retroactively impose additional restrictions or interfaces on existing implementations with a new trait.

Copy link
Author

Choose a reason for hiding this comment

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

Why not? HAL has not released yet and it has unproven feature. So it is possible to add new unproven trait to new HAL versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but whatever you put in the traits will not have any influence on the trait impls which are out there.

fn mode_output(&mut self) -> Result<bool, Error>;
Copy link
Contributor

@therealprof therealprof Oct 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

These functions will (in most real world uses) require inline blocking due to the read-modify-write operations involved to change between input an output which is generally frowned upon.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, indeed, but there are some platform related options available to make it more safe like using asm instructions to block interrupts or using CAS instructions.
I know that it would not be fastest possible implementation, but it has to be as easy to use as possible. Performance drawbacks can be described in trait documentation. BTW there is an opinion that for such cases performance (1 pin bus throughput) should not be big deal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe fn mode_output(Self) -> Result<Self::Output, Self::Error>?


/// Switch pin into input mode without activating any pull up/down resistors,
/// Result::Ok value is true if pin mode was changed or false if pin already was in input mode.
///
/// /// Expected behaviour after success result:
/// - InputPin functions should work as for ouptut pin
/// - OutputPin functions should return errors
fn mode_input(&mut self) -> Result<bool, Error>;
}