Add option to generate #defines for the Signal names when generating C source. #548
Conversation
Pull Request Test Coverage Report for Build 4765020020
💛 - Coveralls |
README.rst
Outdated
|
|
||
| .. code-block:: text | ||
|
|
||
| $ python3 -m cantools generate_c_source --include_signal_names tests/files/dbc/motohawk.dbc |
There was a problem hiding this comment.
that option should use minuses instead of underscores (--include-signal-names) and it should probably be called --generate-signal-name-macros. That said, I don't really see any harm in including these macros unconditionally, if it is decided that we want them...
| @@ -0,0 +1,175 @@ | |||
| /** | |||
There was a problem hiding this comment.
this, and the corresponding .c file in this PR, seems to be spurious: It should be generated as part of running the tests?
|
Updated based on feedback. Thank you |
| @@ -0,0 +1,211 @@ | |||
| /** | |||
There was a problem hiding this comment.
you also need to remove this file from the PR...
|
thanks. Can you please explain what the advantage of these macros relative to just using the strings with the names directly is? |
|
Using "signal_name" is open typos and errors, whereas a #define will allow the IDE to predict and highlight errors, also a static analyzer will also detect this before you get to running the tests. |
|
fair enough. can you put the message names through the same treatment? |
|
Yes sure :) Do you want the message(frame) names to be optional too or lumped in with the signal names? |
|
As I see it there's no reason not to generate all of these macros unconditionally. (I might have missed something...) |
|
Makes sense to me... other then then files will be a little bigger, but the compiler will removed anything that is unsed. |
|
disk storage is cheap these days ;) (compile time might be another issue, but I doubt that this will be more than a few milliseconds even for pretty large databases.) |
|
Done
|
tests/files/c_source/vehicle.h
Outdated
| #define VEHICLE_RT_SB_GPS_TIME_VALIDITY_GPS_TIME "Validity_GPS_Time" | ||
| #define VEHICLE_RT_SB_GPS_TIME_VALIDITY_GPS_WEEK "Validity_GPS_Week" | ||
| #define VEHICLE_RT_SB_GPS_TIME_ACCURACY_GPS_TIME "Accuracy_GPS_Time" | ||
| #define VEHICLE_RT_SB_GPS_TIME_GPS_TIME "GPS_Time" |
There was a problem hiding this comment.
hm, I suppose prefixing these macros with NAME_ (or using a _NAME postfix) would probably be a good idea to avoid naming collisions with other stuff...
|
all green, let's merge. Thanks for your contribution. |
This PR adds a command line option to include #defines for the CAN Signal names.
Format
This PR for issue: #547