Slimming down carState

[adeebshihadeh]

The current car port backlog is untenable. While this isn't a magic bullet, I think the first step is to strip out anything that's not strictly necessary for openpilot. These fields may have become luxuries we can no longer afford.

Our API to the car is essentially carstate.py and carcontroller.py, and the more we reduce that surface area, the easier porting, fingerprinting, and maintenance will be.

@sshane @jyoung8607 Any thoughts? This is a fairly aggressive pass at carState. As a side benefit, the smaller carState means we can throw more of them in qlogs.

[sshane]

is this used for USER_DISABLE?

[adeebshihadeh]

apparently not

(openpilot)  1 batman@workstation-adeeb2:~/fourpilot$ rg -i clutchpress selfdrive/
(openpilot)  1 batman@workstation-adeeb2:~/fourpilot$

[sshane]

this is for the safety. just pull it from CS.yaw_rate

[sshane]

self.yaw_rate = cp.vl["Yaw_Data_FD1"]["VehYaw_W_Actl"]

[sshane]

this is a huge downgrade...

[sshane]

I think it makes sense to start to remove unnecessary fields

[jyoung8607]

  gasDEPRECATED @3 :Float32;        # this is user pedal only
  brakeDEPRECATED @5 :Float32;      # this is user pedal only
  engineRpmDEPRECATED @46 :Float32;
  genericToggleDEPRECATED @23 :Bool;
  cumLagMsDEPRECATED @50 :Float32;
  wheelSpeedsDEPRECATED @2 :WheelSpeeds;

I'm good with these. Wheel speeds are obviously safety relevant, but we keep the important outcomes in the form of vEgoRaw and standstill. Likewise for gasPressed and brakePressed.

  clutchPressedDEPRECATED @28 :Bool;

Weak preference to keep this. It's just a bool, and it'll be necessary later for a good OP long experience. Cars with ACC and MT tend to stay engaged for a few seconds while shifting, but we want to reject initial entry, and we'll want to freeze or even reset the gas/accel integrator. We're not doing these things yet, but I don't want to lose the prerequisite signals.

  steeringRateDegDEPRECATED @15 :Float32;
  yawRateDEPRECATED @22 :Float32;   # best estimate of yaw rate

Not stoked about losing convenient views of these, given their active use in safety/fault-mitigation in a few ports, but life would go on.

Other fields

• At a glance, canErrorCounter is recorded by card but doesn't seem like it's doing anything useful
• steeringAngleOffsetDeg is Toyota only and could just live there, but also safety relevant, hmm
• Can we consolidate vCruise and cruiseState.speed?
• Likewise vCruiseCluster and cruiseState.speedCluster
• cruiseState.speedOffset appears to be unused

Other ideas

• Some of our Float32s might be represented sufficiently with Float16s
• Perhaps consider a new carSensors type of object for nice-but-optional data, exclude it from qlogs

[adeebshihadeh]

  clutchPressedDEPRECATED @28 :Bool;

Weak preference to keep this.

Removed for now. Easy enough to add back if necessary.

• Can we consolidate vCruise and cruiseState.speed?
• Likewise vCruiseCluster and cruiseState.speedCluster

Potentially. There is a meaningful difference in that cruiseState is the car's own state. Regardless of whether we decide to keep it, this difference is not at all clear, and that should be fixed.

• Some of our Float32s might be represented sufficiently with Float16s

float32 is the smallest type in capnproto.

• Perhaps consider a new carSensors type of object for nice-but-optional data, exclude it from qlogs

So the qlogs were really just a bonus. It's more about separating the necessary vs the nice-to-haves, and at the very least, I think we need a different file if we want to bring back the nice-to-haves.

[adeebshihadeh]

Alright, pretty much done with the first pass at this.

It's clear that there's lot more to do here on a per-port basis. Some examples:

• do we really need this weighting?

  opendbc/opendbc/car/honda/carstate.py

  Line 159 in de7d108

  # blend in transmission speed at low speed, since it has more low speed accuracy

• can we remove carState.standstill or just define it as vEgoRaw < 0.1?
• carState.wheelSpeeds can go once the body doesn't use it for closed loop control on the joystick