Power calculation inconsistent between ina219 and ina3221

[sages]

Expected Behavior

Have connected a ina219 in series with the three sections of an ina3221 and using the test code expected to see the same current, voltage and power readings for each device. Taking into account the shunt tolerance and the voltage drop across each shunt.

Actual Behavior

Screen dump below of displayed readings.

15:03:32.408 -> Nr Adr Type Bus Shunt Bus Bus 15:03:32.408 -> == === ====== ======== =========== =========== =========== 15:03:32.408 -> 1 64 INA219 4.6000V 4.9400mV 49.4080mA 227.0530mW 15:03:32.448 -> 2 66 INA3221 4.6000V 4.9200mV 49.2000mA 28.7500mW 15:03:32.448 -> 3 66 INA3221 4.6160V 4.9600mV 49.6000mA 30.6960mW 15:03:32.448 -> 4 66 INA3221 4.6320V 4.9600mV 49.6000mA 30.8020mW

Note that the power reading for the ina219 is about expected given the ~5v @ 50mA draw (ie ~250mW)

Steps to Reproduce the Problem

Explain what needs to be done in order to reproduce the problem.

1. Wire the ina3221 current shunts in series followed by the ina219 shunt.
2. Run the test code and see outputs

Specifications

• Library Version: 1.011
• IDE Version: 1.8.12
• Platform: Arduino NodeMCU ESP8266 hardware, INA219 breakout board and INA3221 breakout board
• Subsystem: Test sketch from here https://github.com/SV-Zanshin/INA/tree/master/examples/DisplayReadings

Suspect that there is an overflow in the power calc somewhere

[SV-Zanshin]

Which INA3221 breakout board are you using? I have had one which was wired up incorrectly.

[sages]

I'm using this INA3221 breakout board: https://www.aliexpress.com/item/4000341802106.html?spm=a2g0s.9042311.0.0.19fa4c4djaLTjK Prior to using it, I checked it for for the issue noted by others on earlier boards where it was preconfigured with one side of the shunt being grounded. All of the shunts appear to be floating (similar to how the ina219 is configured. The ina3221 power calc is done in code. My initial suspicion is that for the other devices the calc in int64_t INA_Class::getBusMicroWatts is doing the calc for a int_32 and not an int_64 value and is overflowing. I can try type casting later on but might not get a chance till tomorrow.

[sages]

Quick look at code in int64_t INA_Class::getBusMicroWatts(const uint8_t deviceNumber)

microWatts = (getShuntMicroVolts(deviceNumber) 1000000 / ina.microOhmR) getBusMilliVolts(deviceNumber) / 1000;

In my test setup with a shunt voltage of 4.9200mV -> 4920uV if you multiply by 1000000 you overflow a 32 bit unsigned int and the calc will fail. Compare the code to the microamp calc where you divide by the shunt resistance first. That also has potential for overflow. The non ina3221 current and power calcs are typecast to 64 bit unsigned but the ina3221 is only implied after the calc is finished and being assigned to the return value.

[sages]

Modified code below from INA.cpp

comments with changes in line below comments. comment lines start with //***

Compiled and tested in my setup.

int32_t INA_Class::getBusMicroAmps(const uint8_t deviceNumber) { /*! @brief Returns the computed microamps measured on the bus for the specified device @details The computed reading is returned and if the device is in triggered mode the next conversion is started @param[in] deviceNumber to return the value for @return int32_t signed integer for computed microamps on the bus */ readInafromEEPROM(deviceNumber); // Load EEPROM to ina structure int32_t microAmps = 0; if (ina.type == INA3221_0 || ina.type == INA3221_1 || ina.type == INA3221_2) // Doesn't compute Amps { //************************* re this code is susceptible to inaccuracy depending upon the value of the shunt resistor // ************************* if the result isn't an integer value microAmps = getShuntMicroVolts(deviceNumber) * ((int32_t)1000000 / (int32_t)ina.microOhmR); } else { microAmps = (int64_t)readWord(ina.currentRegister, ina.address) * ina.current_LSB / 1000; } // of if-then-else an INA3221 return (microAmps); } // of method getBusMicroAmps() int64_t INA_Class::getBusMicroWatts(const uint8_t deviceNumber) { /*! @brief returns the computed microwatts measured on the bus for the specified device @details The computed reading is returned and if the device is in triggered mode the next conversion is started @param[in] deviceNumber to return the value for @return int64_t signed integer for computed microwatts on the bus */ int64_t microWatts = 0; readInafromEEPROM(deviceNumber); // Load EEPROM to ina structure if (ina.type == INA3221_0 || ina.type == INA3221_1 || ina.type == INA3221_2) // Doesn't compute Amps { // ****************re fixup 2/7/20 *********************************** // *********************** added brackets around the shunt component similar to the current calc above. same // *********************** comment regarding potential errors. microWatts = (int64_t)(getShuntMicroVolts(deviceNumber) * (1000000 / ina.microOhmR)) * getBusMilliVolts(deviceNumber) / 1000; } else { microWatts = (int64_t)readWord(INA_POWER_REGISTER, ina.address) * ina.power_LSB / 1000; } // of if-then-else an INA3221 return (microWatts);

output after changes:

20:10:54.627 -> Nr Adr Type Bus Shunt Bus Bus 20:10:54.627 -> == === ====== ======== =========== =========== =========== 20:10:54.627 -> 1 64 INA219 4.5920V 4.9100mV 49.1030mA 225.2220mW 20:10:54.667 -> 2 66 INA3221 4.5920V 4.8800mV 48.8000mA 224.0890mW 20:10:54.667 -> 3 66 INA3221 4.6080V 4.9200mV 49.2000mA 226.7130mW 20:10:54.667 -> 4 66 INA3221 4.6240V 4.9600mV 49.6000mA 229.3500mW

[SV-Zanshin]

While the formatting was lost in your previous post, I'll add some explicit casts to the code computations to avoid any overflows and then push those changes. Thanks for finding the error!

[sages]

Thank you for writing the code. The auto detect is good value.

I'm struggling with the code formatting.

Specific code changes described here.

Example of your code in:

int32_t INA_Class::getBusMicroAmps(const uint8_t deviceNumber) {

if (ina.type == INA3221_0 || ina.type == INA3221_1 || ina.type == INA3221_2) // Doesn't compute Amps { *microAmps = getShuntMicroVolts(deviceNumber) ((int32_t)1000000 / (int32_t)ina.microOhmR);* } else { microAmps = (int64_t)readWord(ina.currentRegister, ina.address) ina.current_LSB / 1000; } // of if-then-else an INA3221

And my changes /replication of your code in this function. I've type cast the calculation and added brackets around the constant/shunt component of the calculation

int64_t INA_Class::getBusMicroWatts(const uint8_t deviceNumber) {

if (ina.type == INA3221_0 || ina.type == INA3221_1 || ina.type == INA3221_2) // Doesn't compute Amps { // ****re fixup 2/7/20 *** microWatts = (int64_t)(getShuntMicroVolts(deviceNumber) (1000000 / ina.microOhmR)) getBusMilliVolts(deviceNumber) / 1000; } else { microWatts = (int64_t)readWord(INA_POWER_REGISTER, ina.address) * ina.power_LSB / 1000; } // of if-then-else an INA3221

cheers Rod

[SV-Zanshin]

I didn't see your repost, but opted to go all the way and cast all the calculation components to 64-bit ones in both getBusMicroAmps() and getBusMicroWatts(). The changes are part of the "Development_v1.0.12" branch and once I have a couple of changes/fixes or about a month goes by I'll put that into a release and publish that.

Once again, good catch and good debugging.