Launch Control Palmer#23
Conversation
This reverts commit 4ed266b.
sebLopezCot
left a comment
sebLopezCot
left a comment
There was a problem hiding this comment.
Some stylistic things
| #define MAX_ACCEL_VAL 1000 | ||
| #define MIN_ACCEL_VAL 0 | ||
|
|
||
| // Launch control contants |
| #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.
Do we still want this comment?
There was a problem hiding this comment.
Sorry, this comment was to indicate that we want lower threshold than normal. I can fix it to make it clearer
|
|
||
| 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.
Just remove this commented code
There was a problem hiding this comment.
And also there is a typo here "loggind"
| @@ -90,13 +103,14 @@ void execute_controls(void) { | |||
| // } | |||
There was a problem hiding this comment.
Same here: remove commented code
| position: 0 | ||
| length: 1 | ||
| c_type: bool | ||
| launch_ctrl_slip_ratio: # Ranges from 105 to 115 |
There was a problem hiding this comment.
Why does it range between these values? Will this ever change?
There was a problem hiding this comment.
I talk about this is confluence docs. This number was something that Luis suggested but could use some more backing.
There was a problem hiding this comment.
Yeah, might be good to at least show how you might come up with this on confluence.
| control_settings.torque_temp_limited = false; | ||
|
|
||
| lc_settings.using_launch_ctrl = false; | ||
| lc_settings.slip_ratio = 112; |
There was a problem hiding this comment.
Would recommend bringing 112 out to a DEFAULT_SLIP_RATIO constant
| if (pedalbox_min(accel) < LC_ACCEL_BEGIN) { | ||
| printf("[LAUNCH CONTROL ERROR] Accel min (%d) too low\r\n", pedalbox_min(accel)); | ||
| return true; | ||
| } else if (pedalbox.brake_2 > LC_BRAKE_BEGIN) { |
There was a problem hiding this comment.
Why only brake_2? Don't we want the max of the 2 brakes?
There was a problem hiding this comment.
I have learned the reason, but would still like a comment here explaining why we only need to check brake 2
There was a problem hiding this comment.
I actually don't remember the reason myself...should definitely go in a comment
| return false; | ||
| } | ||
|
|
||
| void set_lc_state_before() { |
There was a problem hiding this comment.
Is this function ever used? Can we delete it?
| } | ||
| break; | ||
| case SPEEDING_UP: | ||
| if (torque_command < 1000) sendSpeedCmdMsg(500, torque_command); |
There was a problem hiding this comment.
In confluence, we have "LC_SPEEDING_UP_TORQUE", but here it is not a constant. Can we fix this?
There was a problem hiding this comment.
Also can we make 300 and 500 some sort of constants too and add them to the documentation in confluence?
There was a problem hiding this comment.
Also technically the if statement here with < 1000 should probably be reflected in the state machine somehow. I think you intended to back off on the torque command if we start to exceed 1000, right?
There was a problem hiding this comment.
Oh yeah I realized I had made it a constant and forgot to push it. I'll fix the others
There was a problem hiding this comment.
And yeah my intention we to command max(commanded torque, 1000)
|
Hey Dani, I think in terms of state machine logic, this looks good. Nick and I both went through the diagrams in confluence and it seems to match. Some stylistic things are in order. Also make sure to pull our changes before resolving issues. Also, nice work on the diagrams! Super easy to read. |
|
This PR has been open for too long, @daniwhite . |
3 participants
This pull request updates the VCU code to include launch control for testing at palmer on Sunday Sept. 9.