7 ms·

See my other comment about Ford code being reference only. Supported cars have most signals defined in a dbc file, the mapping from name to id and bytes. See h

by pd0wm 5y ago

See my other comment about Ford code being reference only. Supported cars have most signals defined in a dbc file, the mapping from name to id and bytes. See https://github.com/commaai/opendbc https://github.com/commaai/opendbc This results in much cleaner car abstraction layers: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c858fa64e35a59e88ca1596d3/selfdrive/car/volkswagen/volkswagencan.py https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85...

woodruffw 5y ago
Thanks for the reference! That certainly does look much nicer. That being said, I do still see some "magical" looking constants and dictionary keys here[1], as well as lots of small numbers later in the file. I think it would be confidence-inspiring to have those better documented as well. Edit: I found some more magic-looking bytestrings in the Volkswagen support here[2]. [1]: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c858fa64e35a59e88ca1596d3/selfdrive/car/volkswagen/volkswagencan.py#L5-L14 https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85... [2]: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c858fa64e35a59e88ca1596d3/selfdrive/car/volkswagen/values.py https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85...
- pd0wm 5y ago
  At some point the values have no deeper meaning and are just whatever VW decided on as their API. DBC files have concepts of enums, but that’s really diminishing returns and wouldn’t make the code that much easier to understand. Also sometimes there are fixed values in the message that we’ve only observed as static data. Ironically that makes it impossible to reverse engineer what they mean. So those just have to be labeled as hardcoded value. Those are actually magic. Those are the ECU firmware identifiers from the manufacturer that are used by openpilot to recognize which car it’s connected to.

by pd0wm 5y ago

woodruffw 5y ago

Thanks for the reference! That certainly does look much nicer. That being said, I do still see some "magical" looking constants and dictionary keys here[1], as well as lots of small numbers later in the file. I think it would be confidence-inspiring to have those better documented as well. Edit: I found some more magic-looking bytestrings in the Volkswagen support here[2]. [1]: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c858fa64e35a59e88ca1596d3/selfdrive/car/volkswagen/volkswagencan.py#L5-L14 https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85... [2]: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c858fa64e35a59e88ca1596d3/selfdrive/car/volkswagen/values.py https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85...

pd0wm 5y ago
At some point the values have no deeper meaning and are just whatever VW decided on as their API. DBC files have concepts of enums, but that’s really diminishing returns and wouldn’t make the code that much easier to understand. Also sometimes there are fixed values in the message that we’ve only observed as static data. Ironically that makes it impossible to reverse engineer what they mean. So those just have to be labeled as hardcoded value. Those are actually magic. Those are the ECU firmware identifiers from the manufacturer that are used by openpilot to recognize which car it’s connected to.