-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/steering system #34
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: main
Are you sure you want to change the base?
Conversation
| uint32_t min_steering_2; // (optional) for redundant steering sensor | ||
| uint32_t max_steering_2; | ||
|
|
||
| uint32_t min_sensor_steering_1; // Min value that the sensor can output (if ADC reads less than this, sensor is likely unplugged) |
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.
Did you double-check this behavior of the steering sensor when you unplugged it?
| steering_params = params; | ||
| } | ||
|
|
||
| const SteeringSystemData_s &get_steering_system_data() { |
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.
Nice! we should probably do this in more places throughout VCF
| // min_observed_steering_2 = std::min(min_observed_steering_2, raw_value_2); | ||
| // max_observed_steering_2 = std::max(max_observed_steering_2, raw_value_2); | ||
| } | ||
| uint32_t min_observed_steering_1 = 4096; |
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.
Where did you get these values from? Might wanna #define them
lib/systems/src/SteeringSystem.cpp
Outdated
| _last_update_micros = curr_micros; | ||
|
|
||
| SteeringSystemData_s out = {}; | ||
| out.steering_is_implausible = _evaluate_steering_implausibilities(static_cast<int>(steering_data.analog_steering_degrees), static_cast<int>(steering_params.min_sensor_steering_1), static_cast<int>(steering_params.max_sensor_steering_1), static_cast<int>(steering_params.min_steering_1), static_cast<int>(steering_params.max_steering_1), steering_params.implausibility_margin, dt); |
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.
doesn't the _evaluate_steering_implausibilities method have access to the steering_params struct instance? They shouldn't need to be passed in as method arguments.
| } | ||
|
|
||
| bool SteeringSystem::_evaluate_steering_dtheta(int steering_analog, float dt) | ||
| { |
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.
Maybe add a comment explaining this.
| return steering_less_than_min || steering_greater_than_max; | ||
| } | ||
|
|
||
| bool SteeringSystem::_evaluate_steering_oor(int steering_analog, int min_sensor_value, int max_sensor_value) |
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.
udpate this method signature to use params?
| return steering_oor || steering_min_max_implaus || steering_dtheta; | ||
| } | ||
|
|
||
| bool SteeringSystem::_evaluate_min_max_steering_implausibilities(int steering_analog, int min_steering_value, int max_steering_value, float implausibility_margin) |
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.
update signature to use params
|
|
||
| HT_TASK::TaskResponse update_steering_calibration_task(const unsigned long& sysMicros, const HT_TASK::TaskInfo& taskInfo) { | ||
| // Update observed steering limits (ONLY USED FOR RECALIBRATION) | ||
| SteeringSystemInstance::instance().update_observed_steering_limits(VCFData_sInstance::instance().interface_data.steering_data); |
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.
is this something we need to call if we're not in the recal state?
No description provided.