[woodruffw]

I'm not anywhere near a specialist when it comes to car software, CAN, or self-driving, but I'm concerned by some of the code that (appears to) control some of the supported cars: I pulled up a random file[1], and it's littered with undocumented bytestrings that are apparently sent over the CAN bus, as magic constants (with accompanying comments that mention "aggressive" and "laggy" behavior).

I have no insight into any of the proprietary work in this space, so I have no idea whether this is unique or commonplace. But it certainly isn't confidence inspiring, particularly as a cyclist.

[1]: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85...

[pd0wm]

The file you referenced is related to Ford, which is not a supported car/brand. The code for Ford is provided as a reference and is forced into “read-only” both by openpilot and panda (the bridge between openpilot and the car).

[kelnos]

On one hand I agree with you wholeheartedly (I also think python is a weird language choice for something that probably has some hard real-time requirements), but also consider that stuff like that may very well be present in the code bases of the closed-source commercial solutions, but unless someone who has worked on one of them speaks up, we'll never know.

[DesolationJones]

There's openpilot code, and then there's panda code that runs on a separate real time micro chip. All the safety relavent code runs on the panda, and it's written in C following misra c guidelines. They also comply with iso 26262.

See more info about their safety model in the safety architecture section of this blog post.

https://comma-ai.medium.com/how-to-write-a-car-port-for-open...

[sjburt]

If actual controls engineers are involved, it's probably going to a be a terrifying blob of C autogenerated from Simulink or something like that.

[woodruffw]

Yeah, that's why I couched my critique a little bit: I have no insight whatsoever into the commercial side of this and, for all I know, it could be even worse!

But that alone is bloodchilling: it terrifies me, as a pedestrian and cyclist, to think that stuff like this is probably controlling the pieces of machinery that could kill me in a split second.

[sjburt]

In their system, the actual real-time safety critical code is elsewhere, on a dedicated processor. The python code sends messages to the safety code, which applies limits and sends commands to the car hardware. And other safety systems like AEB are totally untouched and still functional.

The safety code has a limit on steering ramp rate and a max torque limit. The driver can easily overpower the torque limit, and the ramp rate limit means it won't suddenly jerk.

The python code I think is temporary, they ultimately want to do end-to-end models that do both perception and control (rather than a model to do perception and traditional controller to do control). If that effort fails I think they would probably replace the python code, the current stuff is definitely amateurish.

[loganboyd]

George just said today during a Q+A session that they do not intend to do learned control because traditional control is already greatly exceeds human ability.

[nanis]

Ouch. The method is called `update` and it is littered with stuff like:

   if (frame % 10) == 0:

      can_sends.append(make_can_msg(1648, b'\x00\x00\x00\x40\x00\x00\x50\x00', 1))
      can_sends.append(make_can_msg(1649, b'\x10\x10\xf1\x70\x04\x00\x00\x00', 1))

      ...

I know the identifiers etc are probably hard to catalog, but that is all the more reason to give things symbolic names and maybe even avoid having to specify those IDs repeatedly.

[pd0wm]

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

This results in much cleaner car abstraction layers: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85...

[woodruffw]

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/de0ce142ae51cf9c85...

[2]: https://github.com/commaai/openpilot/blob/de0ce142ae51cf9c85...

[pd0wm]

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.

[solarkraft]

There’s some messing around involved in building the interfaces to the cars. Comma’s excuse for why you shouldn’t be concerned is that both Open Pilot and the cars themselves have limits against erratic movements.

[woodruffw]

> Comma’s excuse for why you shouldn’t be concerned is that both Open Pilot and the cars themselves have limits against erratic movements.

Are these limits baked into the cars at some level beneath Comma, or do they rely on the same physical layers and networks? It makes perfect sense to me that the car has its own limits; my concern is that installing Comma takes the car outside of its expected operating parameters and that I have no real way of verifying whether those limits are still in place.

[DesolationJones]

They have a seperate device that enforces more conservative limits than what the car allows.

https://comma-ai.medium.com/how-to-write-a-car-port-for-open...

Any safety limits like on the eps firmware stays intact and aren't bypassed.