Skip to content
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

Closed
wants to merge 14 commits into from

Conversation

sebLopezCot
Copy link
Contributor

No description provided.

Copy link
Contributor Author

@sebLopezCot sebLopezCot left a 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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?


unpack_can0_DashRequestLC(msg, &unpacked_msg);

if (unpacked_msg.launch_ctrl_slip_ratio != 65535) {
Copy link
Contributor Author

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?

Copy link
Member

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.

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
Copy link
Contributor Author

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) {
Copy link
Contributor Author

@sebLopezCot sebLopezCot Sep 7, 2018

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
Copy link
Contributor Author

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?

can_spec_my18.yml Show resolved Hide resolved
can_spec_my18.yml Show resolved Hide resolved
src/dashboard/src/page_manager.c Show resolved Hide resolved
@@ -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;
Copy link
Contributor Author

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

@nistath
Copy link
Member

nistath commented Sep 8, 2018

Closed to avoid confusion. Please move all review comments to #23 .

@nistath nistath closed this Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants