dfrph issues

[jontrulson]

• using a different formula than original to compute result. Needs to
  be tested on real hardware.
• It also appears to be averaging over the computed pH value, rather
  than the computed voltage it should be averaging over.

[pylbert]

Original C++ code appears to assumed 5v which is a very common theme for much of the sensor code I've looked over. I'm not 100% sure this is a safe assumption. The dropout on the sensor seems to be ~80% of the op amp rails. I tested this down to ~1v where this is no longer linear. I believe this is where the magical 3.5 value comes from in the original sensor C++ src - ie, pH scale of 14/(80% of 5v) = magic number of 3.5.

This is a good example where voltage is not needed. The range of the adc (for default mraa edison) is 0 -> 1023, the dynamic range of the pH sensor is 0 -> 14, however, the op amp voltage range appears to be 0 -> .8 * Vref. My c src scales the raw counts by 1/.8 = 1.25 to account for the limited op amp range. The C++ source converts to voltage first, then pH. The max voltage which can come out of the pH sensor is .8 * Vref. @ 5v we have 3.5 pH/v. This will only return a correct value if the sensor is operated @ 5v. This is a great argument to use normalized values vs raw counts, ie 0.0 -> 1.0 since a raw ADC count is meaningless outside of the sensor src.

I could be wrong with the above guess at the magic number's origin, but I've tested on hardware for a range of rail voltages for neutral pH and it appears to hold true.

*note - in the original dfrph c++ sensor code, the aRes value uses 1 << adc #bits = (for default mraa) 1024. Since it's not possible to read a value of 1024 from a 10 bit adc, it would not be possible to achieve the max unit value from the sensor. I see the 1024 value throughout our sensor C++ code.

We should try to write our math according to how values are interpreted. An adc read can be viewed as a normalized value (disregarding drop-out or offset from the sensor hardware):

Value(units) = ADCraw/ADCmax * UNITmax

float DFRPH::volts()
{
int val = m_aio.read();
return(val * (m_aref / m_aRes));
}

in my mind should be:

float DFRPH::volts()
{
int val = m_aio.read();
return(m_aref * (val / m_aRes));
}

[pylbert]

Fixed the loop that averages pH value.

[jontrulson]

On 07/26/2016 06:55 PM, Noel Eck wrote:

Original C++ code appears to assumed 5v which is a very common theme
for much of the sensor code I've looked over. I'm not 100% sure this
is a safe assumption. The dropout on the sensor seems to be ~80% of
the op amp rails. I tested this down to ~1v where this is no longer
linear. I believe this is where the magical 3.5 value comes from in
the original sensor C++ src - ie, pH scale of 14/(80% of 5v) = magic
number of 3.5.

Many of the original drivers always assumed 5v, since Arduino Uno and
compatibles only support that voltage value.

For this particular case, the device only operates at 5vdc, so you have
no choice.

http://www.dfrobot.com/wiki/index.php/PH_meter%28SKU:_SEN0161%29

In devices where both 3.3 and 5.0 are supported, I always allowed
supplying it, though older drivers typically did not.

Now is a great time to fix these oversights.

I like the fact that you may have figured out what the magic number
means. I hate magic numbers. Occasionally I manage to figure out what
one is, but I didn't try on this device. Nice.

This is a good example where voltage is not needed. The range of the
adc (for default mraa edison) is 0 -> 1023, the dynamic range of the
pH sensor is 0 -> 14, however, the op amp voltage range appears to be
0 -> .8 * Vref. My c src scales the raw counts by 1/.8 = 1.25 to
account for the limited op amp range. The C++ source converts to
voltage first, then pH. The max voltage which can come out of the pH
sensor is .8 * Vref. @ 5v we have 3.5 pH/v. This will only return a
correct value if the sensor is operated @ 5v. This is a great argument
to use normalized values vs raw counts, ie 0.0 -> 1.0 since a raw ADC
count is meaningless outside of the sensor src.

I could be wrong with the above guess at the magic number's origin,
but I've tested on hardware for a range of rail voltages for neutral
pH and it appears to hold true.

When we ordered this sensor, we also purchased 2 bottles of Standard
Buffer Solution - one at 4.01 pH, the other at 7.0 pH. The existing
driver was tested against those with excellent results. I will be more
than happy to retest with your changed formula, when ready.

*note - in the original dfrph c++ sensor code, the aRes value uses 1
<< adc #bits = (for default mraa) 1024. Since it's not possible to
read a value of 1024 from a 10 bit adc, it would not be possible to
achieve the max unit value from the sensor. I see the 1024 value
throughout our sensor C++ code.

We should try to write our math according to how values are
interpreted. An adc read can be viewed as a normalized value
(disregarding drop-out or offset from the sensor hardware):

Value(units) = ADCraw/ADCmax * UNITmax

float DFRPH::volts()
{
int val = m_aio.read();
return(val * (m_aref / m_aRes));
}

in my mind should be:

float DFRPH::volts()
{
int val = m_aio.read();
return(m_aref * (val / m_aRes));
}

Well, they are both equivalent -- I guess it's just a matter of how you
look at it, which is your point.I guess :)

I spent some time looking around this morning, and there are many sites
(including the sample code on that wiki page above) that use the 1024.0
value (for 10b ADC).

But, I also found other sites that suggest that 1023.0 is the proper
value, and math-wise, it works.

With 1024, we introduce a small error (4.8mV), which of course gets
smaller with higher ADC resolutions. It's mainly in the noise, which is
probably why Adruino and the like don't care about a 4mV difference. At
12b, the error drops to less than 1 uV. But still, it's an error we
should avoid.

Sparkfun actually has a nice page on this:

https://learn.sparkfun.com/tutorials/analog-to-digital-conversion/relating-adc-value-to-voltage

Explains it well. So, going forward we should compute voltage using
max_adc.

[jontrulson]

Close... But two issues:

• the "extra" throwaway analog reads. These need to go. See my comment on joystick12. Please do not add any more of these back into the code, and remove the ones you've added to joystick and this one. If this is really a problem, this is not the way to fix it.
• I guess I wasn't clear. The ADC value needs to be averaged over a number of samples, NOT the computed pH value. Also, this averaging should be supported in the C driver. The C++ driver can just use the functionality in the C driver.

[pylbert]

Again, w/o the double read, I cannot calibrate the sensor. We can address this in the c driver sync tomorrow.

Sorry Jon, I'm not sure I understand what you mean by averaging over a number of samples. Maybe you mean raw adc reads and then convert to pH? I also don't see why all C sensors need to have a filtering method. Isn't this easy enough to do from a calling method where the user will have control over the sample period?

[jontrulson]

On 07/27/2016 01:08 PM, Noel Eck wrote:

Again, w/o the double read, I cannot calibrate the sensor. We can
address this in the c driver sync tomorrow.

Ok, so I looked at the Sensor Notes - indeed I marked the pH sensors as
failed on Edison:

**DFRobot pH sensors

These (both pro and non-pro) fail on edison. It always reports 3.75v.
Unplugging the sensor (but not the sensor board) reports 0v to 4.99v,
randomly. G2 works fine (and seems pretty accurate).**

So maybe you are seeing a different manifestation of this issue
(different version of sensor board? /Definitely/ different edison firmware).

Sorry Jon, I'm not sure I understand what you mean by averaging over a
number of samples. Maybe you mean raw adc reads and then convert to
pH? I also don't see why all C sensors need to have a filtering
method. Isn't this easy enough to do from a calling method where the
user will have control over the sample period?

In the past it's never been a problem... Yes, I mean reading in a number
of ADC values (samples), and averaging them. Then with that value,
compute the result, whatever that is. If you average over the computed
value, you just amplify the noise.

As I mentioned, many of these sensors are noisy, it's just the way it
is. Averaging can smooth out the variations, within limits of course.
As for a period between samples - never really needed it for most
sensors... Again, the existing drivers HAVE all been tested on
hardware, with any anomalies noted in the Sensor problem document.