Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_models: add tx fuzzy test #32897

Closed
wants to merge 10,000 commits into from
Closed

Conversation

bongbui321
Copy link
Contributor

@bongbui321 bongbui321 commented Jul 3, 2024

resolve #32425

Uses FuzzyGenerator to generate fuzzy CarControl messages

hypothesis example generation is extremly slow.

Nissan speed: 4 wheels in OP and 2 rear wheels in panda
Nissan steering angle out of -1311 - 1310 -> CANPacker wrong value
Tesla has clip to current steering angle

adeebshihadeh and others added 30 commits June 5, 2024 17:32
* stash

* staaaaash

* clean up

* more clean up

* fix

fix

* fix

* stash

clean up

* more

* this is just simpler to understand

* explicitly check
…ctionary (commaai#32592)

Optimize gear shifter parsing for improved performance
* Loading torque data only once and reusing it across function calls.

* apply review

* still check only one entry

* fix that

---------

Co-authored-by: Shane Smiskol <[email protected]>
* improve performance

* remove DEPRECATED

* formatting

* catch kjException

---------

Co-authored-by: Shane Smiskol <[email protected]>
* improve prev event counters

* just rename

---------

Co-authored-by: Shane Smiskol <[email protected]>
increase lag in optima to 0.2
* squash

* fix doc

* compile device

* compile device

* Update ref

* add msgq to precommit exclusions

* No service ports

* fix compile

* address comments

* More comments

* Delete logger

* Update opendbc

* Linting

* bump msgq
* Dead cereal stuff

* Dead code

* also dead

* More cleanup
* rm generate_javascript.sh

* Unused
* blacklist more files from release

* dbcs

* fix

* just use the lfs

* rm that too

* cleanup

* cleanup
* card: remove old canRcvtimeout field

* deprecate
* bump

* bump2

* bump3
* remove wrapper that is functools.cache

* format
* is this even used?

* remove
* pandad: fix loopback test

* clear all params

* try this

---------

Co-authored-by: Comma Device <[email protected]>
…Non-US only) 2020-23" entry (commaai#32658)

The term "Non-US" in cars.md for the "Corolla Hybrid (Non-US only) 2020-23" is not precise enough and should be changed to South America

Users may think "Non-US" includes Europe or Canada, which is not the case for this entry.

It has two issues:

* The 2023 Corolla Hybrid in Europe and Canada include a new Toyota Safety Sense 3.0 that has yet to be supported due to it being a new version and the presence of Toyota Security Key.
* The 2020-2022 Corolla Hybrid in Europe is the same as the 2020-2022 Corolla Hybrid in North America, which is supported by Openpilot and has full-speed ACC due to the presence of an electronic parking brake which is not present in the 2020-2022 Corolla Hybrid in South America and the reason why it is listed as not having full-speed ACC.

The entry should be reverted to at least "Corolla Hybrid (South America) 2020-23" which was proposed but not merged in the pull request:

commaai@28454c0

Pull request for that change that does not include that commit: commaai#26943

Examples of confusion throughout the community:

comma.ai Discord:

https://discord.com/channels/469524606043160576/524327905937850394/1235264758580772996

https://discord.com/channels/469524606043160576/524327905937850394/1228090600272691370

https://discord.com/channels/469524606043160576/954493346250887168/1209952008329633813

https://discord.com/channels/469524606043160576/524327905937850394/1182093384647721140

https://discord.com/channels/469524606043160576/954493346250887168/1147751657543848017

openpilot community Discord:

https://discord.com/channels/771493367246094347/771495215570747403/1247737844727021629

> I think that the supported cars list (https://github.com/commaai/openpilot/blob/master/docs/CARS.md) is incorrect.
>
> I have a UK 2020 Toyota Corolla Hybrid
>
> In the cars list it's listed as "no accel below" 17 mph and no resume from > stop.
>
> This is not my experience. For me it works just fine down to a complete stop and it can resume from a stop too.
>
>

CC: @AlexandreSato
* Replace markdown-it-py with small function in common/

* simple test

* unused

* lock

* linting fixes
haraschax and others added 2 commits July 17, 2024 16:17
* add livepose

* Formatting

* Add to sevices

* Update locationd to publish livePose

* Remove fields and increase decimation

* Fix field indices

* Remove the line

* Add livePose to pubmaster

* Fix llk decimation

* Update ref commit

* XYZ measurements instead of lists

* Update locationd

* Update ref commit

* Lower the qlog size in test_onroad

* Update lower and upper boundary

---------

Co-authored-by: Kacper Rączy <[email protected]>
reduce cameraOdometry decimation
@bongbui321
Copy link
Contributor Author

Might merge tx fuzzy with carstate fuzzy.

Occasionally catches commaai/panda#1948. Not entirely sure why hypothesis doesn't consistenly repro that failed data generation

@bongbui321 bongbui321 force-pushed the add_fuzzy_tx branch 2 times, most recently from 057ec11 to 894e653 Compare July 19, 2024 17:28
@bongbui321 bongbui321 marked this pull request as ready for review July 19, 2024 21:37
@bongbui321
Copy link
Contributor Author

@sshane ready for review

Copy link
Contributor

@adeebshihadeh adeebshihadeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we merged all your bugfixes yet, so how is this passing? Are we not getting good coverage on a single run?

If so, what would it take to hit all the known bugs on a single run? How long would that take in CI?

@@ -323,10 +329,13 @@ def test_car_controller(car_control):
@settings(max_examples=MAX_EXAMPLES, deadline=None,
phases=(Phase.reuse, Phase.generate, Phase.shrink))
@given(data=st.data())
def test_panda_safety_carstate_fuzzy(self, data):
def test_panda_safety_carstate_tx_fuzzy(self, data):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's split the new TX stuff into a separate test case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh is this merged for speed?

@bongbui321
Copy link
Contributor Author

I don't think we merged all your bugfixes yet, so how is this passing? Are we not getting good coverage on a single run?

I don't think we merged all your bugfixes yet, so how is this passing? Are we not getting good coverage on a single run?

Nissan max steering angle [issue](https://github.com/commaai/openpilot/issues/32978) is opened and included in this PR

Tesla [issue](https://github.com/commaai/openpilot/issues/32999): This might happen with a few runs, and I suspect that if run locally, hypothesis will try to recreate that failing examples pretty consistently. That requires a more thoughtful fix, which I'm hesitant to open a fix PR since I'm not familiar with the expected behaviour of the platform

The Nissan speed issue is addressed here:

    # Nissan calculate vEgoRaw with avg speed of 4 wheels
    # while panda get vehicle_speed from rear wheels only,
    # so there might be a big enough difference between the two
    if self.CP.carName == "nissan":
      panda_vehicle_speed_last = self.safety.get_vehicle_speed_last() / VEHICLE_SPEED_FACTOR
      if abs(self.CI.CS.out.vEgoRaw - panda_vehicle_speed_last) > 0.2:
        cs_msg = self.CI.CS.out.to_dict()
        cs_msg["vEgoRaw"] = panda_vehicle_speed_last
        self.CI.CS.out = car.CarState.new_message(**cs_msg).as_reader()

This realistically shouldn't happen front wheels and rear wheel speed should be close

@adeebshihadeh
Copy link
Contributor

Can you rebase?

@maxime-desroches
Copy link
Contributor

This PR was closed because of a git history rewrite.
Please read #33399 for what to do with your fork and your PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
car vehicle-specific CI / testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_models: add a test that fuzzes the tx messages