Car docs bot: diff on generated readme file

[sshane]

We have a bot that uses the car docs Python data structures and prints a pretty diff to each PR, if it changes any car docs. That could be vehicle name, supported trim, minimum steering speed, parts, or detail sentence for the website.

Current bot output: #31888 (comment)

Our current solution requires checkouts and pickling, which is fragile: print_docs_diff.py and selfdrive_tests.yaml#L256-L308.

Solution

Just use the base and head commit CARS.md files in the PR, for example:

wget -O master_table.md https://raw.githubusercontent.com/commaai/openpilot/master/docs/CARS.md
diff master_table.md docs/CARS.md

See the generated CARS.md README file: https://github.com/commaai/openpilot/blob/master/docs/CARS.md

Diffing behavior:

• Should be agnostic to the columns and other changes (you can add or remove without issues)
• Minimal understanding of the file, which makes this small, simple, and robust. You may need special handling of the detail sentence however, which is fine.
• Changes to column values should be picked up as a column change, not addition and removal. GitHub already does that for us!

[agniv-the-marker]

Hey! I think I have code that's almost there but I'm not sure how you're supposed to check the detail sentence? After looking at the code, the best I can come up with is:

def get_detail_sentence(data):
  make, model, package, longitudinal, fsr_longitudinal, fsr_steering, steering_torque, auto_resume, hardware, video = data.split("|")
  min_steer_speed, min_enable_speed = None, None # default values
  sentence_builder = "openpilot upgrades your <strong>{car_model}</strong> with automated lane centering{alc} and adaptive cruise control{acc}."
  if min_steer_speed > min_enable_speed:
    alc = f" <strong>above {min_steer_speed * CV.MS_TO_MPH:.0f} mph</strong>," if min_steer_speed > 0 else " <strong>at all speeds</strong>,"
  else:
    alc = ""
  acc = ""
  if min_enable_speed > 0:
    acc = f" <strong>while driving above {min_enable_speed * CV.MS_TO_MPH:.0f} mph</strong>"
  elif auto_resume:
    acc = " <strong>that automatically resumes from a stop</strong>"
  if steering_torque != STAR_ICON:
    sentence_builder += " This car may not be able to take tight turns on its own."

  return sentence_builder.format(car_model=f"{make} {model}", alc=alc, acc=acc)

This, however, clearly doesn't work because *_speed are both None and when these are initialized they utilize carParams, which rely on more information than what is in the markdown? (This is also why the experimental stuff is removed).

If there was any way around this that maintained functionality it would let me finish this code, but I'm not sure of a solution that isn't just use pickle again. I'll keep looking.

[agniv-the-marker]

I figured out the first thing* it's just a conversion.

Experimental is stranger...

[sshane]

You can try to put the sentence in the row as a comment, or better yet as a column (but there's so many it would probably regress readability of the rest as they get smooshed)

[agniv-the-marker]

Oh, you mean like a whole reworking of CARS.md to include a new column (it would definitely make it harder to read, though you could do something similar to what the "hardware needed" section does).

This seems kind of a lot for just a diff visualizer though since it's just taking a row from the CARS.md and trying to find the values of openpilotLongitudinalControl, experimentalLongitudinalAvailable... I think you'd need to rewrite the autogenerator for CARS.md which shouldn't be too much (to include columns on those pieces of information).

[agniv-the-marker]

Yeah, I think you have to throw this into the markdown file because it just seems wildly inconsistent across makes/models:

[sshane]

I would also accept a tooltip/hover text with the sentence and a small amount of parsing specific for it (the sentence is mainly viewable on our website at https://comma.ai/vehicles, we only care about the diff here though)

Our auto generated docs site looks partially broken, would be nice to fix that too! https://docs.comma.ai/CARS.html#supported-cars

[agniv-the-marker]

I'm sorry if I'm being unclear, but from how the code seems to be set up, there is no immediately obvious way to access if a car has access to experimental mode from just the markdown files. Because of this detail sentences cannot immediately be generated to include those :(

From looking at https://comma.ai/vehicles though, it seems like there should just be a tracker of car -> experimental if you expect this to CHANGE over time.

If you do not expect this to change, then there's no real reason to track it in the diffs :P

For the broken thing, no clue why hardware gets shifted all the way there.

[sshane]

It's just an attribute on the docs object, you can do {{car_docs.detail_sentence}} in the jinja template and regenerate

[agniv-the-marker]

Should be fixed up now! Just left it as a sentence, wasn't really sure what you would be hovering over :>