IO LTC6813 Restructure to move Cell Voltages Array(#1029)

moved the cell voltage structure to Io_LTC6813CellVoltages.c, tested on
the Car and it works.

### Summary
<!-- Quick summary of changes, optional -->
BMS LTC6813 IO Driver Refactor done and tested on car

### Changelist 
<!-- Give a list of the changes covered in this PR. This will help both
you and the reviewer keep this PR within scope. -->
added function in the IO Driver for LTC6813
modified Accumulator Struct with new function to get Voltage
modified BMS tests to work with the new function structure
modified CAN Message names to work with MacCAN

### 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. -->


### Resolved Issues
<!-- Link any issues that this PR resolved like so: `Resolves #1, #2,
and #5` (Note: Using this format, Github will automatically close the
issue(s) when this PR is merged in). -->

### Checklist
*Please change `[ ]` to `[x]` when you are ready.*
- [x] I have read and followed the code conventions detailed in
[README.md](../README.md) (*This will save time for both you and the
reviewer!*).
- [ ] If this pull request is longer then **500** lines, I have provided
*explicit* justification in the summary above explaining why I *cannot*
break this up into multiple pull requests (*Small PR's are faster and
less painful for everyone involved!*).

README.md

@@ -119,6 +119,12 @@ candump can0 -c -t d | cantools decode --single-line can_bus/dbcs/CanMsgs.dbc
 cantools monitor scripts/codegen/CAN/App_CanMsgs.dbc -b socketcan -c can0 -B 500000
 ```
 
+On Mac:
+
+```
+cantools monitor can_bus/dbcs/CanMsgs.dbc -b pcan -c PCAN_USBBUS1 -B 500000
+```
+
 ## Continuous Integration (CI)
 
 We run (and require) continuous integration on every pull request before it is merged. This automatically makes sure the

can_bus/json/INVL/INVL_tx.json

@@ -279,7 +279,7 @@
         "msg_id": 9,
         "cycle_time": 100,
         "signals": {
-            "1V5_ReferenceVoltage": {
+            "ReferenceVoltage_1V5": {
                 "start_bit": 0,
                 "bits": 16,
                 "signed": true,
@@ -289,7 +289,7 @@
                 "max": 327.67,
                 "unit": "V"
             },
-            "2V5_ReferenceVoltage": {
+            "ReferenceVoltage_2V5": {
                 "start_bit": 16,
                 "bits": 16,
                 "signed": true,
@@ -299,7 +299,7 @@
                 "max": 327.67,
                 "unit": "V"
             },
-            "5V_ReferenceVoltage": {
+            "ReferenceVoltage_5V": {
                 "start_bit": 32,
                 "bits": 16,
                 "signed": true,
@@ -309,7 +309,7 @@
                 "max": 327.67,
                 "unit": "V"
             },
-            "12V_ReferenceVoltage": {
+            "ReferenceVoltage_12V": {
                 "start_bit": 48,
                 "bits": 16,
                 "signed": true,

can_bus/json/INVR/INVR_tx.json

@@ -279,7 +279,7 @@
         "msg_id": 59,
         "cycle_time": 100,
         "signals": {
-            "1V5_ReferenceVoltage": {
+            "ReferenceVoltage_1V5": {
                 "start_bit": 0,
                 "bits": 16,
                 "signed": true,
@@ -289,7 +289,7 @@
                 "max": 327.67,
                 "unit": "V"
             },
-            "2V5_ReferenceVoltage": {
+            "ReferenceVoltage_2V5": {
                 "start_bit": 16,
                 "bits": 16,
                 "signed": true,
@@ -299,7 +299,7 @@
                 "max": 327.67,
                 "unit": "V"
             },
-            "5V_ReferenceVoltage": {
+            "ReferenceVoltage_5V": {
                 "start_bit": 32,
                 "bits": 16,
                 "signed": true,
@@ -309,7 +309,7 @@
                 "max": 327.67,
                 "unit": "V"
             },
-            "12V_ReferenceVoltage": {
+            "ReferenceVoltage_12V": {
                 "start_bit": 48,
                 "bits": 16,
                 "signed": true,

firmware/thruna/BMS/Inc/App/App_Accumulator.h

@@ -42,7 +42,8 @@ struct Accumulator *App_Accumulator_Create(
     bool (*config_monitoring_chip)(void),
     bool (*write_cfg_registers)(bool[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT]),
     bool (*start_voltage_conv)(void),
-    bool (*read_cell_voltages)(float[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT]),
+    bool (*read_cell_voltages)(void),
+    float (*get_cell_voltage)(uint8_t, uint8_t),
     bool (*start_cell_temp_conv)(void),
     bool (*read_cell_temperatures)(void),
     float (*get_min_cell_temp)(uint8_t *, uint8_t *),

firmware/thruna/BMS/Inc/Io/Io_LTC6813/Io_LTC6813CellVoltages.h

@@ -9,12 +9,17 @@
  * @param cell_voltages Buffer to write out the read voltages to
  * @return True if cell voltages are read back successfully. Else, false
  */
-bool Io_LTC6813CellVoltages_ReadVoltages(
-    float cell_voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT]);
+bool Io_LTC6813CellVoltages_ReadVoltages(void);
 
 /**
  * Start an ADC conversion on the LTC6813 for measured cell voltages
  * @return True if the ADC conversions have been sent out successfully. Else,
  * false
  */
 bool Io_LTC6813CellVoltages_StartAdcConversion(void);
+
+/**
+ * @brief Read cell voltages from the previously stored values
+ * @return Cell Voltage Value
+ */
+float Io_LTC6813CellVoltages_GetCellVoltage(uint8_t segement, uint8_t cell);

firmware/thruna/BMS/Src/App/App_Accumulator.c

@@ -63,12 +63,12 @@ struct Accumulator
 
     // Cell voltage monitoring functions
     bool (*start_cell_voltage_conv)(void);
-    bool (*read_cell_voltages)(float[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT]);
+    bool (*read_cell_voltages)(void);
+    float (*get_cell_voltage)(uint8_t, uint8_t);
 
     // Voltage information
     VoltageStats            voltage_stats;
     AccumulatorMonitorState state;
-    float                   cell_voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT];
 
     // Balancing information
     bool     balance_enabled;
@@ -157,7 +157,7 @@ static void App_Accumulator_CalculateCellsToBalance(struct Accumulator *accumula
     {
         for (uint8_t cell = 0U; cell < ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT; cell++)
         {
-            const bool needs_discharging                 = (accumulator->cell_voltages[segment][cell] > target_voltage);
+            const bool needs_discharging = (accumulator->get_cell_voltage(segment, cell) > target_voltage);
             accumulator->cells_to_balance[segment][cell] = needs_discharging;
         }
     }
@@ -224,7 +224,8 @@ struct Accumulator *App_Accumulator_Create(
     bool (*config_monitoring_chip)(void),
     bool (*write_cfg_registers)(bool[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT]),
     bool (*start_voltage_conv)(void),
-    bool (*read_cell_voltages)(float[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT]),
+    bool (*read_cell_voltages)(void),
+    float (*get_cell_voltage)(uint8_t, uint8_t),
     bool (*start_cell_temp_conv)(void),
     bool (*read_cell_temperatures)(void),
     float (*get_min_cell_temp)(uint8_t *, uint8_t *),
@@ -247,6 +248,7 @@ struct Accumulator *App_Accumulator_Create(
     // Cell voltage monitoring functions
     accumulator->num_comm_tries          = 0U;
     accumulator->read_cell_voltages      = read_cell_voltages;
+    accumulator->get_cell_voltage        = get_cell_voltage;
     accumulator->start_cell_voltage_conv = start_voltage_conv;
 
     // Voltage information
@@ -312,7 +314,7 @@ float App_Accumulator_GetCellVoltage(
         return 0.0f;
     }
 
-    return accumulator->cell_voltages[segment][cell];
+    return accumulator->get_cell_voltage(segment, cell);
 }
 
 float App_Accumulator_GetMaxVoltage(const struct Accumulator *const accumulator, uint8_t *segment, uint8_t *cell)
@@ -397,8 +399,7 @@ void App_Accumulator_RunOnTick100Hz(struct Accumulator *const accumulator)
         case GET_CELL_VOLTAGE_STATE:
         {
             // Attempt to read voltages from the LTCs, write output to cell voltages array
-            UPDATE_PEC15_ERROR_COUNT(
-                accumulator->read_cell_voltages(accumulator->cell_voltages), accumulator->num_comm_tries);
+            UPDATE_PEC15_ERROR_COUNT(accumulator->read_cell_voltages(), accumulator->num_comm_tries);
 
             // Calculate min/max/segment voltages
             App_Accumulator_CalculateVoltageStats(accumulator);

firmware/thruna/BMS/Src/Io/Io_LTC6813/Io_LTC6813CellVoltages.c

@@ -32,6 +32,8 @@ enum CellVoltageRegGroup
 
 extern struct SharedSpi *ltc6813_spi;
 
+static float cell_voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT];
+
 static const uint16_t cv_read_cmds[NUM_OF_CELL_V_REG_GROUPS] = {
     [CELL_V_REG_GROUP_A] = RDCVA, [CELL_V_REG_GROUP_B] = RDCVB, [CELL_V_REG_GROUP_C] = RDCVC,
     [CELL_V_REG_GROUP_D] = RDCVD, [CELL_V_REG_GROUP_E] = RDCVE, [CELL_V_REG_GROUP_F] = RDCVF,
@@ -46,15 +48,9 @@ static uint16_t discharge_bits[ACCUMULATOR_NUM_SEGMENTS] = { 0U };
  * @param cell_voltages Buffer to write out the read voltages to
  * @return True if the data is read back successfully. Else, false
  */
-static bool Io_ParseCellVoltageFromAllSegments(
-    uint8_t  curr_reg_group,
-    uint16_t rx_buffer[NUM_REG_GROUP_RX_WORDS],
-    float    cell_voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT]);
-
-static bool Io_ParseCellVoltageFromAllSegments(
-    uint8_t  curr_reg_group,
-    uint16_t rx_buffer[NUM_REG_GROUP_RX_WORDS],
-    float    cell_voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT])
+static bool Io_ParseCellVoltageFromAllSegments(uint8_t curr_reg_group, uint16_t rx_buffer[NUM_REG_GROUP_RX_WORDS]);
+
+static bool Io_ParseCellVoltageFromAllSegments(uint8_t curr_reg_group, uint16_t rx_buffer[NUM_REG_GROUP_RX_WORDS])
 {
     bool status = true;
 
@@ -114,8 +110,7 @@ static bool Io_ParseCellVoltageFromAllSegments(
     return status;
 }
 
-bool Io_LTC6813CellVoltages_ReadVoltages(
-    float cell_voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT])
+bool Io_LTC6813CellVoltages_ReadVoltages(void)
 {
     // Exit early if ADC conversion fails
     if (!Io_LTC6813Shared_PollAdcConversions())
@@ -138,7 +133,7 @@ bool Io_LTC6813CellVoltages_ReadVoltages(
         // Transmit the command and receive data stored in register group.
         bool voltage_read_success = Io_SharedSpi_TransmitAndReceive(
             ltc6813_spi, (uint8_t *)tx_cmd, TOTAL_NUM_CMD_BYTES, (uint8_t *)rx_buffer, NUM_REG_GROUP_RX_BYTES);
-        voltage_read_success &= Io_ParseCellVoltageFromAllSegments(curr_reg_group, rx_buffer, cell_voltages);
+        voltage_read_success &= Io_ParseCellVoltageFromAllSegments(curr_reg_group, rx_buffer);
 
         // If SPI communication or parsing fails, save result but continue to update data for remaining cell register
         // groups
@@ -157,3 +152,8 @@ bool Io_LTC6813CellVoltages_StartAdcConversion(void)
 {
     return Io_LTC6813Shared_SendCommand(ADCV);
 }
+
+float Io_LTC6813CellVoltages_GetCellVoltage(uint8_t segement, uint8_t cell)
+{
+    return cell_voltages[segement][cell];
+}

firmware/thruna/BMS/Src/main.c

@@ -298,11 +298,12 @@ int main(void)
     accumulator = App_Accumulator_Create(
         Io_LTC6813Shared_SetCfgRegsToDefaultSettings, Io_LTC6813Shared_WriteConfigurationRegisters,
         Io_LTC6813CellVoltages_StartAdcConversion, Io_LTC6813CellVoltages_ReadVoltages,
-        Io_LTC6813CellTemperatures_StartAdcConversion, Io_LTC6813CellTemperatures_ReadTemperatures,
-        Io_LTC6813CellTemperatures_GetMinTempDegC, Io_LTC6813CellTemperatures_GetMaxTempDegC,
-        Io_LTC6813CellTemperatures_GetAverageTempDegC, Io_LTC6813Shared_EnableBalance, Io_LTC6813Shared_DisableBalance,
-        Io_LatchedFaults_CheckImdLatchedFault, Io_LatchedFaults_CheckBspdLatchedFault,
-        Io_LatchedFaults_CheckBmsLatchedFault, Io_ThermistorReadings_MuxSelect, Io_ThermistorReadings_ReadSelectedTemp);
+        Io_LTC6813CellVoltages_GetCellVoltage, Io_LTC6813CellTemperatures_StartAdcConversion,
+        Io_LTC6813CellTemperatures_ReadTemperatures, Io_LTC6813CellTemperatures_GetMinTempDegC,
+        Io_LTC6813CellTemperatures_GetMaxTempDegC, Io_LTC6813CellTemperatures_GetAverageTempDegC,
+        Io_LTC6813Shared_EnableBalance, Io_LTC6813Shared_DisableBalance, Io_LatchedFaults_CheckImdLatchedFault,
+        Io_LatchedFaults_CheckBspdLatchedFault, Io_LatchedFaults_CheckBmsLatchedFault, Io_ThermistorReadings_MuxSelect,
+        Io_ThermistorReadings_ReadSelectedTemp);
 
     ts = App_TractiveSystem_Create(
         Io_VoltageSense_GetTractiveSystemVoltage, Io_CurrentSense_GetHighResolutionMainCurrent,

firmware/thruna/BMS/Test/Src/Test_Faults.cpp

@@ -38,6 +38,7 @@ FAKE_VALUE_FUNC(float, get_low_res_current);
 FAKE_VALUE_FUNC(float, get_high_res_current);
 FAKE_VALUE_FUNC(bool, start_temp_conv);
 FAKE_VALUE_FUNC(bool, read_cell_temperatures);
+FAKE_VALUE_FUNC(float, get_cell_voltage, uint8_t, uint8_t);
 FAKE_VOID_FUNC(thermistor_mux_select, uint8_t);
 FAKE_VALUE_FUNC(float, read_thermistor_temp);
 FAKE_VALUE_FUNC(float, get_min_temp_degc, uint8_t *, uint8_t *);
@@ -60,40 +61,11 @@ static bool
     return true;
 }
 
-static float cell_voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT];
-
-static bool read_cell_voltages(float voltages[ACCUMULATOR_NUM_SEGMENTS][ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT])
+static bool read_cell_voltages(void)
 {
-    for (uint8_t segment = 0; segment < ACCUMULATOR_NUM_SEGMENTS; segment++)
-    {
-        for (uint8_t cell = 0; cell < ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT; cell++)
-        {
-            voltages[segment][cell] = cell_voltages[segment][cell];
-        }
-    }
-
     return true;
 }
 
-static void set_cell_voltage(AccumulatorSegment segment, uint8_t cell, float voltage)
-{
-    if (segment < ACCUMULATOR_NUM_SEGMENTS && cell < ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT)
-    {
-        cell_voltages[segment][cell] = voltage;
-    }
-}
-
-static void set_all_cell_voltages(float voltage)
-{
-    for (uint8_t segment = 0; segment < ACCUMULATOR_NUM_SEGMENTS; segment++)
-    {
-        for (uint8_t cell = 0; cell < ACCUMULATOR_NUM_SERIES_CELLS_PER_SEGMENT; cell++)
-        {
-            set_cell_voltage((AccumulatorSegment)segment, cell, voltage);
-        }
-    }
-}
-
 class BmsFaultTest : public BaseStateMachineTest
 {
   protected:
@@ -121,9 +93,9 @@ class BmsFaultTest : public BaseStateMachineTest
         bspd_ok = App_OkStatus_Create(enable_bspd_ok, disable_bspd_ok, is_bspd_ok_enabled);
 
         accumulator = App_Accumulator_Create(
-            configure_cell_monitors, write_cfg_registers, start_voltage_conv, read_cell_voltages, start_temp_conv,
-            read_cell_temperatures, get_min_temp_degc, get_max_temp_degc, get_avg_temp_degc, enable_balance,
-            disable_balance, check_imd_latched_fault, check_bspd_latched_fault, check_bms_latched_fault,
+            configure_cell_monitors, write_cfg_registers, start_voltage_conv, read_cell_voltages, get_cell_voltage,
+            start_temp_conv, read_cell_temperatures, get_min_temp_degc, get_max_temp_degc, get_avg_temp_degc,
+            enable_balance, disable_balance, check_imd_latched_fault, check_bspd_latched_fault, check_bms_latched_fault,
             thermistor_mux_select, read_thermistor_temp);
 
         precharge_relay = App_PrechargeRelay_Create(enable_pre_charge, disable_pre_charge);
@@ -173,9 +145,10 @@ class BmsFaultTest : public BaseStateMachineTest
         RESET_FAKE(read_page);
         RESET_FAKE(write_page);
         RESET_FAKE(page_erase);
+        RESET_FAKE(get_cell_voltage);
 
         // Set initial voltages to nominal value
-        set_all_cell_voltages(3.8);
+        get_cell_voltage_fake.return_val   = 3.8;
         start_voltage_conv_fake.return_val = true;
 
         // A temperature in [0.0, 60.0] degC to prevent other tests from entering the fault state
@@ -266,7 +239,7 @@ TEST_F(BmsFaultTest, check_state_transition_to_fault_state_from_all_states_overv
             ASSERT_FALSE(App_CanAlerts_BMS_Fault_CellOvervoltage_Get());
 
             // Set cell voltage critically high and confirm fault is set
-            set_cell_voltage((AccumulatorSegment)segment, cell, MAX_CELL_VOLTAGE + 0.1f);
+            get_cell_voltage_fake.return_val = MAX_CELL_VOLTAGE + 0.1f;
             LetTimePass(state_machine, 20);
             ASSERT_EQ(App_GetFaultState(), App_SharedStateMachine_GetCurrentState(state_machine));
             ASSERT_TRUE(App_CanAlerts_BMS_Fault_CellOvervoltage_Get());
@@ -276,7 +249,7 @@ TEST_F(BmsFaultTest, check_state_transition_to_fault_state_from_all_states_overv
             ASSERT_TRUE(App_CanAlerts_BMS_Fault_CellOvervoltage_Get());
 
             // Clear fault, should transition back to init
-            set_cell_voltage((AccumulatorSegment)segment, cell, MAX_CELL_VOLTAGE - 0.1f);
+            get_cell_voltage_fake.return_val = MAX_CELL_VOLTAGE - 0.1f;
             LetTimePass(state_machine, 20);
             ASSERT_EQ(App_GetInitState(), App_SharedStateMachine_GetCurrentState(state_machine));
             ASSERT_FALSE(App_CanAlerts_BMS_Fault_CellOvervoltage_Get());
@@ -301,7 +274,7 @@ TEST_F(BmsFaultTest, check_state_transition_to_fault_state_from_all_states_under
             ASSERT_FALSE(App_CanAlerts_BMS_Fault_CellUndervoltage_Get());
 
             // Set cell voltage critically low and confirm fault is set
-            set_cell_voltage((AccumulatorSegment)segment, cell, MIN_CELL_VOLTAGE - 0.1f);
+            get_cell_voltage_fake.return_val = MIN_CELL_VOLTAGE - 0.1f;
             LetTimePass(state_machine, 20);
             ASSERT_EQ(App_GetFaultState(), App_SharedStateMachine_GetCurrentState(state_machine));
             ASSERT_TRUE(App_CanAlerts_BMS_Fault_CellUndervoltage_Get());
@@ -311,7 +284,7 @@ TEST_F(BmsFaultTest, check_state_transition_to_fault_state_from_all_states_under
             ASSERT_TRUE(App_CanAlerts_BMS_Fault_CellUndervoltage_Get());
 
             // Clear fault, should transition back to init
-            set_cell_voltage((AccumulatorSegment)segment, cell, MIN_CELL_VOLTAGE + 0.1f);
+            get_cell_voltage_fake.return_val = MIN_CELL_VOLTAGE + 0.1f;
             LetTimePass(state_machine, 20);
             ASSERT_EQ(App_GetInitState(), App_SharedStateMachine_GetCurrentState(state_machine));
             ASSERT_FALSE(App_CanAlerts_BMS_Fault_CellUndervoltage_Get());
@@ -638,7 +611,7 @@ TEST_F(BmsFaultTest, check_state_transition_to_fault_disables_bms_ok)
     ASSERT_EQ(disable_bms_ok_fake.call_count, 0);
 
     // Set cell voltage critically high and confirm fault is set
-    set_cell_voltage((AccumulatorSegment)0U, 0U, MAX_CELL_VOLTAGE + 0.1f);
+    get_cell_voltage_fake.return_val = MAX_CELL_VOLTAGE + 0.1f;
     LetTimePass(state_machine, 20);
     ASSERT_EQ(App_GetFaultState(), App_SharedStateMachine_GetCurrentState(state_machine));