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

i2c trait Error type has no bounds #84

Closed
LunNova opened this issue May 25, 2018 · 13 comments
Closed

i2c trait Error type has no bounds #84

LunNova opened this issue May 25, 2018 · 13 comments

Comments

@LunNova
Copy link

LunNova commented May 25, 2018

Is it intentional that there are no bounds on the associated Error types for the i2c traits?

As they are not guaranteed to implement anything, it doesn't seem to be possible to make a custom Error type which can implement From for them in a useful way.

It would be nice to be able to write code like this:

fn frob_foos(read: &mut impl Read, write: &mut impl Write) -> Result<bool, MyErrorType???> {
    write.write(ADDR, &[6, 0])?;
    write.write(ADDR, &[7, 0xFF])?;
    write.write(ADDR, &[2, 0x55])?;
    write.write(ADDR, &[0])?;
    let mut buf = [0u8];
    read.read(ADDR, &mut buf)?;
    // ...
}
@therealprof
Copy link
Contributor

Not that I've tried but why can't you use the associated type used for the error implementation?

If to return a custom error, why not map it to your error type using the map_err() method?

@LunNova
Copy link
Author

LunNova commented May 28, 2018

Not that I've tried but why can't you use the associated type used for the error implementation?

That works fine for a function which only uses a Read or only uses a Write. For a function using both a read or write there's no guarantee they return the same error type.

If to return a custom error, why not map it to your error type using the map_err() method?

How can I do this usefully in a generic way when there is no trait implemented by the associated type? I can map any type to an unknown error which will work but there's no way of getting info about what the error is from it, so that information will be lost.

@therealprof
Copy link
Contributor

That works fine for a function which only uses a Read or only uses a Write. For a function using both a read or write there's no guarantee they return the same error type.

That's a possibility but a somewhat weird one since the Error type would be implemented by the same hal implementation on the same hardware using the same registers...

How can I do this usefully in a generic way when there is no trait implemented by the associated type? I can map any type to an unknown error which will work but there's no way of getting info about what the error is from it, so that information will be lost.

That is correct. However I'm a bit puzzled how you would use the implementation dependent errors in a generic way. I really only see three options here:

  1. You make your error handling hal specific
  2. You make your error handling generic by throwing away the concrete error cause
  3. We introduce a generic (super-)set of interface specific standard errors and the users will have to live with the fact that some errors can not be (easily) produced by some hardware or implementers do not care to implement them and only supply the general errors, bringing you back to square one

The first two points can be implemented today.

@LunNova
Copy link
Author

LunNova commented May 28, 2018

That's a possibility but a somewhat weird one since the Error type would be implemented by the same hal implementation on the same hardware using the same registers...

and because it's a possibility you can't compile a function which returns a single error type from either a Read or a Write.

There's no way of constraining the associated types of the two things to be the same.

@therealprof
Copy link
Contributor

@nallar In your first post you mentioned it's not possible to implement a custom conversion using From (which I do not think is accurate) but now you're saying you can't return the original error from the hal in a mixes usage scenario which is a completely different issue.

I'm not sure whether the latter can be easily addressed but if you have any idea we can certainly discuss it.

@LunNova
Copy link
Author

LunNova commented May 28, 2018

Yes, I'm getting a bit distracted from the original question. Oops.

In your first post you mentioned it's not possible to implement a custom conversion using From (which I do not think is accurate)

You can, but only by discarding the error information. ('not in a useful way')

I'm not sure whether the latter can be easily addressed but if you have any idea we can certainly discuss it.

I don't know how to fix it.

I don't have solutions, only problems. :(

@hannobraun
Copy link
Member

That works fine for a function which only uses a Read or only uses a Write. For a function using both a read or write there's no guarantee they return the same error type.

For what it's worth, this should work some day:

trait Read {
    type Error;
}

trait Write {
    type Error;
}


fn do_stuff<T>(_: T) -> Result<(), <T as Read>::Error>
    where
        T: Read + Write,
        <T as Read>::Error == <T as Write>::Error,
{
    Ok(())
}

Currently fails with:

error: equality constraints are not yet supported in where clauses (#20041)
  --> src/lib.rs:13:9
   |
13 |         <T as Read>::Error == <T as Write>::Error,
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Would something like the following work?

pub enum MyError<R: ErrorTrait, W: ErrorTrait> {
    Read(R),
    Write(W),
}


pub fn do_stuff<T>(_: T)
    -> Result<(), MyError<<T as Read>::Error, <T as Write>::Error>>
    where
        T: Read + Write,
        <T as Read>::Error: ErrorTrait,
        <T as Write>::Error: ErrorTrait,
{
    Ok(())
}

@LunNova
Copy link
Author

LunNova commented May 29, 2018

Thanks @hannobraun. That last one is a good solution for now. It's a bit boilerplatey but allows it to be fully generic without losing the error info. It can also implement From for both error types allowing use of ?.

🥇

e: the : ErrorTrait bounds need removed since there is no trait for the i2c error types

Your version with equality constraints will be the best once they are done.

@hannobraun
Copy link
Member

@nallar Great to know it works for you!

the : ErrorTrait bounds need removed since there is no trait for the i2c error types

ErrorTrait could be something that a HAL implementation defines. Or it could be a trait you define in your own code, that you implement on HAL implementation error types, or that you provide blanket implementations for based on other traits. Not sure how useful that would be in practice, but it's something to keep in mind.

I just meant to show how to require that the error types have a common API, as a possible alternative to the equality constraint. But I should have been more explicit about that part of the example.

@eldruin
Copy link
Member

eldruin commented Sep 16, 2020

This should be solved once we land #229

@danieldulaney
Copy link

danieldulaney commented Nov 18, 2020

In case anybody else stumbles on this, another alternative is:

pub fn do_stuff<I2C, E>(interface: &mut I2C) -> Result<(), E>
where
    I2C: Read + Write,
    E: From<<I2C as Read>::Error> + From<<I2C as Write>::Error>
{
    interface.write(...)?;
    interface.read(...)?;

    Ok(());
}

This gives the caller maximum flexibility to define whatever error they want, as long as it has From impls. Also, it works out great in the case where both Read::Error and Write::Error happen to be the same: the user can just return that error type without having to define any boilerplate.

If you want to define a custom error type (for example with Debug), you can just add that as a requirement. It's a little more boilerplate, but not much.

pub fn do_stuff<I2C, E, WE, RE>(interface: &mut I2C) -> Result<(), E>
where
    I2C: Read<Error = RE> + Write<Error = WE>,
    E: From<<I2C as Read>::Error> + From<<I2C as Write>::Error>,
    WE: Debug,
    RE: Debug
{
    interface.write(...)?;
    interface.read(...)?;

    Ok(());
}

@therealprof
Copy link
Contributor

This should be solved once we land #229

If we ever land that...

@eldruin
Copy link
Member

eldruin commented Aug 4, 2022

Closing. This was solved in #296

@eldruin eldruin closed this as completed Aug 4, 2022
peckpeck pushed a commit to peckpeck/embedded-hal that referenced this issue Nov 10, 2022
84: Prepare 0.4.0-alpha.3 release r=ryankurte a=eldruin

Now that rust-embedded#83 is in, it would be good to publish a new alpha release.

Co-authored-by: Diego Barrios Romero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants