Adds extended ID flag to extended CAN IDs

marcelbuesing / socketcan-isotp

ISO-TP socketcan library for Rust

23 stars 12 forks source link

Adds extended ID flag to extended CAN IDs #1

Closed thomasantony closed 3 years ago

thomasantony commented 4 years ago

Thanks for making this library. It's been very helpful!

I found that the library cannot directly use extended CAN ids and it is not documented anywhere that it is so. I am pretty new to this stuff and took a bit of reading to figure out the extended flag bit.

This is a small change that should make it a bit easier. This makes it so that the library works with extended IDs for source and destination addresses regardless of whether the user set the correct flag.

brainstorm commented 3 years ago

I actually need this change and would like to see it merged/published, shall I declare that constant or can you push it @thomasantony (thanks, btw!)?

marcelbuesing commented 3 years ago

I actually need this change and would like to see it merged/published, shall I declare that constant or can you push it @thomasantony (thanks, btw!)?

After discussions in the socketcan crate related to something similar I think it might be better to actually use proper types for identifiers. Would you mind giving the embedded-can branch a try, I just pushed some changes? For the following reasons I think that would be a better approach.

Even a small identifier might be used in a frame that has the extended frame bit set to 1.
Extended frame ids might accidentally exceed the 18bit available for extended ids, so it makes sense to validate them.

brainstorm commented 3 years ago

Thanks a ton @marcelbuesing for the speedy response, your latest commit on embedded-can branch worked fine for me and my hardware/usecase!

Unfortunately I'm facing another challenge with this crate, but it's unrelated to the ExtendedId, so I'll open a separate issue for it.

At this point I'd close this PR (since the types mechanism is fairly clear) and perhaps merge the embedded-can branch to master and release to crates.io?

Kudos!

marcelbuesing commented 3 years ago

I just released 1.0.0 to crates.io containing the changes.