Skip to content
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

Fix: ECU-BMS FLT GPIO Checking for Array Faulting #153

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

mjohal67
Copy link
Contributor

@mjohal67 mjohal67 commented Aug 1, 2024

Pull Request Description

When arrays were plugged into the battery, at high irradiances (i.e. currents), the pack would fault, for no apparent reason (no fault statuses lit up on telemetry). The only place where a fault could get triggered without it showing up in the fault code is with noise being induced on one of the fault GPIOs from the masterboard to the ECU.

Prior to this change, there were three GPIOs that the ECU would read and, if set, would transition to a fault state on. FLT, COMM, and OT.

Firmware Changes:

  • ECU no longer cares about COMM and OT GPIOs. These are encapsulated within the FLT GPIO from the masterboard's perspective, so they're redundant information.
  • Scrutineer Ryan suggested doing a "checksum" using a spare GPIO and the FLT GPIO. I used the BAL GPIO from the masterboard, and set it high at the beginning of the BMS's firmware.

When the BMS starts up, it'll set BAL high, indicating no fault, and will set FLT high (indicating fault). As normal, the BMS will check the pack once and set the fault pin accordingly, signalling to the ECU to continue with its startup sequence (or to go to a fault state).

Note: During startup, the ECU doesn't care about the BAL pin (no need, since arrays won't be opened until later in startup). We also never saw issues with the arrays causing the battery to fault on startup.

During normal monitoring, if the BMS faults, it'll set FLT high and BAL low - if the ECU reads that combination of GPIOs, only then will it fault. So, if either FLT goes high, or BAL goes low individually, the ECU won't fault (hence, checksum).

This fix was implemented prior to morning charging of day 3 of competition, and was tested in front of scrutineers by pulling out the slave board connection to the masterboard, inducing communications fault. For the entirety of day 3 of race, we saw no faults (the sunniest day of FSGP 2024) - additionally, there were no faults during ASC. I believe it's safe to say this fixed the issue.

Monday Link

Effected Components

  • AMB
  • BMS
  • DID
  • ECU
  • MCB
  • MDI
  • TEL
  • CI/CD

Testing

  • Vehicle testing
  • Bench top testing
  • Unit testing
  • Other
  • N/A

Sanity check

  • CAN ID table updated
  • IOC updated and commited
  • gitignore updated and commited
  • Steps confirmed

Sources

Copy link
Contributor

@ishanjoshi23 ishanjoshi23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks ok and agrees with what you wrote in the PR description. Just wanted to double check with you @mjohal67 that the commented out line is indented to be commented out. Has this been documented in Monday as well?

CONT_FAN_PWM_set(FAN_FULL);
}
else // no fault; balance modules, drive control signals and fans
{
BAL_updateBalancing(pack); // write bal settings, send bal commands
CONT_BAL_switch(pack->status.bits.balancing_active);
// CONT_BAL_switch(pack->status.bits.balancing_active);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to make sure here that the intention is to comment this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

@AarjavJain101 AarjavJain101 merged commit 96d53a8 into master Aug 8, 2024
3 of 4 checks passed
@AarjavJain101 AarjavJain101 deleted the user/mjohal67/comp_array_bms_faults branch August 8, 2024 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants