Skip to content

Commit

Permalink
Disable open wire check because we're getting module communication er…
Browse files Browse the repository at this point in the history
…rors with it enabled (#1298)

### Changelist 
<!-- Give a list of the changes covered in this PR. This will help both
you and the reviewer keep this PR within scope. -->

Does what it says on the tin.

### Testing Done
<!-- Outline the testing that was done to demonstrate the changes are
solid. This could be unit tests, integration tests, testing on the car,
etc. Include relevant code snippets, screenshots, etc as needed. -->

Copy-pasted our testing today from slack:

What we were seeing before this:
- Talked to each module directly in isolation (setting
ACCUMULATOR_NUM_SEGMENTS to 1). No issues on any segments from ISOSPI A
or ISOSPI B.
- Tested each individual cable in isolation, also no issues.
- However, when we hooked up 3 modules we'd get communication error
faults after ~10 seconds. I think once or twice we managed to get to 4
segments, but never 5
 
With this change:
- Disabled open wire check. No idea why this would break anything.
- After that, I could pretty reliably get up to 4 modules talking. 5
still work, but after rearranging the cables I found a configuration
that did work.
- Absolutely no idea whats going on here. Is there anything that would
cause the communication to be unreliable? Maybe sketchy connector,
noise, etc...

I was able to run the battery for a few minutes without any
communication faults (with open wire check disabled).

---------

Co-authored-by: will-chaba <[email protected]>
  • Loading branch information
gtaharaedmonds and will-chaba authored Jun 6, 2024
1 parent f112eb0 commit 9efe2e3
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion firmware/quadruna/BMS/src/app/states/app_allStates.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,18 @@ bool app_allStates_runOnTick100Hz(void)
iso_spi_state_counter = 0;
}

// Open wire check is mysteriously causing communication errors with the LTCs, so disabling it for now.
// TODO: Find out why!
const uint32_t cycles_to_measure =
balancing_enabled ? NUM_CYCLES_TO_MEASURE_BALANCING : NUM_CYCLES_TO_MEASURE_NOMINAL;

if (iso_spi_state_counter >= cycles_to_measure)
{
iso_spi_state_counter = 0;
iso_spi_task_state = RUN_OPEN_WIRE_CHECK;
if (balancing_enabled)
{
iso_spi_task_state = RUN_CELL_BALANCING;
}
}

iso_spi_state_counter++;
Expand Down

0 comments on commit 9efe2e3

Please sign in to comment.