hytech-racing / MCU

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

Current shunt measurement is broken #84

Closed AZhaoT closed 4 months ago

AZhaoT commented 6 months ago

as you can see

Screenshot 2024-05-09 at 3 30 26 PM
jhwang04 commented 6 months ago

Notes

1) The MCAP files from May 1-3 don't even have fields for pack_filtered_read and ts_out_filtered_read. But, it must be using the updated CAN library since the shunt_current_read field is called that (rather than shunt_current). Why does Foxglove only have any data at all for shunt_current_read?

2) The conversion ratio is "wrong" for shunt_current_read. The ACU should be putting the analog read (0 to 4095) on the line, which is getting translated back into some voltage (0 to 3.3). However, for this to display correctly on Foxglove, we need to include the other translations in this conversion factor & offset.

current (amps) = (((analogRead 3.3 / 4095) (4.12 + 5.1) / 5.1 ) - 3.33) / 0.005 = ((analogRead 0.00145686992746) - 3.33) / 0.005 = (analogRead 0.291373985492) - 666

Therefore, our scale factor should be 0.291373985492 and our offset should be -666.

3) I don't think we should have the "-m" in the ACU shunt measurements (see hytech.sim). I looked at the version we were using at Formula South, and that version didn't have the "-m" on the shunt measurements. I think this corresponds to whether we use big-endian or little-endian.

4) The length is wrong in the PCAN file. It should be len=6, not len=8

5) The format (3) and scaling (2) being wrong, though, wouldn't produce the behavior that we're seeing. Currently, the data isn't consistently updated. When it is updated, it changes by the slightest margins (from 20.63 to 20.42). This means that the numbers 0x6400h and 0x6300h were stored on the CAN line. If the format is fixed, then this would be 0x0064h and 0x0063h on the CAN line, which is still incorrect.

5) It's possible that the formatting being wrong is causing problem 1, since the "-m" might cause overlap in the different messages.

6) I also noticed that ACU shunt only updates at 10hz, which is not ideal for integration. ~We can jump this to 100hz by changing Metro timer_shunt(100) to Metro timer_shunt(10).~ This is fine. Our Telemetry CAN line is near-max utilization, so 10hz is sufficient.

TO-DO

jhwang04 commented 6 months ago

Code has been updated and is ready for testing! To test this code, please do the following: 1) Re-flash ACU from the ht07summerTesting branch. 2) Flash the TCU (with HT_CAN release 91 or later) 3) Check Foxglove for ACU_SHUNT_MEASUREMENTS. There should be 3 different values:

If we're still getting strange data, please also run a candump and grep for CAN ID 401h, and send a record of that in Teams. Thanks! :)