hytech-racing / MCU

https://hytech-racing.github.io/MCU/index.html
GNU General Public License v3.0
1 stars 0 forks source link

Added Coulomb Counting code #85

Closed jhwang04 closed 3 months ago

jhwang04 commented 6 months ago

Since I can't be in shop to test this code, I wrote a few notes/concerns about this branch's intended functionality, as well as what has/hasn't been tested. Hopefully the integration process is quick :)

Code overview

This feature uses data from either the Energy Meter or from the ACU shunt measurements to estimate the remaining charge in the accumulator, both in coulombs and as a percentage. It then puts those calculated values onto the Telemetry CAN in the STATE_OF_CHARGE CAN message.

To flash this code, use the teensy41 environment. To run the unit tests for this code, use the test_interfaces environment in the platformio.ini file.

Notes

1) I have unit tested both calculate_SoC_em() and calculate_SoC_acu(). We can use one or the other by calling set_use_em_for_soc(true) (for EM) and set_use_em_for_soc(false) (for ACU). By default, we are using EM. If we want to change to shunt measurements then add ams_interface.set_use_em_for_soc(false) right after ams_interface.init() in main.cpp.

Concerns

1) ~The ACU_SHUNT_MEASUREMENTS CAN message uses three uint16s. The test cases build the CAN message using little-endian (see the generate_can_msg_from_uint_16s function in CAN_testing_utilities.h). However, I'm not sure if the ACU sends the CAN frames using big or little-endian. If the ACU_shunt_measurements data is incorrect, then we need to do two things. First, change the format in PCAN to use big-endian. Second, edit the test_calculate_SoC_acu() function to pass in false when it generates the ACU CAN msgs.~ This has been changed to use big endian, as of Issue #84

2) I wrote the test_calculate_SoC_acu() test cases based on the unit-conversion code, NOT the the other way around, so it is possible it does not function properly. In contrast to the energy meter CAN message, the ACU shunt is an analog read instead of a number in amps. The conversion from analog to amps is identical code to what the ACU was running at Formula South (except it's running on MCU now instead of on ACU). I'm mostly confident this code functions properly, but not 100%.

3) If we tick AMSInterface at 50hz, is that often enough for an integration? Is there a downside to ticking AMSInterface at 100hz? If not, we can move the ams_interface.tick() line into the t.trigger100 if-block.

4) Currently, the SoC and charge member variables are set to 0 by default. In tick(), the AMSInterface checks if bms_voltages_ has been written at least once. If it contains valid voltages and initialize_charge() has not yet been called, then tick() will call initialize_charge(). Ideally, this ensures that SoC_ will be initialized exactly once, and will only be called AFTER bms_voltages_ contains some valid reading from the CAN line. However, I do not have a good way of testing this without running it on the car, so this method of initialization may not work.

5) I couldn't come up with a good way of testing the STATE_OF_CHARGE CAN messages that the AMSInterface is sending, so those are not confirmed to be functional.

jhwang04 commented 6 months ago

Code should be ready to test! To test this code, please do the following: 1) Checkout this branch, plug in a Teensy 4.1, switch your environment to test_interfaces, and run the unit tests. If the unit tests do not all pass, please ensure that you're using HT_CAN release 91 or later. 2) Decide whether to use EM or ACU shunt measurements. By default, EM is selected. If we want to use ACU shunt instead, then see Note 1 in the original PR comment. Please use EM first, since the ACU shunt integration will only work once Issue #84 is resolved. 3) Flash code to MCU. 4) Flash TCU (HT_CAN release 91 or later) 5) Check Foxglove for STATE_OF_CHARGE. We should see two things:

7) If either EM or ACU coulomb counting is functional, then ping @siddhxrth to add SoC to dashboard :)