Adafruit_INA219 icon indicating copy to clipboard operation
Adafruit_INA219 copied to clipboard

On quickly changing voltages and currents, the values are uncorrelated to each other

Open Ve2mrx opened this issue 5 years ago • 7 comments

  • Arduino board: Feather M0 RFM69
  • Arduino IDE version: 1.8.5
  • Library version: 1.0.2

Issue: Each measurement is made separately. In fast changing voltage and current situations, the reading of voltage is not synchronized with it's current measurement. Power calculation is difficult because of this.

Suggestion: Allow a triggered measurement instead of continous. The user code would Trigger, Wait for the measurement, and Read the synchronized values.

The changes involved:

    • Modifying the setCalibration_* to pass a parameter OR
    • better, create a setMode command and remove the mode from setCalibration.
    • Create a command to trigger a measurement.
    • The user would implement the delay after triggering, (maybe return this delay with the calibration?)

Thanks! Martin Boissonneault

Ve2mrx avatar Aug 25 '18 20:08 Ve2mrx

hmm can you try getPower_mW()?

ladyada avatar Aug 25 '18 21:08 ladyada

@ladyada : I manually copied the latest library since the property update has not been picked up by the IDE yet. I updated my Sketches accordingly to use the getPower_mW function.

My remaining issue is that every value read is uncorrelated to its siblings in time. When I read voltage and current, they are read at different times.

Is the INA219 capable of taking one measurement then let you read the voltage and current registers without causing another measurement? I could not find in the Datasheet how to trigger a measurement, I'm guessing reading triggers a new set of measurements? If that is so, then then I think it's impossible to do.

The following code makes me think that reading a register triggers (or would trigger) a measurement for every register read. You have to start a read by writing the register pointer:

void Adafruit_INA219::wireReadRegister(uint8_t reg, uint16_t *value)
{

  _i2c->beginTransmission(ina219_i2caddr);
  #if ARDUINO >= 100
    _i2c->write(reg);                       // Register
  #else
    _i2c->send(reg);                        // Register
  #endif
  _i2c->endTransmission();
  
  delay(1); // Max 12-bit conversion time is 586us per sample

  _i2c->requestFrom(ina219_i2caddr, (uint8_t)2);  
  #if ARDUINO >= 100
    // Shift values to create properly formed integer
    *value = ((_i2c->read() << 8) | _i2c->read());
  #else
    // Shift values to create properly formed integer
    *value = ((_i2c->receive() << 8) | _i2c->receive());
  #endif
}

It would still be useful to have a triggered mode for power savings and synchronization to external events.

Thanks! Martin

Ve2mrx avatar Aug 26 '18 00:08 Ve2mrx

I just found a nugget of valuable information in the INA219 datasheet (SBOS448G –AUGUST 2008–REVISED DECEMBER 2015), page 9, heading 8.3.1, at the very bottom:

The Mode control in the Configuration register also permits selecting modes to convert only voltage or current, either continuously or in response to an event (triggered). (p9, 8.3.1, paragraph 2)

Writing any of the triggered convert modes into the Configuration register (even if the desired mode is already programmed into the register) triggers a single-shot conversion. (p9, 8.3.1, paragraph 5)

I guess the INA219 CAN be triggered and ALL measurements be read before triggering another one! Correlated data is possible!

Martin

Ve2mrx avatar Aug 26 '18 05:08 Ve2mrx

Hi, I see similar issues with this fine library:

Every read of a register adds 1 ms delay which is only in very rare cases helpful.

I can only assume this is to have a converted value ready (as conversion time is by default 532uS). But this really makes no sense in most cases. In continuous mode it will just add 1ms of wait, but as conversion start is not synchronized with register read, it does not add any value. We would get an equally up to date value without wait. In averaging modes, waiting 1ms is completely pointless as well. The only use case where it makes sense, is to wait for the first conversion to complete just after setting a mode which completes conversion in less then 1 ms.

Writing the register addr every time not always required

If the last read from the INA219 was from the same register and there was no reset in between, there is no need to write the register address again to the INA219.

I would be willing to provide a PR which implements:

  • start() -> trigger new conversion by "rewriting" the current mode, this is faster than using getPower_raw()
  • available() -> returns true if completed conversion is available by by reading the CNVR register
  • conditionally disabling 1ms wait in readRegister by calling a different constructor, in order to be fully backward compatible in terms of timing (some existing applications may rely on the 1ms delay.
  • cache last set register addr so that we save time if when reading the same register over and over, this would be only active if the 1ms delay is deactivated.

It will need 3 more bytes of data (for remembering the conversion mode and the last register) and a few bytes of code. Time-wise the impact in the backward compatible mode should be only very, very few cycles ( if(doWait) { ... } ).

@ladyada: My question before doing all of this (which would include migrating my application to use the Adafruit INA219 library from working INA219 code) is: Is there a good chance that this PR (assuming it meets the quality / code style requirements ) gets integrated quickly, i.e. my effort is not wasted with a sufficiently high probability?

hogthrob avatar Dec 29 '19 12:12 hogthrob

Hi @hogthrob, What I would have done would be to trigger a conversion, and read all values in holding variables. Then, either wait for another conversion request or start one automatically depending on a flag.

When needed, the program would trigger, wait, copy the array of correlated values, or simply trigger after reading if freshness is not an issue.

If correlation is not important, the auto-trigger flag would be set to auto and the values read as needed from the array, like it is now.

Of course, it might be a new function in the library, as Adafruit has mentioned that backwards compatibility is critical. I'm only a beginner, but I believe a Pro could implement it with backward compatibility.

Even if it lives only as a pull-request and never gets pulled in, it will be useful to others. Many great features live only in pull requests. My guess is that they want to keep code size and memory requirements to a minimum for small micro-controllers.

I might be able to dig back in this next week if you need me,

Martin

Ve2mrx avatar Dec 29 '19 16:12 Ve2mrx

we'd take a clean PR that requests both at once and places the values into pointers, ideally it would take two Adafruit_Sensor objects and set one to VOLTAGE and one to CURRENT type, then they could be timestamped as well. see

https://github.com/adafruit/Adafruit_LSM6DSOX/blob/master/Adafruit_LSM6DSOX.cpp#L190 it must pass CI and not break any existing code

ladyada avatar Dec 29 '19 19:12 ladyada

I have seen this also and I think paragraph 8.3.1.1 of the datasheets states why, at least when it does power measurement itself... I think the same holds true when you are doing it also.

themindfactory avatar Mar 29 '20 01:03 themindfactory