adafruit / Adafruit_CircuitPython_ADS1x15

CircuitPython drivers for the ADS1x15 series of ADCs.
MIT License
140 stars 59 forks source link

When in continuous mode, first read returns previous pin value #55

Closed zacnelson closed 4 years ago

zacnelson commented 4 years ago

Using an ADS1115 on a Raspberry Pi 4 with Python3.7

Test case: p1 should return a value around 13200 p2 should return a value close to 0

ads.mode = Mode.CONTINUOUS
ads.data_rate = 860
p1 = AnalogIn(ads, ADS.P1)
p2 = AnalogIn(ads, ADS.P2)

output:

>>> p1.value
13185
>>> p1.value
13156
>>> p1.value
13186
>>> p2.value
13182    <-- incorrect
>>> p2.value
4
>>> p2.value
4
>>> p1.value
4        <-- incorrect
>>> p1.value
13194
>>> p1.value
13181
zacnelson commented 4 years ago

From my quick debugging it seems like this my be due to the fact that once the requested pin has been changed, it takes a certain amount of time for the ADS to recapture and convert the value. The library isn't waiting for this conversion time and instead returns what is in the memory address immediately.

evaherrada commented 4 years ago

@zacnelson Hey, are you still having this issue/did it ever go away? If you are still having it, I can try to take a look in a few days.

zacnelson commented 4 years ago

@zacnelson Hey, are you still having this issue/did it ever go away? If you are still having it, I can try to take a look in a few days.

It didn't go away. Based on the commit history it also doesn't seem to have been addressed since filing the bug. Thanks for looking into it!

evaherrada commented 4 years ago

@zacnelson Ok, just got the hardware. I'll try to take a look at this sometime today or tomorrow.

evaherrada commented 4 years ago

Ok, so I got it working, but I'm not entirely sure why, and I need to make the solution less janky.

So, in continuous mode, it just reads whatever was the last value calculated by the ADC, so if the last value was from p1, no matter if you're calling p1 or p2, it'll give you p1's value.

Now, I think there must be a better way to go about doing this, such as requesting the value from a specific analog port on the ADC, so I'll take a look at the datasheet to see how I'm supposed to do that.

caternuson commented 4 years ago

@dherrada FYI - the logic that is in place was done as an attempt to provide "fast" reads in a simple way without breaking the current API. There's a huge discussion here: https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15/issues/27

zacnelson commented 4 years ago

@caternuson thanks for referencing that issue. I read through your development and the end goal makes sense - reduce I2C traffic overhead and get the data as quick as possible. If I understand it correctly, when we're in continuous mode and the pin doesn't change, then we just grab the latest value from the previously set data register. If the pin does change, then we update the ADSx accordingly and then get the latest value from a new data register: https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15/blob/master/adafruit_ads1x15/ads1x15.py#L161. In this way, continuous mode is really designed to read from 1 pin over and over. If I were to read back and forth from P1 to P2 then the returned data would never really be accurate. One suggestion would be to wait the correct amount of conversion time before returning a result when the pin changes. OR return an invalid result or error until enough time has passed.

evaherrada commented 4 years ago

One suggestion would be to wait the correct amount of conversion time before returning a result

@zacnelson I believe that's essentially what single mode does.

evaherrada commented 4 years ago

@caternuson Do you think it'd be worth putting something in the README that mentions that using single mode is better for when you are reading multiple inputs?

caternuson commented 4 years ago

In this way, continuous mode is really designed to read from 1 pin over and over.

That's how I see it also. Since changing pins requires sending the updated mux setting out over I2C, it really limits the data read rate on multiple pins.

OR return an invalid result or error until enough time has passed.

The key here is how to make this determination. There's a status register that can be read, but that's more I2C traffic which slows things down. Better is to use the data ready pin somehow, but that soon becomes a discussion about interrupts.

caternuson commented 4 years ago

@dherrada Sure, that could help. There's also a "fast read" example that shows the general usage. It might even be worth a Learn Guide page to discuss the general issues with trying to read the ADS as fast as possible.

evaherrada commented 4 years ago

@caternuson Ok, I'll add a sentence or two to the readme and then add a new page to the existing learn guide

caternuson commented 4 years ago

I've recreated this issue. Which is weird, as I thought this used to work. Seems like maybe something has changed. I'll try and investigate also.

evaherrada commented 4 years ago

@caternuson So I read through the issues related to this, and from what I gathered the main issue is that there's no interrupts in circuitpython so we can't do an interrupt based approach, which is what you'd normally do to read this as fast as you can. I was also going to mention that continuous mode is designed to work with 1 pin only.

Anything else you'd like me to discuss in the learn guide page?

caternuson commented 4 years ago

@dherrada Thanks. Yep, that's pretty much the main limiter. Sounds good. And guide can always be tweaked later as needed. So wouldn't worry too much about the initial content. The whole "read the ADC as fast as possible" discussion can pull a thread and almost become it's own separate guide.

evaherrada commented 4 years ago

@caternuson This is only a limitation of the circuitpython library, correct? If so, I was going to put it in the 'Python and CircuitPython' section and not make it its own page

caternuson commented 4 years ago

@dherrada This issue is. But if this is a basic behavior of the ADS, then would potentially affect other libraries.

caternuson commented 4 years ago

@zacnelson Would you be able to test code changes if I made a pull request with a potential fix?

zacnelson commented 4 years ago

@caternuson yes, I should be able to test sometime this week.

caternuson commented 4 years ago

@dherrada @zacnelson Please check out PR #57 and test if possible.

caternuson commented 4 years ago

Hopefully fixed by #57. Thanks @zacnelson for finding this and also for testing the PR. And thanks @dherrada for the help also!