Conversation
…Necessary for fixing 25389 in OP and eventual dashcam flag removal for Ram HD trucks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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