embedded-hal: `digital` operations do not allow error propagation

Currently operations defined in the digital module traits do not allow error propagation.

For example, in an I2C I/O expander driver I am writing I want to provide access to the individual pins as a bunch of Px structs implementing OutputPin and InputPin. Setting a pin or reading it involves I2C communication, which could fail. At the moment I just panic if there was a problem for either reading or writing. The same would happen with the other traits: StatefulOutputPin and ToggleableOutputPin

Another example from @Caemor:

The only problem I found with this api since using it is that errors can’t get propagated any further and need to be converted to a default value.

E.g. the linux sys_fs can receive an file error while trying to read the value (https://github.com/rust-embedded/rust-sysfs-gpio/blob/master/src/lib.rs#L337) and this can’t be handled by a simple bool.

Discussion started here

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 2
Comments: 16 (16 by maintainers)

Commits related to this issue

Merge #127 127: Digital v1 <-> v2 compatibility wrappers and shims r=japaric a=ryankurte This PR introduces implicit v1 -> v2 (forward) compatibility, and explicit wrapper types for v2 -> v1 (rever... — committed to rust-embedded/embedded-hal by bors[bot] 5 years ago

Most upvoted comments

@eldruin

pub trait InputPin {
    type Error;
    fn is_high(&self) -> Result<bool, Self::Error>;
}

hannobraun on Sep 27, 2018

woooo!

ryankurte on Mar 22, 2019

Hey, thanks for all the effort! Might just keep this open until we’ve got the compatibility shim in place and a release done.

ryankurte on Dec 17, 2018

You’re right. I like your solution better. There’s an open pull request now: https://github.com/rust-embedded/embedded-hal/pull/97

I think we should direct further discussion there.

hannobraun on Sep 28, 2018

Hmm, I like the idea of alleviating the issue, it but that seems to kindof double the breakage in code changes required? Find and replace is easy enough, but, slightly annoying to have to go through the code to fix things twice.

I wonder if for the proven traits we could do it with feature flags to switch between them. So the next release we mark the existing ones deprecated to let users no and have an opt in feature use-fallible-digital-traits that exposes the new ones so they can immediately upgrade if they choose. The following release we would swap that to default and have a feature use-infallible-digital-traits to opt to regress should they need to. That way the names could be the same, and it’s just a question of consumers removing the first feature when it’s replaced, and our removing the second feature when it’s unneeded.

That said, we are 0.x, people can choose when to update HAL versions, and by doing either we’re causing fragmentation. If we could poll for broader consensus from the wg / some heavy users maybe it’d be simpler for everyone just to replace it all at once?

ryankurte on Sep 28, 2018

I just noticed that my previous comment about the stabilization of ! making a difference here is mostly wrong. It applies to code that knows which embedded-hal implementation it is using, but it won’t make a difference for drivers, embedded-hal’s primary consumer.

hannobraun on Sep 27, 2018