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

Tests don't run correctly if PCM and gas are in same message #1028

Open
sshane opened this issue Aug 12, 2022 · 0 comments
Open

Tests don't run correctly if PCM and gas are in same message #1028

sshane opened this issue Aug 12, 2022 · 0 comments

Comments

@sshane
Copy link
Contributor

sshane commented Aug 12, 2022

On Toyota, the PCM status message (PCM_CRUISE) also has the gas pressed bit, so if you for example change some code that should fail a test:

if (!cruise_engaged) {
  controls_allowed = 0;
}

to

if (!cruise_engaged) {
  controls_allowed = 1;
}

it passes, as generic_rx_checks thinks the gas is pressed and just sets controls_allowed to zero for you.

I also had to be careful in the Bolt camera harness PR which has gas and PCM cruise in the same message: #962

I think the real solution here would be to have tests that only ensure that each safety model sets all required global variables correctly, then we have one test that tests all the behavior (that's independent of all the car safety models).

I'm just not sure if it would be considered a regression to our testing, as now we test fully end to end (CAN message to behavior). This proposal would test CAN message to setting variables/some safety state, then the safety state to safety behavior and enforcement.

@sshane sshane changed the title Toyota: PCM cruise enabled test not running Tests don't run correctly if PCM and gas are in same message Aug 12, 2022
@sshane sshane added bug car safety vehicle-specific safety code tests and removed car safety vehicle-specific safety code labels Aug 12, 2022
@sshane sshane linked a pull request Aug 12, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant