I-CAN-hack / automotive

Rust crate providing a variety of automotive related libraries, such as communicating with CAN interfaces and diagnostic APIs
MIT License
44 stars 8 forks source link

Consider using `automotive_diag` crate for UDS constants #63

Open nyurik opened 4 months ago

nyurik commented 4 months ago

Hi, would you be interested in collaborating on automotive_diag crate which defines constants for UDS and other standards, and allows both std and no-std usage.

pd0wm commented 4 months ago

I have seen your crate and appreciate the work!

I'm not sure if it makes sense to rely on an external dependency for this due to how rust packaging works. If I want to quickly add/modify one of the constants I need to make a PR and wait for a new release of your crate, as you can't push to crates.io when your package depends on a non-released version of a crate.

I do have a refactor planned with a similar structure like you have with a Standard and Extended version to allow proper typing of non-standard commands (instead of just accepting u8 as an argument like I do now).

What did you have in mind for a collaboration? Are there any direct benefits to the end-user for sharing constants between multiple crates?

nyurik commented 4 months ago

@pd0wm thx for responding! So a few thoughts:

So I think we would all benefit from one common implementation rather than going the "lone hero" route. I certainly don't want to be the only hero maintaining my stuff :)

pd0wm commented 4 months ago

I'll check what changes would be needed and consider it. Most likely I wont have time till after Black Hat & Defcon.

Two things I'm missing after a quick glance are a way to iterate over commands (I use strum macros for this), and serde support (although I see a branch with some wip code).

nyurik commented 4 months ago

Sure. Could you give an example of how you would use iteration over enum values? Is this for UI display purposes, or something else? Also, I did try serde with it because I saw you were using it, but the expected string here seems a bit strange. Is this the type of serialization you expect?

insta::assert_snapshot!(
  json,
  @r###"{"command":"DiagnosticSessionControl","command_byte":{"Standard":"DiagnosticSessionControl"}}"###
);
pd0wm commented 4 months ago
nyurik commented 4 months ago

I just released v0.1.8 - should make migration much easier (includes iter & serde & a few other things)