-
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 + Internal speed controller #31
base: launch-control
Are you sure you want to change the base?
Launch control + Internal speed controller #31
Conversation
length: 32 | ||
c_type: int32_t | ||
VCUSpeedCntrlMinInputValue: | ||
can_id: 0x231 |
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.
2 space after :
gore
|
||
// PUBLIC STRUCTS | ||
typedef struct { | ||
int32_t kp_times_1000; |
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.
Small thing but this indentation is annoying
int32_t max_output_value; | ||
int32_t min_input_value; | ||
int32_t max_input_value; | ||
int32_t 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 should be unsigned
} SpeedControllerInternalVars; | ||
|
||
// PUBLIC FUNCTIONS | ||
void init_speed_controller_defaults(int32_t max_input_speed, |
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.
Another annoying indentation inconsistency
src/vcu/src/controls.c
Outdated
|
||
int32_t speedControlTorqueOutput = get_speed_controller_torque_command(); | ||
if (speedControlTorqueOutput >= MAX_TORQUE) { | ||
speedControlTorqueOutput = MAX_TORQUE; |
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.
This isn't correct - you need to compare it with what the driver is commanding (which is torque_command
).
src/vcu/src/controls.c
Outdated
speed_command = 500; // get_launch_control_speed(front_wheel_speed); | ||
sendSpeedCmdMsg(speed_command, torque_command); | ||
|
||
set_speed_controller_setpoint(500); // RPM |
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.
This should be configurable from the dash.
src/vcu/src/speed_controller.c
Outdated
speed_controller_params.kp_times_1000 = 200; | ||
speed_controller_params.ki_times_1000 = 0; | ||
speed_controller_params.kd_times_1000 = 0; | ||
speed_controller_params.i_windup_max = 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.
This is waayyyyyyyy to small
src/vcu/src/speed_controller.c
Outdated
// Multiply by 100 for precision, example: 1234 rpm * 1000 / 12ms = 102833, 1 rpm * 1000 / 12ms = 83 | ||
speed_controller_vars.deriv_rpm_error = | ||
(speed_controller_vars.rpm_error | ||
- speed_controller_vars.last_rpm_error) * 1000 / speed_controller_params.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.
Don't multiply by 1000
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.
(Had offline conversation.)
…thub.com/MITMotorsports/MY18 into launch-control-internal-speed-controller
…:MITMotorsports/MY18 into launch-control-internal-speed-controller
This reverts commit 903e520.
White spacing appears to be hard, @sebLopezCot 😿 |
@@ -108,6 +108,10 @@ void dispatch_init() { | |||
carstats.vcu_controls_received = false; | |||
carstats.vcu_lc_controls_received = false; | |||
|
|||
carstats.rpm_setpoint.rpm_setpoint = 0; | |||
carstats.ki.ki_times_1000 = 1000; |
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 recommend you just zero this when init'ing
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.
Or loading in the default vals (we might want to start putting kp, ki as defines in a header for speed controller
(cherry picked from commit 40490cb)
This PR is used for reviewing the internal speed controller code which may be integrated with launch control. Note: this code should only be used on the jacks for it sets a constant speed and does not use the speeding up state yet.