[jorgeandrecastro]

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!

[jorgeandrecastro]

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!

[adamgreig]

Please could you bump the MSRV in this pull request?

[jorgeandrecastro]

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!

[eldruin]

This sounds good to me, thank you!
Could you add an entry to the changelogs and update the readmes of all projects where the MSRV was updated?
It is unfortunate to have to update the MSRV for the other projects as well but we already have an unreleased MSRV bump in the pipeline anyway.

[eldruin]

Suggested change

	ExtendedId::MAX.standard_id(),
	StandardId::new((ExtendedId::MAX.0 >> 18) as u16).unwrap()
	Ok(ExtendedId::MAX.standard_id()),
	StandardId::new((ExtendedId::MAX.0 >> 18) as u16)

How about this to avoid the unwrap?

[jorgeandrecastro]

This sounds good to me, thank you! Could you add an entry to the changelogs and update the readmes of all projects where the MSRV was updated? It is unfortunate to have to update the MSRV for the other projects as well but we already have an unreleased MSRV bump in the pipeline anyway.

Done! I've applied your suggested change in embedded-can/src/id.rs to avoid .unwrap(), updated the changelogs and readmes for all impacted crates, and formatted the code with cargo fmt

[adamgreig]

Sorry for the lack of response here. We discussed this PR a few weeks ago (rust-embedded/wg#941 (comment), logs https://libera.catirclogs.org/rust-embedded/2026-09-29#1790705260-1790706013) and at the time concluded that while this is a useful improvement to the API, because it's also a breaking change to an ecosystem-wide shared trait we don't want to merge it to master right now and instead save it for a future breaking release. I left a comment in the meeting minutes but forgot to comment here and it looks like GitHub didn't cross-link them.

So, thank you for the PR and the nice improvement. For now I think we'll keep the PR open and plan to merge it alongside any other breaking changes, but there aren't any planned right now. We could also merge to a new branch for a future release, but I think keeping the PR open will have better visibility.