Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Ve2mrx opened this issue Aug 25, 2018 · 7 comments

Comments

@Ve2mrx
Copy link

Ve2mrx commented Aug 25, 2018

  • 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

@ladyada
Copy link
Member

ladyada commented Aug 25, 2018

hmm can you try getPower_mW()?

@Ve2mrx Ve2mrx changed the title On quickly changing voltages and currents, the values are unreliable for power On quickly changing voltages and currents, the values are uncorelated to each other Aug 25, 2018
@Ve2mrx
Copy link
Author

Ve2mrx commented Aug 26, 2018

@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 Ve2mrx changed the title On quickly changing voltages and currents, the values are uncorelated to each other On quickly changing voltages and currents, the values are uncorrelated to each other Aug 26, 2018
@Ve2mrx
Copy link
Author

Ve2mrx commented Aug 26, 2018

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

@hogthrob
Copy link

hogthrob commented Dec 29, 2019

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?

@Ve2mrx
Copy link
Author

Ve2mrx commented Dec 29, 2019

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

@ladyada
Copy link
Member

ladyada commented Dec 29, 2019

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

@themindfactory
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants