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

Volkswagen PQ sfety: check counter for ABS wheel speed message #1717

Closed
wants to merge 1 commit into from

Conversation

sshane
Copy link
Contributor

@sshane sshane commented Nov 11, 2023

This causes a mismatch for commaai/openpilot#30443. @jyoung8607 any reason not to check counter? It was switched from a different message without a counter here, and this passes test_models.py: #730

@sshane sshane added the car safety vehicle-specific safety code label Nov 11, 2023
@sshane sshane changed the title safety: check counter for ABS wheel speed msg Volkswagen PQ sfety: check counter for ABS wheel speed msg Nov 11, 2023
@sshane sshane changed the title Volkswagen PQ sfety: check counter for ABS wheel speed msg Volkswagen PQ sfety: check counter for ABS wheel speed message Nov 11, 2023
@sshane sshane added the bugfix label Nov 11, 2023
@adeebshihadeh adeebshihadeh deleted the pq-check-counter-Bremse_1 branch November 29, 2023 00:06
@jyoung8607
Copy link
Collaborator

Sorry, just saw this. It's okay to check the counter so far as I know. Not sure why this is closed, LMK if you want a PR for it.

@adeebshihadeh
Copy link
Contributor

Just closed because I didn't want us spending any more cycles on dashcam'd cars. If you open a PR that just works, happy to merge.

@sshane
Copy link
Contributor Author

sshane commented Dec 9, 2023

@jyoung8607 fyi, this just worked in the fuzzy carstate test when I opened it, so should be fine to open and run test_models on a few segments if you have any

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

Successfully merging this pull request may close these issues.

3 participants