CAN: returning Result instead of Option (#741) - #749
jorgeandrecastro wants to merge 7 commits into
Conversation
|
Hello! My code changes are ready, but the MSRV check is currently failing on CI. After investigating locally, it turns out the failure is caused by an external dependency (cortex-m-macros) which now requires Rust 1.85 / Edition 2024, whereas the current MSRV for this crate is set to 1.83. Since this is coming from a third-party dependency update outside of this PR, I wanted to point it out. Please let me know how you would prefer to handle this (e.g., bumping the MSRV or locking the dependency version). Thank you! |
|
Please could you bump the MSRV in this pull request? |
Hello !, Its done! I've bumped the MSRV to 1.85 across the affected workspace crates (embedded-can, embedded-io, embedded-hal, etc.) and updated the CI workflow (test.yml) accordingly. All CI checks are now passing successfully! |
Hello embedded-hal team,
I’m submitting this pull request to address issue #741.
To do so, I modified the methods that previously returned an Option to signal logic errors (such as invalid IDs or payloads that are too long). They now return a Result instead, using ErrorKind::InvalidId and ErrorKind::DataTooLong to provide more meaningful error information.
Changes I made
Updated StandardId::new and ExtendedId::new in id.rs so that they return Result<Self, ErrorKind> instead of Option.
Updated the Frame trait definitions in lib.rs (new and new_remote) so that they return Result<Self, ErrorKind>.
Added corresponding unit tests and clear English comments to ensure the expected behavior and make the code easier to maintain.
I hope this pull request addresses the issue and can be useful to embedded developers.
I’d be happy to discuss the changes with you and receive any feedback or suggestions.
Thank you for your time and for reviewing my contribution!