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

WIP [$500 bounty] test_models: add a test that fuzzes the tx messages #32577

Closed
wants to merge 8 commits into from

Conversation

ksfi
Copy link
Contributor

@ksfi ksfi commented May 30, 2024

Concerning bounty #32425

Generating tests for CarControl messages and check if tx is False when controls_allowed if False
Lmk if I'm on the right path @sshane

@github-actions github-actions bot added CI / testing car vehicle-specific labels May 30, 2024
Copy link
Contributor

github-actions bot commented May 30, 2024

Thanks for contributing to openpilot! 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
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

@ksfi ksfi marked this pull request as draft May 30, 2024 18:10
Comment on lines 45 to 46
@st.composite
def car_control_strategy(draw):
Copy link
Contributor

@sshane sshane May 31, 2024

Choose a reason for hiding this comment

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

you need to have openpilot generate these CAN messages, we want to ensure that panda doesn't block anything openpilot will try to send to start to this test

Copy link
Contributor Author

@ksfi ksfi Jun 2, 2024

Choose a reason for hiding this comment

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

oh ok so it should somehow get those CAN messages from state_control in the Control class from controlsd.py with CarStates obtained with fuzzed data

now_nanos = DT_CTRL * 1e9
CI = self.CarInterface(self.CP, self.CarController, self.CarState)

if self.CP.carName == "honda":
Copy link
Contributor

Choose a reason for hiding this comment

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

this test should know as little about the cars/brands as possible

if self.CP.carName == "honda":
if ((addr == 0xE4 or addr == 0x194) and (CC.latActive and act.steerOutputCan != 0)) \
or (addr == 0x296 and bus == bus_buttons and CC.cruiseControl.cancel):
self.assertFalse(tx_ok, "issue TX")
Copy link
Contributor

@sshane sshane May 31, 2024

Choose a reason for hiding this comment

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

same, should not hard code addresses here. should also be a test that asserts true

@ksfi
Copy link
Contributor Author

ksfi commented Jun 14, 2024

@sshane CC messages are now generated by controlsd that's fed with the data required via hypothesis
maybe some other processes can be run to avoid fuzzing that much data lmk

@sshane
Copy link
Contributor

sshane commented Jun 25, 2024

This should really follow the same structure as the rx fuzzy test, going to close. Feel free to retry if you have a simpler approach. This should be simple and provide a high leverage amount of coverage, without starting processes or manually defining message structs (we have https://github.com/commaai/openpilot/blob/master/selfdrive/test/fuzzy_generation.py for that)

Again we should start by asserting that any message the car controller generates should be accepted (tx=1) by panda safety.

@sshane sshane closed this Jun 25, 2024
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.

2 participants