Power calculation inconsistent between ina219 and ina3221 about ina HOT 7 CLOSED

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

from ina.

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.

from ina.

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.

from ina.

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

from ina.

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!

from ina.

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

from ina.

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.

from ina.