[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

[jyoung8607]

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

Reasonable, and thanks for doing it standalone (#2510) so it's easy to cross that bridge when we come to it.

do we really need this weighting?

Been awhile, and haven't looked at all Honda, but for at least one example the wheel speed sensor signals all drop to zero below about 2mph or thereabout. Not usable for FtS/SnG purposes. It may be possible to discard the wheel speeds entirely and go with the single transmission output speed signal.

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.

I like this idea, as a future-someday thing.

can we remove carState.standstill or just define it as vEgoRaw < 0.1?

Stock ACC resume spam is conditioned on standstill (commaai/openpilot#24873) and VW don't like to be spammed with the drivetrain coordinator in the wrong state (commaai/openpilot#22176). Haven't checked other ports.