Skip to content

Conversation

@ad3154
Copy link
Member

@ad3154 ad3154 commented Aug 9, 2023

  • Added a way to encode an extended CAN ID from its components.
  • Added an alias "BROADCAST" address which is the same as "GLOBAL". Internally at Raven we call it global but more broadly (and in AgIsoStack++) it seems to be called "broadcast".

It occurs to me that in my GitHub bio I say contributions are my own unless specified, so I should start saying when commits are on behalf of Raven.

So, this contribution is on behalf of Raven.

Added a way to encode an extended CAN ID from its components.
Also added an alias "BROADCAST" address which is the same as "GLOBAL"
@ad3154 ad3154 added the enhancement New feature or request label Aug 9, 2023
@ad3154 ad3154 requested a review from Notgnoshi August 9, 2023 22:14
@ad3154 ad3154 self-assigned this Aug 9, 2023
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

File Coverage Lines Branches
All files 32% 32% 0%
src/driver/can_id.rs 0% 0% 0%

Minimum allowed coverage is 0%

Generated by 🐒 cobertura-action against 1c48543

@ad3154 ad3154 requested a review from GwnDaan August 9, 2023 22:23
Added tests for CanIdEncode
Notgnoshi
Notgnoshi previously approved these changes Aug 9, 2023
@ad3154 ad3154 force-pushed the ca-adg/identifier-encoder branch 2 times, most recently from ded2794 to f601540 Compare August 10, 2023 16:04
@ad3154 ad3154 requested a review from Notgnoshi August 10, 2023 16:05
Notgnoshi
Notgnoshi previously approved these changes Aug 10, 2023
Copy link
Member

@Notgnoshi Notgnoshi left a comment

Choose a reason for hiding this comment

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

offtopic: I like to use https://conventionalcomments.org/ for comments on PRs, because I'm terrible at conveying tone through comments.

When I leave a nitpick: comment, I intend to convey that it's not something major, and am comfortable merging the PR without addressing the comment.

@ad3154
Copy link
Member Author

ad3154 commented Aug 10, 2023

When I leave a nitpick: comment, I intend to convey that it's not something major, and am comfortable merging the PR without addressing the comment.

Yeah, I know 😁 but these are super minor things in this case, I might as well make it as nice as possible now since they're quick, and it helps me learn rust-ways-of-doing-things (which as you all can probably see is something I still need to work on).

This should make logging the error a little nicer since it provides
more useful info.
@ad3154 ad3154 merged commit cf1ae92 into main Aug 10, 2023
@ad3154 ad3154 deleted the ca-adg/identifier-encoder branch August 10, 2023 16:42
Thom-de-Jong added a commit to Thom-de-Jong/AgIsoStack-rs that referenced this pull request Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants