RobTillaart / INA228

Arduino library for INA228 current and power sensor
MIT License
1 stars 2 forks source link

Partial fix for the Current measurement routines. #7

Closed xkachya closed 1 week ago

xkachya commented 2 weeks ago

I was finally able to get the correct measurement of the Current. I've committed changes, which I'm sure are needed. But this is not enough to fix the measurement issue completely.

Change 1: Update for the getCurrent() image

image

Change 2: Update for the setMaxCurrentShunt() image

Change 3: In my project, I've added some routines to setup(void) to write SHUNT_CAL register calculated value

  INA.reset();

  if (!i2c_dev.begin()) {
    Serial.print("Did not find device at 0x");
    Serial.println(i2c_dev.address(), HEX);
    while (1);
  }

  Adafruit_I2CRegister shunt = Adafruit_I2CRegister(&i2c_dev, 0x02, 2, MSBFIRST);
  int16_t shunt_cal = shunt.read();
  Serial.print("#1: Shunt Calibration: " + String(shunt_cal));
  Serial.print(" - Shunt Calibration (bits): ");
  Serial.println(shunt_cal, BIN);

  float maxCurrent_A = 10;       // Max current in Amps
  float shunt_Om = 0.0002;       // Shunt resistance in Ohms

  INA.setMode(INA228_MODE_CONT_TEMP_BUS_SHUNT);
  INA.setMaxCurrentShunt(maxCurrent_A, shunt_Om);
  INA.setAverage(INA228_16_SAMPLES);

  float current_LSB = maxCurrent_A * pow(2, -19);
  float scale = 1;
  float shunt_cal_calc = 13107.2 * 1000000.0 * shunt_Om * current_LSB * scale;
  uint16_t value = shunt_cal_calc;

  Serial.print("#2: Shunt Calibration calc: " + String(value));
  Serial.print(" - Shunt Calibration (bits): ");
  Serial.println(value, BIN);
  shunt.write(value);

  shunt_cal = shunt.read();
  Serial.print("#3: Shunt Calibration: " + String(shunt_cal));
  Serial.print(" - Shunt Calibration (bits): ");
  Serial.println(shunt_cal, BIN);

Here is the TERMINAL OUTPUT:

INA228 found.
#1: Shunt Calibration: 4096 - Shunt Calibration (bits): 1000000000000
#2: Shunt Calibration calc: 50 - Shunt Calibration (bits): 110010
#3: Shunt Calibration: 50 - Shunt Calibration (bits): 110010     
#4: Temperature (DIETEMP): 2883 - DIETEMP (bits): 101101000011   
#5: Temperature: 22.523438

As a Load in my tests, I'm using a precise 126 Om resistor with 0.1% tolerance. Finally, I was able to get using the INA228 pretty close values to the INA231 results. image

RobTillaart commented 2 weeks ago

Please have a look at PR #8, , there is similar work in progress

RobTillaart commented 2 weeks ago

updated row 306 to make it possible to configure Shunt to 0.0002 Om, because in my INA228 module, there is an integrated 0.0002 Om Shunt. This change will make it possible to use the library with such devices as mine.

Then 0.0001 Ohm should be used, I think to not be affected by float precission.

xkachya commented 2 weeks ago

Please have a look at PR #8, , there is similar work in progress

I will

RobTillaart commented 2 weeks ago

Think I got the getCurrent()

The SHUNT_CAL register must be written before the CURRENT register is read.

8.1.2 _Note that the current is calculated following a shunt voltage measurement based on the value set in the SHUNT_CAL register. If the value loaded into the SHUNT_CAL register is zero, the current value reported through the CURRENT register is also zero. After programming the SHUNT_CAL register with the calculated value, the measured current in Amperes can be read from the CURRENT register. The final value is scaled by CURRENTLSB

float INA228::getCurrent()
{
  //  PAGE 31 (8.1.2)
  float shunt_cal = 13107.2e6 * _current_LSB * _shunt;
  //  depends on ADCRANGE in INA228_CONFIG register.
  if (getADCRange() == 1) 
  {
    shunt_cal *= 4;
  }
  //  shunt_cal must be written to REGISTER.
  //  work in progress PR #7
  _writeRegister(INA228_SHUNT_CAL, shunt_cal);

  //  remove reserved bits.
  uint32_t value = _readRegister(INA228_CURRENT, 3) >> 4;
  return value * _current_LSB;
}

please give it a try.

RobTillaart commented 2 weeks ago

@xkachya

I have updated a lot of code in the develop branch, this includes your work in this PR (including my remarks). Have walked through the whole code and refactored several functions to either fix or make it more maintainable rtc.

If you have time could you give it a try to see it it works.

Again thanks for your time and work to improve this library!

RobTillaart commented 2 weeks ago

@xkachya

Please note that 13107.2 * 1000000.0 = 13107.2e6 == scientific notation, Arduino floats can work with that. Don't know if you are familiar with it.

RobTillaart commented 2 weeks ago

Note: the getDiagnoseAlertBit functions still need attention, but that is for another time.

xkachya commented 2 weeks ago

Regarding SHUNT_CAL register. In my understanding, a configuration of this register should be done before the start of the measurement, during the initialization phase of the program. This is a configuration register, similar to ADC_CONFIG. We are configuring ADC_CONFIG during the initialization phase and then just reading from it. We need to configure the device once, and then retrieve the measurement results as much as we need. Also, based on my observation, I see the impact of the SHUNT_CAL register on the Power results. In my case, when I calculate and write the SHUNT_CAL during Setup() in my program, it is enough to read the correct values during the Loop() cycle. As for me, the setMaxCurrentShunt() is a better place to update the SHUNT_CAL register.

RobTillaart commented 2 weeks ago

@xkachya Added a performance sketch in the examples to get a first level indication.

RobTillaart commented 2 weeks ago

Regarding SHUNT_CAL register. ... As for me, the setMaxCurrentShunt() is a better place to update the SHUNT_CAL register.

mmm, you have a very good argument here. Will try to refactor that in today.

RobTillaart commented 2 weeks ago

I patched the files + examples. build runs.

xkachya commented 2 weeks ago

I've used develop branch to test it on my Project. I've removed my routines related to the SHUNT_CAL register update in the Project. And I've got valid results using the library only. It looks like _writeRegister() works fine now.

INA228 found.
#1: Shunt Calibration: 4096 - Shunt Calibration (bits): 1000000000000
#2: Shunt Calibration calc: 50 - Shunt Calibration (bits): 110010    
#3: Shunt Calibration: 50 - Shunt Calibration (bits): 110010  
#4: Temperature (DIETEMP): 2703 - DIETEMP (bits): 101010001111
#5: Temperature: 21.117188

Thank you for your work.

RobTillaart commented 2 weeks ago

Thank you for your work.

You're welcome,

Maybe it is possible to use your test-project as an example with the library? Only if you are willing (and allowed) to share of course,

RobTillaart commented 2 weeks ago

And I've got valid results using the library only. It looks like _writeRegister() works fine now.

Thanks for testing! That result means develop could be merged into master. (I want to check the readme against the .h file once again to see if I missed things.)

BTW, did you notice the new function getCurrentLSB()?

And final question, can you run the performance sketch, and post the results (+ board you used)?

RobTillaart commented 2 weeks ago

(I want to check the readme against the .h file once again to see if I missed things.)

done

RobTillaart commented 1 week ago

@xkachya I close this issue as the proposal are all in PR https://github.com/RobTillaart/INA228/pull/6

Today I got two INA228's and based upon the first test runs I had several minor changes in the above PR. So I'm going to merge and release 0.1.2, and then we can take it from there.