-
Notifications
You must be signed in to change notification settings - Fork 1.4k
safety: split longitudinal and lateral checks #2297
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
base: master
Are you sure you want to change the base?
Conversation
@adeebshihadeh did you have any specific ideas about this or want me to explore it further? |
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.
LGTM aside from comments
#include "opendbc/safety/safety_helpers.h" | ||
#include "opendbc/safety/safety_lateral.h" | ||
#include "opendbc/safety/safety_longitudinal.h" |
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.
no need for the safety_
prefix - the folder is already called safety!
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.
helpers doesn't fully capture what's in this file. interpolate
is a helper, generic_rx_checks
implements safety
steering_disengage_prev = steering_disengage; | ||
} | ||
|
||
static void stock_ecu_check(bool stock_ecu_detected) { |
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'd just put these non-helper functions back in safety.h for now
Everything else stays in safety.h
Need to move common functions like interpolate either into safety_declarations.h or a new file (I added safety_helpers.h for now), might as well move everything? If so, then can main.c and safety.h be the same thing?