-
Notifications
You must be signed in to change notification settings - Fork 1.7k
refactor: button event handling with ButtonEventConfig and ButtonMap #2733
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
Conversation
5f4e266 to
3085b55
Compare
| BUTTON_MAPS: list[ButtonMap] = ( | ||
| ButtonMap(ButtonType.accelCruise, CruiseButtons.RES_ACCEL), | ||
| ButtonMap(ButtonType.decelCruise, CruiseButtons.DECEL_SET), | ||
| ButtonMap(ButtonType.mainCruise, CruiseButtons.MAIN), | ||
| ButtonMap(ButtonType.cancel, CruiseButtons.CANCEL) | ||
| ) | ||
| SETTINGS_BUTTON_MAPS: list[ButtonMap] = ( | ||
| ButtonMap(ButtonType.gapAdjustCruise, CruiseSettings.DISTANCE), | ||
| ButtonMap(ButtonType.lkas, CruiseSettings.LKAS) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not a dict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to a list so I could index into it without converting to a list because I had an idea but then I changed the create_button_events code so it was not necessary. I don't see why it couldn't be a dict now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, a dict would enforce that you can only have 1 button event for a button value which may be restrictive? So for example you could not have it so that pressing the Resume/cancel button creates 2 events, cancel and resume. Do you agree?
|
This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity. |
|
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
This looks like a better button event interface. Should be able to simplify gm and toyota logic further.