Use of &'static str for error values

[RossPorter506]

Hi, thanks for writing this library, I've been using it without issues on the 915MHz band.

I wanted to ask about the use of &'static str as the main error type. It makes it difficult to match on specific errors because the user doesn't know what errors actually exist without wading through the library source code, and on no-std targets the strings are baked in even if they don't need to be printed, which inflates the size of the binary.
Compare this to an error enum that implements Debug, which is both much easier to match against but also the error strings are only included if the errors are actually printed (if they are unwrapped, for instance). I quickly threw together and tested a version of this library with enums and this reduced the size of the library by 20-30% (in my project the library accounted for 2-3kB of program memory, and the enum version was ~600B smaller). On my 32kB microcontroller this was very noticeable.

I'm happy to write a PR for this as I've been playing around with some error handling implementations already, so if you're willing we could discuss specifics.

[KizzyCode]

Hey, thank you very much for your feedback – I'm happy to hear that it works with 915MHz too! And you're completely right, tbh it's mostly a remnant from the first iterations; I've actually a branch with that lying around already – just didn't had the time/need to rebase and publish it yet 😅

It's basically an enum with optional (aka cargo-feature-gated) human readable information (description + file!() + line!()) – if that feature is disabled, it will boil down to an enum (but if you want, you can have nice debugging info for your logs too).

Thanks for bringing this up; that's a good occasion for me to get this finally done – I'll publish something later today 😊

[KizzyCode]

So, I've created #2 – if you have time, please let me know if that suits your needs 😊

Also, from what I've tested, the static strings in the macros should be eliminated if you compile with optimizations and without --feature=backtrace – but it might make sense to double check this, as I haven't tested it against a real microcontroller build yet 😅

[RossPorter506]

Thanks, this is much improved. I don't see any of the strings in my binary :)
It's still not entirely clear which methods return which error kinds without checking the source though. My error handling currently looks something like:

match rfm95.complete_tx() {
    Ok(None) => {...} ,
    Ok(Some(n)) => {...} ,
    Err(Error{kind: IoError}) => {...} ,
    Err(Error{kind: _ }) => unreachable!(), // complete_tx() only returns IoError
}

But short of having different error enums for different methods I don't really see an easy solution for this. Perhaps as a bandaid the possible error types a method returns could be put in their docstring?

[KizzyCode]

Hm, tough question. With a single enum, errors are somewhat “infectious”, meaning that IRL I find it difficult to uphold docstring guarantees if various errors might bubble up from underlying functions without the type system yelling at me.

Different error types actually might make sense though; especially since InvalidArgument | IoError are probably always fatal, whereas Timeout | InvalidMessage are often recoverable.

Since there are only four error kinds, it should be possible to build a strict hierarchy without a combinatory explosion; so distinct types could be reasonable possible.

IoError
IoError | InvalidArgument
IoError | Timeout | InvalidMessage

would make sense. I’ll look into it 😊

[KizzyCode]

So, I've updated #2 with more distinct error types. If you have time, please let me know if that feels more ergonomic for you.

[RossPorter506]

This will do perfectly, thanks for your efforts!

[KizzyCode]

You're welcome 😊

---

Implemented by #2