-
Notifications
You must be signed in to change notification settings - Fork 0
Integrate isoSPI redundancy #47
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: develop
Are you sure you want to change the base?
Conversation
caiodasilva2005
left a comment
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 is very awesome Kaushik thank you for doing this. Do we have to add to the can json in Odyssey definitions for the isospi status message?
| void read_adbms_data(cell_asic chips[NUM_CHIPS], uint8_t command[2], TYPE type, | ||
| GRP group, SPI_HandleTypeDef *hspi) | ||
| { | ||
| adbms_wake_isospi(hspi); |
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.
Was this tested during your previous tests of isoSpi redundancy? Didn't know that we didn't have to wake before reading on the 6830
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.
The wake function calls were moved into the driver functions. After break recovery, the driver functions must determine the chip count on each isoSPI line and issue wake commands using the dynamic count for each line.
Core/Src/isospi_recovery.c
Outdated
| mutex_get(&state_mach->state_mutex); | ||
|
|
||
| // Sets non-critical isospi break fault | ||
| state_mach->fault_code_noncrit |= ISOSPI_BREAK_FAULT; |
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.
Maybe we should add a function to the state machine interface like set_segments_comms_fault() just to make it clear that we are mutating the fault status of the state machine form an external file
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.
That makes sense. I can add this.
Also, should I update the fault name to SEGMENT_COMMS_FAULT per the new fault table.
caiodasilva2005
left a comment
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.
Looks very good. Once building then I can approve merge
Changes
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please reach out to your Project Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
Closes #29