DBC VAL_ parser

[dek3rr]

This parser, CANDefine, pulls definition-value pairs from the respective dbc. It shares a similar call structure with CANParser.
For example:
given,
VAL_ 399 STEER_STATUS 5 "fault" 4 "no torque alert 2" 2 " no torque alert 1 " 0 "normal" ;
the value of 2 corresponding to "no torque alert 1" is callable by:
cd.dv["STEER_STATUS"]['NO_TORQUE_ALERT_1']
Repeated lines in the dbc are removed, leading and trailing white space is removed in each def/val pair, and naming is converted to uppercase with underscores for convention.
This is a very forgiving parser, but it requires a unique naming call--this is by design
VAL_ 506 FCW 3 "fcw" 2 "fcw" 1 "fcw" 0 "no_fcw" ;
will only return FCW : 1 and NO_FCW : 0.

This will greatly reduce gear shifter logic in carstate and also removes other hard coding.

[rbiasini]

Thanks for working on this. This will improve readability of the carstate.

I’m not sure why we need to change the C part of the CANParser though. Why can’t we just use the python regex to build a dict containing all the mappings from int to enum values? When creating the python instance of the CANParser we store this dict as an attribute.

So a lookup in carstate will look like this:
self.stopped = cp.vl["ACC_HUD"]['CRUISE_SPEED'] == cp.dv["ACC_HUD"]["CRUISE_SPEED"]['STOPPED']

[dek3rr]

I originally had it as an attribute, but we aren't actually parsing the CAN so I thought it made more sense for it to live separately beside CANParser instead of inside it. The deciding factor for me was that certain definitions may also be useful outside of carstate so it wouldn't always make sense to me to call CANParser in those situations. CANDefine would be more lightweight.

Sorry if I'm not understanding correctly, but I believe you're asking why I didn't just build the dictionary in the regex dbc.py file. I would argue that it makes it clearer to have everything follow the same call path into parser.py and built the same way as CANParser.

Parsing definitions is very closely related to parsing messages and signals but is also distinctly different since they are static. This was just my solution to keep things uniform while maintaining some separability.

[jfrux]

@dekerr Just gotta step in and say you sound very knowledgable and you sound like you've thought this through. I think we need a bit of separation of concerns for sure in the codebase and a more maintainable and in the end TESTABLE codebase. This codebase is lacking a lot of modern unit testing and could definitely get there if we get methods smaller and make things more approachable.

My random 2 cents... :)

[rbiasini]

@dekerr yeah, I actually like CANDefine and in general being able to access dbc file info in a lightweight way, without calling CANParser. Let me think a bit more about it.

[rbiasini]

@dekerr merged this change internally. The only mod to CANdefine I did is to change the dv dict so that there is an extra layer: you need to specify the message too (either by address or name). Indeed, signal names are not meant to be unique in a dbc.
Basically you look up by cd.dv["GEARBOX"]["GEAR_SHIFTER"] instead of simply cd.dv["GEAR_SHIFTER"]
So, closing this PR as it will be released in 0.5.2. Nice work!