eminence/lifx

lifx_core::Error doesn't implement std::error::Error

elias123tre opened this issue · 5 comments

The title says it all. It would be nice to have a return type of for example Result<(), Box<dyn Error>> and use error propagation with ? operator.

Example (see last line):

use lifx_core::{BuildOptions, Message, RawMessage};

fn build_message() -> Result<RawMessage, Box<dyn std::error::Error>> {
    let options = BuildOptions {
        target: None,
        ack_required: false,
        res_required: false,
        sequence: 0,
        source: 0,
    };
    Ok(RawMessage::build(&options, Message::GetInfo)?)
}

Error from compiler (on line with question mark):

error[E0277]:
the trait bound `lifx_core::Error: std::error::Error` is not satisfied
required because of the requirements on the impl of `From<lifx_core::Error>` for `Box<dyn std::error::Error>`
required because of the requirements on the impl of `FromResidual<Result<Infallible, lifx_core::Error>>` for `Result<RawMessage, Box<dyn std::error::Error>>`

Good idea. Your issue is a funny coincidence because after being dormant for several years, I was just thinking yesterday about giving another look at lifx_core to see if it needs a refresh. I'll get this fixed soon.

Nice! I saw that the development had lacked recently but submitted an issue anyways. Nice to see you're still active :)

I don't think the fix is that hard so I can probably open a PR and implement it myself if you want.

Sure, if you want to submit a PR, that would be great.

I went to create a PR and it seems the issue is already resolved with #2. Although the crates.io version does not have this change. Maybe a version bump and publish to crates.io will solve this.

Edit: Maybe a link to the crates.io package in the readme would be nice too.

Hah, ok. I'm a bit embarrassed by having forgotten all about that fix. I'll try to get this published this upcoming weekend.