test_models: fuzz test panda and CarState #30443
Merged
Conversation
This file contains 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
sshane
commented
Dec 19, 2023
Comment on lines
+332
to
+337
| prev_panda_gas = self.safety.get_gas_pressed_prev() | ||
| prev_panda_brake = self.safety.get_brake_pressed_prev() | ||
| prev_panda_regen_braking = self.safety.get_regen_braking_prev() | ||
| prev_panda_vehicle_moving = self.safety.get_vehicle_moving() | ||
| prev_panda_cruise_engaged = self.safety.get_cruise_engaged_prev() | ||
| prev_panda_acc_main_on = self.safety.get_acc_main_on() |
There was a problem hiding this comment.
Ideally we re-use the warm up code in test_panda_safety_carstate (or that whole function), but so far I haven't found a nice way to generate examples inside the function. Will do some more looking after this is merged
|
Last 5 runs:
|
sshane
commented
Dec 19, 2023
|
This is a significant slowdown to GH actions. Can we lower it to 15s? |
|
I measured master and this branch at different times, but the performance you get on GH Actions seems inconsistent through time. Now on master after this change, it's literally only ~15s slower than the times I posted here. |
|
Ok let's monitor it and reduce if we see slowdowns >15s. I would like all CI to finish <5m soon. We're close to that, but we can't regress too much. |
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.
Actually finding lots of legitimate mismatches between openpilot/CANParser and panda safety:
CANParsermaintains values from invalid msgs invl_allwhen it shouldn't add them in the first place, causing bad brake press values in CS when it gets a valid msg - CANParser: process all signals before updating values opendbc#977CHECKSUMbeforeCOUNTER, leading to counter not always getting updated properly if checksum is invalid - CANParser: process all signals before updating values opendbc#977counter_failto the max opendbc#976VW PQ: standstill checks don't match, panda doesn't check counter fordashcam!MSG_BREMSE_1butCANParserdoessafety: check interceptor msg counter panda#1738safety: add safety param for gas interceptor panda#1735Tesla CS and safety check two different messages fordashcam!standstill/vehicle_moving, probably becauseESP_Bisn't on both PT and the other bus. @robbederks can we just make openpilot useDI_torque2? This also fixes possible mismatches for the angle safety since it uses vehicle speed - Tesla: match vehicle_speed message openpilot uses panda#1256BRAKE_MODULEcheck to the same set asPOWERTRAIN_DATAwhich always exists, so it randomly selects an address and only checks one message (Honda Bosch: add alt brake msg to rx checks panda#649) - Honda: fix alt brake address check race condition panda#1746