Conversation
…Necessary for fixing 25389 in OP and eventual dashcam flag removal for Ram HD trucks
There was a problem hiding this comment.
Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:
- Convert your PR to a draft unless it's ready to review
- Read the contributing docs
- Before marking as "ready for review", ensure:
- the goal is clearly stated in the description
- all the tests are passing
- include a route or your device' dongle ID if relevant
|
Another approach is to directly set front and rear tire stiffness in CarSpecs. e.g:
This allows tireStiffnessFactor customization to be deprecated and retired. |
|
Here is an implementation of the alternate approach above: |
|
I performed some informal testing with Rivians and am changing my recommendation to setting front and rear values directly. Rivians have an oddly scaled stock tire stiffness where the rear is 50% higher than the front and may benefit from tuning these values individually. This approach is also perfectly backwards compatible, no existing cars are affected by this change. PR is updated with the new proposed approach. |
|
This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity. |
|
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
Car behavior reportReplays driving segments through this PR and compares the behavior to master. ✅ No changes detected |
|
This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity. |
|
This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity. |
Purpose
This PR addresses the dual use of tireStiffnessFactor by decoupling its use for scaling the Civic reference constants from its use in paramsd as a live parameter. It fixes the paramsd errors reported in commaai/openpilot#25389 and is necessary (but not sufficient) for moving Ram HD (2500/3500) trucks out of dashcam mode.
History and Analysis
tireStiffnessFactor is a dimensionless value used to scale the reference Civic constants TIRE_STIFFNESS_FRONT and TIRE_STIFFNESS_REAR using the vehicle's mass and center of gravity relative to the Civic. The function used is scale_tire_stiffness() and it was introduced in commaai/openpilot#720
When paramsd was later introduced (2022?), it incorporated the tireStiffnessFactor as an online learned live parameter. The implementation assumes that tireStiffness factor is 1.0 and establishes boundaries of 5x up and down for the tireStiffnessFactor, and 2x up and down for steerRatio. These two values are closely linked in the learner. The paramsd service also explicitly requests resetting the stiffnessFactor to 1.0 after every drive.
Vehicles using a tireStiffnessFactor of 1.0 are unaffected by the coupling of static scaling and dynamically learned parameters. However, vehicles that set a custom tireStiffnessFactor (currently Honda, HKG, Mazda, Toyota, possibly others) have an increasing probability of crossing the paramsd boundaries and causing errors as the learned values approach their fixed boundaries of 5x/2x.
This proability of error is higher than cars set to 1.0 because the learner does not rebase the factor at 1.0 and instead learns from the factor value set by the CarSpecs. So the risk of developing paramsd errors increases in probability the closer the tireStiffnessFactor is set to 0.2 (and 5.0 but no vehicles scale up to date).
Justification
In the case of Ram HD trucks, the observed post-scaling stiffness values (in N/rad) need to be at or lower than the lowest scaling possible of 0.2 in order for paramsd to reach a stable solution. See commaai/openpilot#25389 for discussion of the reasons why (high mass, plus solid front axle, and other steering dynamics that are encapsulated in the tire stiffness value).
Due to the way that scale_tire_stiffness() scales using mass and center of gravity, a value of 0.2 is not outside of the normal values in OP when you look at the post-scaling value used in the vehicle dynamics model. Setting a HD truck to 0.2 yields the lowest "allowed" stiffness values of 86,873 N/rad front and 107,901 N/rad rear. This is a sampling of front/rear reference values measured in N/rad from other OP supported vehicles:
115,535/111,939 - Acura RDX (factor = 0.444)
95,078/100,199 - Hyundai Kona EV (factor = 0.385) - almost all Hyundais scale down to this range with 0.385
129,372/136,340 - Toyota Sienna (factor = 0.444)
Proposed Fix and Backwards Compatibility
Reasons to apply this PR:
Red diffUpdated for backwards compatibilityFor vehicles currently set to tireStiffnessFactor = 1.0, there is no impact from this change.For vehicles that set a custom tireStiffnessFactor, the Civic scaling is unchanged, but the arbitrary factor the learner starts from is rebased to 1.0. This will be visible in the logs and PJ since all drives will show tireStiffnessFactor starting at 1.0 with this change, instead of the current behavior of starting at the factor value.For discussion: Should this be implemented in a different way for deprecation or backwards compatibility reasons? As implemented in this PR, perfect backwards compatibility for vehicles with customized factors can be achieved by setting the factor (e.g. "ret.tireStiffnessFactor = 0.385") in _get_params() for that platform or vehicle. However, it is unclear if this is necessary.Updated PR is now fully backwards compatible.Testing
TBD based on discussion and final implementation, but I can provide logs of Ram trucks before and after this change