VCU linelock regen implementation#243
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new RegenLinelock component to the VCU model, enabling regenerative braking control via a rear linelock valve. The implementation includes logic for estimating battery pack OCV, calculating torque limits based on available current and voltage headroom, and enforcing safety gates related to temperature, motor speed, and CAN message validity. Firmware changes include new CAN message handlers for battery pack status and switch commands. Review feedback focuses on improving safety by ensuring torque limits are always enforced, expanding parameter validation, and aggregating input validity into the component's summary fault bit.
| const float cap_nm = params->regen_linelock.absolute_regen_torque_cap_nm; | ||
| if (cap_nm > 0.0f) { | ||
| torque_nm = clamp_f(torque_nm, 0.0f, cap_nm); | ||
| } |
There was a problem hiding this comment.
The if (cap_nm > 0.0f) check is potentially dangerous. If absolute_regen_torque_cap_nm is set to 0.0f (or not initialized), the clamp is skipped, and the function could return an extremely high torque value calculated at low speeds. Removing the if ensures the torque is always clamped to the configured limit.
const float cap_nm = params->regen_linelock.absolute_regen_torque_cap_nm;
torque_nm = clamp_f(torque_nm, 0.0f, cap_nm);| params->regen_linelock.min_cell_temp_c < | ||
| params->regen_linelock.max_cell_temp_c && | ||
| params->regen_linelock.max_cell_temp_c > 0.0f; |
There was a problem hiding this comment.
The parameter validation should include a check for absolute_regen_torque_cap_nm to ensure it is a positive value, as it is used as a divisor or clamp limit in torque calculations.
params->regen_linelock.min_cell_temp_c <
params->regen_linelock.max_cell_temp_c &&
params->regen_linelock.max_cell_temp_c > 0.0f &&
params->regen_linelock.absolute_regen_torque_cap_nm > 0.0f;| out->faults.regen_linelock_motor_speed_low = motor_speed_low; | ||
| out->faults.regen_linelock_current_hard_cut = | ||
| state->current_hard_cut_latched; | ||
| out->faults.regen_linelock_any_fault = state->current_hard_cut_latched; |
There was a problem hiding this comment.
regen_linelock_any_fault should include regen_linelock_input_invalid. Loss of critical CAN inputs (battery status, inverter data) is a fault condition that should be aggregated into the component's summary fault bit to ensure the vehicle can react appropriately (e.g., by cutting torque via any_fault_exists).
| out->faults.regen_linelock_any_fault = state->current_hard_cut_latched; | |
| out->defaults.regen_linelock_any_fault = state->current_hard_cut_latched || | |
| !input_valid; |
Added VCU functionality for line lock and regen control