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

Honda Radarless: only forward buttons frame when controls are off. #1814

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

csouers
Copy link
Contributor

@csouers csouers commented Jan 16, 2024

Related to: commaai/openpilot#31022

Changes:

  • Forward SCM_BUTTONS frame on Honda Bosch Radarless w/ stock ACC when !controls_allowed
  • Add on to button spam test confirming tx of the untested cruise buttons
  • Test the forwarding block of SCM_BUTTONS when controls are on.

@jyoung8607
Copy link
Collaborator

@csouers I'm looking at this one, as a prereq to some other PRs that @vanillagorillaa had open. I see in the openpilot PR history that cancel/resume spamming wasn't reliable without this change.

Is there any reason why we couldn't always block at the Panda safety layer, and always proxy at the openpilot layer, rewriting in openpilot as needs dictate? Your current approach could be made to work in the usual case, but is a little more complicated than what seems necessary, and might break in off-nominal situations.

Consider the case when Panda could transition to controls-allowed, but openpilot rejects the engagement. This can happen if you hit Set while openpilot is calibrating, or a seatbelt isn't fastened, or any other noEntry event is present.

We can still apply safety logic to the transmitted messages.

@jyoung8607 jyoung8607 self-assigned this Oct 5, 2024
@csouers
Copy link
Contributor Author

csouers commented Oct 5, 2024

Is there any reason why we couldn't always block at the Panda safety layer, and always proxy at the openpilot layer

@jyoung8607 Nope! Just an attempt to retain as much of a pcm_enable policy as possible, at the expense of complexity. Subaru pre-global does a full proxy anyway and that's been merged. I'll rework this to match.

@csouers csouers marked this pull request as draft October 6, 2024 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants