Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

CAN: raw ID getter function#669

Merged
MabezDev merged 1 commit into
rust-embedded:master from
us-irs:can-raw-id-getter
Oct 2, 2025
Merged

CAN: raw ID getter function #669
MabezDev merged 1 commit into
rust-embedded:master from
us-irs:can-raw-id-getter

Conversation

@robamu

@robamu robamu commented May 13, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@robamu robamu requested a review from a team as a code owner May 13, 2025 08:04
@robamu robamu changed the title (削除) raw ID getter function (削除ここまで) (追記) CAN: raw ID getter function (追記ここまで) May 13, 2025

Copy link
Copy Markdown
Contributor

What is your use case for this? This has come up before (#428).

As described in the linked PR I’m not a big fan of the proposed API as it makes mixing Standard and Extended IDs easy, something that usually is not intended.

robamu commented May 13, 2025
edited
Loading

Copy link
Copy Markdown
Contributor Author

I just wanted to retrieve the raw ID for quick debugging/printing purposes (after I could not simpy print it with defmt). I did not care about the Standard ID / Extended ID distinction in that case, and I found it weird/unintuitive that I had to unmatch the value.
Basically, it is just an API that I expected to be there.

After going through the mentioned thread for a bit, I can see the point of intentionally making the mixup harder.
The possible mixup of standard and extended ID is something that could be mentioned inside the documentation as well, but I guess this becomes a question about how to design the API and whether to expose methods like this which might increase convenience, but also introduce bugs when not used with care. I have not worked with more complex CAN buses yet with both standard and extended frames (just getting started) where this might become an issue.

robamu commented May 24, 2025

Copy link
Copy Markdown
Contributor Author

What about a raw_id_unchecked method with better documentation?

timokroeger reacted with thumbs up emoji

Copy link
Copy Markdown
Member

after I could not simpy print it with defmt

Have you enabled defmt-0.3 feature?

robamu commented May 24, 2025
edited
Loading

Copy link
Copy Markdown
Contributor Author

The latest main has defmt support, but not 0.4.1. A new release for embedded-can is required

robamu commented Jul 22, 2025

Copy link
Copy Markdown
Contributor Author

@timokroeger I updated the PR

timokroeger reacted with thumbs up emoji

@robamu robamu force-pushed the can-raw-id-getter branch 2 times, most recently from 047043d to 9c72048 Compare July 22, 2025 13:29

Copy link
Copy Markdown
Member

Looks reasonable to me but pinging @rust-embedded/hal for review

@MabezDev MabezDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@MabezDev MabezDev added this pull request to the merge queue Oct 2, 2025
Merged via the queue into rust-embedded:master with commit c4c1077 Oct 2, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MabezDev MabezDev MabezDev approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /