-
Notifications
You must be signed in to change notification settings - Fork 9
Restructured VCU FSM to separate individual cases in EM_Fault(). #91
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
base: main
Are you sure you want to change the base?
Conversation
vcu/Src/drive_by_wire.c
Outdated
| sendCAN_VCU_Throttle_Failure(); | ||
| int currentState = fsmGetState(&VCUFsmHandle); | ||
| DEBUG_PRINT("Throttle read failure, trans to fatal failure\n"); | ||
| sendDTC_FATAL_VCU_F7_EV_FATAL(); |
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.
Let's send DTC 30 before invoking VCU FATAL DTC. Without it, the fatal DTC doesn't tell us anything about the root cause.
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.
They are called before you call that function pretty sure.
common/Data/2024CAR.dbc
Outdated
| SG_ ChannelCurrentMCRight : 0|16@1+ (0.01,0) [0|0] "Amps" Vector__XXX | ||
| SG_ ChannelCurrentAUX : 0|16@1+ (0.01,0) [0|0] "" Vector__XXX | ||
|
|
||
| BO_ 2287668226 VCU_Toggle_EM_Off: 8 VCU_F7 |
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.
You can put this all in one message and have distinct signals
vcu/Src/drive_by_wire.c
Outdated
| { STATE_EM_Enable, EV_EM_Toggle, &ToggleMotorsOff }, | ||
| { STATE_Failure_Fatal, EV_ANY, &fatalTransition }, | ||
| { STATE_ANY, EV_CAN_Receive_HV, &processHvState}, // From DCU. Check HV toggle response from the BMU | ||
| // { STATE_ANY, EV_CAN_Receive_EM, &processEmState}, // From DCU. Happens locally now so remove it |
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.
Remove any events nots used, especially stuff commented out
vcu/Src/drive_by_wire.c
Outdated
| { STATE_ANY, EV_BTN_HV_Toggle, &sendHvToggle}, // From DCU | ||
| { STATE_ANY, EV_BTN_EM_Toggle, &sendEmToggle}, // From DCU | ||
| { STATE_EM_Enable, EV_BTN_TC_Toggle, &toggleTC}, // From DCU | ||
| // { STATE_EM_Enable, EV_BTN_Endurance_Mode_Toggle, &toggleEnduranceMode}, // From DCU |
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.
Endurance mode not in use, remove for now as we don't even have a working endurance mode
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.
Remove debounce timer, I can't comment on it since it wasn't modified, see line 156
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.
On second thought keep the transition and add a note endurance mode not currently in use, also remove everything that says "from DCU"
| { | ||
| sendDTC_WARNING_Throttle_Failure(3); | ||
| return EM_Fault(EV_Throttle_Failure); | ||
| return Throttle_Shutdown(EV_Throttle_Failure); |
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.
Remove comments on lines 216 and 217
| VCU_States_t state = shutDownMotors(STATE_HV_Disable); | ||
| return state; | ||
| } | ||
| uint32_t Generic_EM_Fault(uint32_t event){ |
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.
change all function return types to be VCU_States_t instead as necessary
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.
its supposed to be uint32_t but I think I messed up by declaring it as VCU_States_t in the function
common/Data/2024CAR.dbc
Outdated
| BO_ 2287668226 VCU_Toggle_EM_Off: 8 VCU_F7 | ||
| SG_ EMFaultEvent : 0|8@1+ (1,0) [0|255] "" Vector__XXX | ||
|
|
||
| BO_ 2287733762 VCU_HV_Disable_Twice: 8 VCU_F7 |
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.
What is Disable twice?
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.
Its for disableRedundancy, tbh idk why I didn't keep it the same name
vcu/Src/drive_by_wire.c
Outdated
| uint32_t EM_Fault(uint32_t event) | ||
| { | ||
| int newState = STATE_Failure_Fatal; | ||
| uint32_t BPS_Failure_Shutdown(uint32_t event){ |
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.
BPS is brake pressure sensor. you have redundant transitions
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 was just copying the transitions from how it was initially lol
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.
My suggestion is when working on something, don't assume what was previously done is perfect. Investigate what it is you're working on and find ways to improve it outside of what was just asked of you. It's some good feedback one of my previous employers gave me to improve as an engineer. For example in this case the VCU state machine isn't perfect and could use some optimization
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.
Yeah I found out that BPS_Failure wasn't implemented out so I removed it
vcu/Src/drive_by_wire.c
Outdated
| { STATE_EM_Enable, EV_EM_Toggle, &EM_Fault }, | ||
| { STATE_HV_Enable, EV_Hv_Disable, & DisablingHV}, | ||
| { STATE_EM_Enable, EV_Bps_Fail, &BPS_Failure_Shutdown }, | ||
| { STATE_EM_Enable, EV_Hv_Disable, & DisablingHV}, |
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.
remove extra space between & and DisablingHV
vcu/Src/drive_by_wire.c
Outdated
| sendCAN_VCU_Toggle_EM_Off(); | ||
| DEBUG_PRINT("EM Toggle, trans to EM Disabled\n"); | ||
| //disable TC | ||
| disableRegen(); |
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 would move disableRegen and disable_TC to MotorStop
No description provided.