-
Notifications
You must be signed in to change notification settings - Fork 4
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
Launch control refactor (up to date with competition VCU code) #22
Conversation
This reverts commit 4ed266b.
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.
An initial look at the PR. Most of this is stylistic. I can make these changes if given the okay. I still want to do a more in depth analysis of how the algorithm works.
@@ -14,6 +14,15 @@ | |||
#define MAX_ACCEL_VAL 1000 | |||
#define MIN_ACCEL_VAL 0 | |||
|
|||
// Launch control contants |
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.
spelling error
#define LC_cGR 347 // Gear ratio times 100 | ||
#define LC_ACCEL_BEGIN 950 // 95% | ||
#define LC_ACCEL_RELEASE 50 // 5% | ||
#define LC_BRAKE_BEGIN 100 // We want a lower threshold |
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.
Do we still want this comment?
src/vcu/src/can_handles.c
Outdated
|
||
unpack_can0_DashRequestLC(msg, &unpacked_msg); | ||
|
||
if (unpacked_msg.launch_ctrl_slip_ratio != 65535) { |
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.
What is 65535? Should this really go in the Dash Request handler?
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.
-1 is the convention for "hey vcu use your default value". Should've been -1 cast to the correct type.
src/vcu/src/controls.c
Outdated
else { | ||
// Only use limits when we're not doing regen | ||
} else { // Only use limits and launch control when we're not doing regen | ||
// Even if we are doing launch control, calculate limits for loggind also so that we are not doing |
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.
spelling error "loggind"
} else { // Only use limits and launch control when we're not doing regen | ||
// Even if we are doing launch control, calculate limits for loggind also so that we are not doing | ||
// launch control when we're supposed to be limiting | ||
|
||
int32_t voltage_limited_torque = get_voltage_limited_torque(torque_command); | ||
// static uint32_t last_vt = 0; | ||
// if (HAL_GetTick() - last_vt > 10) { |
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.
Just remove this commented timer code.
position: 0 | ||
length: 1 | ||
c_type: bool | ||
launch_ctrl_slip_ratio: # Ranges from 105 to 115 |
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 does it range from 105 to 115?
@@ -22,6 +28,9 @@ void init_controls_defaults(void) { | |||
control_settings.volt_lim_min_gain = 0; | |||
control_settings.volt_lim_min_voltage = 300; | |||
control_settings.torque_temp_limited = false; | |||
|
|||
lc_settings.using_launch_ctrl = false; | |||
lc_settings.slip_ratio = 112; |
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.
Might want to bring 112 out to a DEFAULT_SLIP_RATIO constant
Closed to avoid confusion. Please move all review comments to #23 . |
No description provided.