Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added define's for CAN message ID's + increased specialization of enu… #40

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

warrickw
Copy link
Contributor

@warrickw warrickw commented Feb 10, 2023

I've made 4 changes in my repo:

  • Tweaked the parser val and val_item by moving where a space is. It wasn't parsing my values because it was expecting a space after each val_item, which the last item won't have. I've moved it to expect a space before each item.
  • Added generation of #define's for the CAN ID's of the messages
  • Updated the enum value generation to generate the message name
  • Updated the encode/decode function generation to include the message name

Considering the following value list - there's two signals called state, but for different message ID's (1 and 2):

VAL_ 1 state 0 "Inactive" 1 "Active";
VAL_ 2 state 3 "OFF" 4 "ON";

Generated enum definition before change (won't compile):
image

After (now generated after corresponding message definition):
image

(After, but without the -N flag to remove the message ID's)

image

Q:
I'm unsure what effect my proposed changes have to the global VAL_TABLE_ section in a dbc file. I suspect they won't get generated after these changes, because I'm only generating enums for vals that are associated with a message.

@howerj
Copy link
Owner

howerj commented Feb 10, 2023

I'll take a look when I can (maybe this weekend?), thanks for the detailed pull request though. A quick glance suggests it is sensible, although I would be worried it could break existing code that use the current mechanism as this would be a big breaking change.

@howerj
Copy link
Owner

howerj commented Feb 16, 2023

I think I will have to hide this behind an options flag (or the old behaviour) so as to not break existing code. I'll merge this on to a branch and then work on that, I think.

@howerj howerj changed the base branch from master to feature/better-enums February 16, 2023 10:56
@howerj howerj merged commit 1ad05ae into howerj:feature/better-enums Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants