-
Notifications
You must be signed in to change notification settings - Fork 19
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
Regen Pedal Remap and Hysteresis Derate #1351
base: master
Are you sure you want to change the base?
Conversation
ebcc6d4
to
3b248db
Compare
ae55042
to
3774fa5
Compare
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.
few questions
static RegenBraking_Inputs regenAttributes = { .enable_active_differential = true }; | ||
static ActiveDifferential_Inputs activeDifferentialInputs; | ||
static PowerLimiting_Inputs powerLimitingInputs = { .power_limit_kW = POWER_LIMIT_REGEN_kW }; | ||
static bool regen_enabled = true; |
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 is this a static variable? can we just have a function which determines it? I worry about stale information
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.
initialized once at the beginning and persists between calls until we change it wheelSpeedInRange
. I think you would want this to make sure you have the last condition the car state was in for hysterisis? what stale info do u mean
@@ -76,11 +80,22 @@ bool app_regen_safetyCheck(RegenBraking_Inputs *regenAttr, ActiveDifferential_In | |||
|
|||
static bool wheelSpeedInRange(ActiveDifferential_Inputs *inputs) | |||
{ | |||
// Hysterisis |
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 this hysterisis or does it just cut it off < 7kph?
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.
it'll cut off at 5kph if you were regening before and then start back up again after 7kph
// no power limit, no active differential | ||
regenAttr->left_inverter_torque_Nm = MAX_REGEN_Nm * pedal_percentage * regenAttr->derating_value; | ||
regenAttr->right_inverter_torque_Nm = MAX_REGEN_Nm * pedal_percentage * regenAttr->derating_value; | ||
return apps_pedal_percentage / (MAX_PEDAL_PERCENT - PEDAL_SCALE - 0.1f); |
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 don't think this is right, I think you need to
return apps_pedal_percentage / (MAX_PEDAL_PERCENT - PEDAL_SCALE - 0.1f); | |
return (apps_pedal_percentage-0.1f) / (MAX_PEDAL_PERCENT - PEDAL_SCALE - 0.1f); |
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.
because this assumes that the increasing part of the graph is rooted at the origin
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.
oh nice catch oops
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.
fak i should write tests for this
9f6a6e4
to
89f4b63
Compare
Changelist
Testing Done
Resolved Tickets
FIRM-156