Implement defmt::Format for Error #11
Conversation
|
Uhh, this change isn't compatible with I'll have to change that on another day. (Alternatively, feel free to push to my branch if you want to take over.) |
| @@ -124,6 +124,34 @@ pub enum Error { | |||
| TimerError, | |||
| } | |||
|
|
|||
| #[cfg(feature = "defmt")] | |||
| impl defmt::Format for Error { | |||
There was a problem hiding this comment.
Are there any reasons against the use of derive?
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
However, I have to admit that I have not looked at the output of cargo expand and of course there could be good reasons for a manual implementation.
There was a problem hiding this comment.
Ah, I simply wasn't aware that a derive macro exists for defmt 🙂 I'll test the output differences in my project sometime. (I haven't figured out how to convert defmt writes to a string on regular std-Rust on Linux...)
There was a problem hiding this comment.
Seems this is not possible currently: knurling-rs/defmt#463
There was a problem hiding this comment.
Have you had a chance to test the derive? If not, I'm also fine with the current implementation.
There was a problem hiding this comment.
Sorry, not yet. I'll try to take a look again soon.
| run: cargo build --release --features strict | ||
|
|
||
| - name: Build in strict mode with log feature |
There was a problem hiding this comment.
My first intuition would have been to implement the features as a matrix axis. But that would have made too much overhead, and so it is nice and lean. 👍
There was a problem hiding this comment.
Yeah, I think a matrix makes sense if we have even more distinct features, but for now this is probably the simplest and fastest way (because build cache is shared).
|
Any updates on this? If a test of derive is not possible due to time constraints, I would be happy to merge the PR in its current state and switch to derive later if necessary. |
|
Hey @marius-meissner, somehow I missed your reply. I'm a bit low on free time, and am not currently working on the project anymore where I integrated esp-at-nal. Thus, if you'd be willing to merge as-is, that's probably the best option for now 🙂 |
This allows printing erorrs with defmt. Really helpful for debugging.
Atat already implements this trait for their errors.