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

Improved ADC speed on Arduino Due #1634

Merged
merged 1 commit into from Nov 12, 2013
Merged

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Oct 21, 2013

Proposal to improve ADC reading speed: keep the ADC enabled after every ADC conversion.

This will bypass ADC startup time, improving its conversion rate at the cost of slightly increasing power consumption (less than 1mA 5mA).

See #1418 for more discussion.

@jtouch
Copy link

jtouch commented Oct 21, 2013

I support the idea of leaving the ADC running to avoid the STARTUP wait time.

However, from the specs we're talking an additional 5-9 mA, not <1mA. That's still very small, though.

Sleep Mode 0.1 uA typ (1 max)
Normal Mode (IBCTL= 00) 4.7 mA typ (7.1 max)
Normal Mode (IBCTL= 01) 6 mA typ (9 max)

cmaglie added a commit that referenced this pull request Nov 12, 2013
Improved ADC speed on Arduino Due
@cmaglie cmaglie merged commit 3ba9480 into arduino:ide-1.5.x Nov 12, 2013
@cmaglie cmaglie deleted the adc-fix branch November 12, 2013 12:45
@cmaglie cmaglie mentioned this pull request Dec 16, 2013
@dc42
Copy link

dc42 commented Dec 21, 2013

I would like to see the analogRead function take 2 additional parameters, both of them optional (i.e. with default values).

The first is a delay time in microseconds to wait between setting the input multiplexer and starting the conversion. This is mainly for the atmega-based Arduinos, to allow additional sample time when using inputs with high source resistance. For example, using a delay of 10us here gives accurate readings with source resistance up to 100K.

The second would be a boolean parameter, which on the Due would indicate whether to leave the ADC enabled or not.

The default values of these parameters could be set so as to leave the behaviour unchanged from the current behaviour when the new parameters are not provided.

@jtouch
Copy link

jtouch commented Dec 23, 2013

In both cases, it seems like these might be more useful as additional
commands or flags that aren't in the common set. I.e., I doubt it would
be useful to complicate the common code with checks for optional
parameters, rather than to just create new commands - these would be
rarely used, and seem to complicate the common case.

Joe

On 12/21/2013 6:03 AM, dc42 wrote:

I would like to see the analogRead function take 2 additional
parameters, both of them optional (i.e. with default values).

The first is a delay time in microseconds to wait between setting the
input multiplexer and starting the conversion. This is mainly for the
atmega-based Arduinos, to allow additional sample time when using inputs
with high source resistance. For example, using a delay of 10us here
gives accurate readings with source resistance up to 100K.

The second would be a boolean parameter, which on the Due would indicate
whether to leave the ADC enabled or not.

The default values of these parameters could be set so as to leave the
behaviour unchanged from the current behaviour when the new parameters
are not provided.


Reply to this email directly or view it on GitHub
#1634 (comment)
Bug from MailScannerWebBug

@JYMellen
Copy link

I think this change to analogRead may have caused an issue for long term continuous reads of multiple channels. I have an application that reads six A/D channels and processes the information continuously. A/D readings fail at random times ranging all the way to about 5000 minutes. This is not the case when the sketch is compiled under 1.5.1 (I have test devices that are currently in the 120,000+ minute range). This is on a Due. The significant difference that is that the sketch compiled under 1.5.7 runs about 500 hz, whereas under 1.5.1 its about 80 hz. Failure results in bad reads INF and NaN values reported back from the analogRead.

@dc42
Copy link

dc42 commented Aug 27, 2014

JYMellen, how can you be getting INF and NaN values, which only exist for float/double types, when analogRead returns an int?

@aramazhari
Copy link

I agree with JYMellen, this has caused incorrect input readings for continuous analog inputs.
Delay () seem to have no effect on the matter.
I have read a suggestion somewhere that was about repeating analogRead () three times to have the correct value.

I believe if the command is not going to have overload functions then there has to be a very clear warning on the analogread docs for Due.

This kept me busy for days trying to recheck everything and finally finding out that it is an engineering mistake.

Simply put, continously reading analoginput is pointless on Due and should be avoided.

@jtouch
Copy link

jtouch commented Feb 22, 2016

It would be useful if you (aramazhari) could be more specific about the failure. I agree with dc42 - the error reported by JYMellen doesn't make sense for an int.

@aramazhari
Copy link

Ok. So I would like to provide a simple program that explains the matter at hand which is as follows:

1.Attach two 50K potentiometers PotA and PotB to analog pin 0 (PotA) and pin 1 (PotB) (I'm using the 3 volt and the gnd from the Due board).
2.Run this loop (The rest of the code is up to you):

uint_32t A;
uint_32t B;
void loop()
{
A = analogRead(0);
B = analogRead(1);
Serial.print(A);
Serial.print(" , ");
Serial.println(B);
}

3.Observe A and B values.

4.Try turning PotB (pin 1 - variable B) and see the value of variable A changing at some intervals (ex. when PotB value is larger 1000 the PotA may increase by 10 or so and vice versa)

5.Now run this code:

uint_32t A;
uint_32t B;
void loop()
{
A= analogRead(0);
A= analogRead(0);
A= analogRead(0);
B = analogRead(1);
B = analogRead(1);
B = analogRead(1);
Serial.print(A);
Serial.print(" , ");
Serial.println(B);
}
6.Try turning pot connected to pin 1 (variable B) and see the code work as expected without any problems.

Note that this issue is reproducable and has been experienced by others

I hope this is clear enough. The point here is not whether the read value is inf,NaN (which I agree is only for floating point precisions), but to observe that analog pin input currents are (in common language) leaking to each other and causing the read values invalid.

Since this is not the expected behaviour, there should either be a clear indication of the situation in the docs or this implementation of analogRead should either be improved for Due or simply provide function overloads.

I understand that the whole point of Arduino is to simplify for the faster prototyping, which is highly appreciated. The cost here is not only slower performance (which is by design of simplification and can be tolerated) but also having actually incorrect analog input readings. The latter is not acceptable and should be considered as a bug.

@jtouch
Copy link

jtouch commented Feb 22, 2016

Oh - in that case, you're seeing the parasitic capacitance of the ADC mux circuit. I wouldn't be surprised if it would be useful to "drain" that away when switching between inputs.

Try this:

Wire pin 2 to ground.

Now try:
(void)analogRead(2);
A= analogRead(0);
(void)analogRead(2);
(void)analogRead(1);

(use any method to discard the reads of pin 2)

Reading pi2 before each analogRead should dump that capacitance to ground quickly enough.

Yes, I would expect the Due circuitry should have taken care of this, but if it didn't at least this should work too.

@peabo7
Copy link

peabo7 commented Feb 23, 2016

@jtouch that's an interesting idea, thanks for suggesting it. You could try to drive the capacitor to the previously read value by varying the on and off time of the other measurements (ground or Vcc). Then the subsequent reading would be limited by the settling time of the newly measured signal.

I think ultimately that the procedure has to be hand-crafted to the various analog inputs, which typically have varying input impedances.

If you have a high impedance input, you must necessarily give it time for the sample/hold capacitor to acquire the value.

Figuring the actual sampling rate of each signal, with a timing loop which samples at twice the individual rate would help solve this problem. This is probably a medium hard problem, and may need the code to operate at interrupt level.

Peter Olson

@aramazhari
Copy link

@jtouch @peabo7 Thanks for your inputs.

Both of the suggested solutions are workarounds for a problem that does not exist on Arduino UNO or Mega. My point is not how to get around it, but how to let everyone else in the community know that there is such an issue specific to Due along with workaround solutions such as yours in the docs page.

The second piece of the code that I mentioned already solves the problem, given that it repeats the analogRead() method multiple times just as @jtouch suggested.

Using a variable resistor is probably one of the basic steps of prototyping as well as discovery of embedded systems. It shouldn't be this hard to go around things.

@peabo7
Copy link

peabo7 commented Feb 23, 2016

I just realized that you don't need GND or Vcc wired inputs for this, provided you are able to keep information about the last sampled value and the sampled values of other inputs and their impedances.

You can use any of them as current sources to adjust the sample/hold capacitor, as long as the respecting input impedance is large enough that the this unexpected use doesn't feed through significantly to the measurement points. It does anyway in the present regime, but in a mostly random way.

Ultimately the solution is to avoid reading analog inputs from different channels faster than you need to and guarantee settling time appropriate to the input impedance of a channel.

Putting more thought than I have today could be really helpful.

Peter Olson

@aramazhari
Copy link

@peabo7 Note that delay() function does not help at all.
The following code:

uint_32t A;
uint_32t B;
void loop()
{
delay(200); // These are very large and unacceptable delays by the way
A = analogRead(0);
delay(200);
B = analogRead(1);
delay(200);
Serial.print(A);
Serial.print(" , ");
Serial.println(B);
}

still suffers from the issue.

@peabo7
Copy link

peabo7 commented Feb 23, 2016

@aramazhari I like event-driven loop style, where individual actions such as reading the ADC are scheduled via the microsecond timer. In this scheme, nobody ever does analogRead explicitly, but instead uses a layer of subroutines to say when they next need a reading, and something in the loop figures out when to read the value and also knows what channel is needed to be read, schedules it, and implements an API to read the event and/or to notify of a significant change.

This doesn't work with the ADC interface as currently implemented, which is why I suggested there might be helpful to interrupt routines.

It also might be difficult if there are libraries that use the current analogRead().

@aramazhari
Copy link

@peabo7 again, all these suggestions are very useful. That is why it should be included in the official documentation of arduino due api.

@dc42
Copy link

dc42 commented Feb 23, 2016

This problem is caused by the need for the sample capacitor in the ADC to charge through the source resistance after the mux has switched sources and before the conversion is started. It exists on both the AVR and the SAM processors.

In the case of the AVR, it occurs when the source resistance is greater than 10K. I asked that a configurable delay be included in analogRead between switching the mux and starting the conversion about three years ago. My suggestion was rejected (so I have used my own version of the Arduino core ever since then). I asked for it again a few posts earlier in this thread, and the response was that it makes the code too complicated (even though it only needs about 4 lines of code to implement).

In the case of the SAM, this problem can occur even with 10K source resistance. The chip itself provides a configurable delay between switching the mux and starting the conversion, but the Arduino core initialises this to zero (which is a mistake IMO). In RepRapFirmware we added the following initialisation code to avoid the problem:

// Reconfigure the ADC to avoid crosstalk between channels (especially on Duet 0.8.5)
adc_init(ADC, SystemCoreClock, ADC_FREQ_MIN, ADC_STARTUP_FAST);     // reduce clock rate
adc_configure_timing(ADC, 3, ADC_SETTLING_TIME_3, 1);               // add transfer time

This code also reduces the ADC clock speed, which we can do because in RepRapFirmware we only do one conversion every millisecond. But you can try making just the adc_configure_timing call, because AFAIR it is the ADC_SETTLING_TIME_3 parameter that makes the largest difference.

@peabo7
Copy link

peabo7 commented Feb 23, 2016

@aramazhari This would fix it, but it is really awkward because you have to predict reliably in advance which channel needs reading. I reduced the delay without figuring what it needs to be.

What is really needed is analogSelect(int channel) to set the registers for the next conversion without waiting for anything.

Debugging this without relying on Serial.print* is tricky, but can be done.

uint_32t A;
uint_32t B;
void loop()
{
A = analogRead(0);
// analogSelect(1);   // no wait or else
analogRead(1);
delay(10);
B = analogRead(1);
// analogSelect(0);   // no wait or else
analogRead(0);
Serial.print(A);
Serial.print(" , ");
Serial.println(B);
}

@peabo7
Copy link

peabo7 commented Feb 23, 2016

@aramazhari Oops. did it wrong

uint_32t A;
uint_32t B;
void loop()
{
A = analogRead(0);
// analogSelect(1);   // no wait or else
delay(10);
analogRead(1);
B = analogRead(1);
// analogSelect(0);   // no wait or else
delay(10);
Serial.print(A);
Serial.print(" , ");
Serial.println(B);
}
111

@aramazhari
Copy link

@dc42 thanks for mentioning how to adjust the mux delay. I think this is better than other suggestions.

@peabo7 as i said before the delay () function has no effect.

@jtouch
Copy link

jtouch commented Feb 23, 2016

"What is really needed is analogSelect(int channel) to set the registers for the next conversion without waiting for anything." - I disagree. The time for this switching to have an impact depends on how much charge has been accumulated and whether the input you select can drain/fill that charge fast enough.

Adding a delay parameter would help only if you knew that the sampling circuit would drain/fill the charge during the delay. If you switch from low-impedance source to a low-impedance source and the voltage values differ significantly, you end up not being able to quickly drain/fill the on-board circuit fast enough.

IMO, this is already explained in the documentation - these devices are not great at sampling high-impedance sources. That's not uncommon for any measuring device (e.g., oscilliscope probes have a variety of impedances).

IMO, optimizing for speed and warning users of the hazards of high-impedance sources is the best solution. If you want to sample high-impedance sources, it seems like it would be more effective to insert an op-amp follower to make the external circuit match the device requirements - rather than just trying to slow down the device and hope it works.

@dc42
Copy link

dc42 commented Feb 23, 2016

IMO, this is already explained in the documentation - these devices are not great at sampling high-impedance sources. That's not uncommon for any measuring device (e.g., oscilliscope probes have a variety of impedances).<<

Actually, these chips are just fine at sampling high impedance sources, if you use an appropriate delay after switching the mux. I have an atmega328-based device that uses a source resistance of 2.35Mohms on one of the ADC channels, to monitor the voltage of the 9V battery powering it through a voltage divider comprised of two 4M7 resistors. The resistors are that high to avoid draining the battery, because the unit is powered off by putting the mcu into sleep mode.

For the atmega328p, a 10us delay between switching the mux and starting the conversion provides accurate readings for source resistances of up to 100K, for all values of voltages on the old and new channel inputs. Because the ADC conversion time is about 110us, this extra delay is not very significant.

@jtouch
Copy link

jtouch commented Feb 24, 2016

Delay makes assumptions about how fast the internal circuit can dump charge. The only safe solution is to dump the charge somewhere else explicitly.

See the discussion here: http://forum.arduino.cc/index.php?topic=18339.0

See esp the following part: "But anyway, if you take out the code that reads the pin with the 10K pulldown on it, everything goes pear shaped and the values read from the touch switches end up all over the place. That is, unless those delays are more like 20-40ms. "

@dc42
Copy link

dc42 commented Feb 24, 2016

The "internal circuit" you speak of is just a capacitor. It dumps or acquires charge through the multiplexer. The only assumption you need to make if you want to calculate the delay needed as a function of source resistance is that this capacitance does not vary hugely from its nominal value, which is 14pF for the atmega328p (see fig. 24-8 in the datasheet). There might be a 2:1 variation in the capacitance, although I doubt that it's as high as that.

Regarding that discussion you link to, all he is doing is ensuring that instead of getting crosstalk between his two resistive touch pads, he gets crosstalk between each pad and a pin that is at a fixed potential (i.e. ground). No wonder it works much better.

@jtouch
Copy link

jtouch commented Feb 24, 2016

Agreed, but the issue is that the specific processor (and thus C S/H) can vary wildly.

There's no corresponding circuit shown for the Due's SAM processor.

This isn't "crosstalk" (there's no induction going on here); it's really just charge that accumulates in the capacitance of the circuit as a whole (not just the indicated C S/H). The fixed potential isn't a reference; it's discharging the capacitor.

@dc42
Copy link

dc42 commented Feb 24, 2016

  1. From Wikipedia:

"In electronics, crosstalk is any phenomenon by which a signal transmitted on one circuit or channel of a transmission system creates an undesired effect in another circuit or channel."

So IMO this is a correct use of the word 'crosstalk', just not the inductive kind of crosstalk that you are thinking of.

  1. The reason that crosstalk between channels (i.e. the input voltage to one channel affecting the reading from another channel) occurs is precisely because the S/H capacitor is charged to the input voltage of one channel via the mux, then switched to another channel of the mux, but very quickly switched away from the mux to the successive approximation circuitry. Yes there will be a small amount of parasitic capacitance in the mux, but it will be a lot smaller than the S/H capacitor, which is a fabricated capacitor in the chip and therefore much larger than the parasitic capacitance of the rest of the circuit (if it were not, then the whole successive approximation circuit wouldn't work properly). The current AVR Arduino core switches the mux and then starts the conversion in the very next instruction, which is asking for trouble unless the source resistance of the ADC input is very low.
  2. The designers of the SAM3X8E ADC clearly recognised the need for additional delay between changing the mux and starting the conversion, because the ADC Mode Register includes the TRACKTIM field, which provides a delay of up to 16 ADC clocks between switching the mux to the new channel and starting the conversion. But the last Arduino SAM core I looked at sets TRACKTIM to zero.

IMO what we need is one extra trailing parameter to analogRead, with a default value so that it can be omitted. In the AVR core, this parameter should be used to insert a delayMicroseconds call between changing the mux and starting the conversion. In the SAM core, it should be used to set the TRACKTIM field of the ADC Mode Register.

btw I've been working with both analog and digital electronics for more than 40 years, so I do know what I am talking about.

@jtouch
Copy link

jtouch commented Feb 24, 2016

We don't need to be too pedantic about crosstalk, but the crosstalk here is semantic not real. It's one circuit used for two different events but whose state isn't being cleaned up. Typically crosstalk is for two separate circuits.

The question about adding the delay is important - how does the delay help if both muxed inputs are very high impedance? In particular, measuring one input will always contaminate the next input you measure (by the amount of that capacitance). How can you ever know what delay to actually use? Isn't that a function of the ability of the next input to drain/charge the capacitor (and you can't know that because you don't know what you'll attach and you don't know the capacitance of every Arduino device)?

IMO, this seems like it points right back to the part documentation advice - don't use high impedance inputs.

@dc42
Copy link

dc42 commented Feb 25, 2016

Adding capacitance on high impedance inputs actually helps, because it provides a reservoir from which the S/H capacitor can be charged. Let's take the worst case, which is a voltage we want to sample fed through a resistor to an input and no additional capacitance connected to that input. The S/H capacitor inside the ADC is connected to another input at a different voltage, then we switch it over to "our" input. We need to wait for it charge or discharge through the resistor, until the voltage on the capacitor is close enough to the input voltage on the other side of the resistor to provide an accurate reading when we start the conversion.

It's fairly obvious that the worst case will be when our input is at ADVREF and the previous input is at ground potential, or vice versa. To get an accurate reading, we want the failure of the S/H capacitor to fully charge or discharge to introduce an error of no more than 0.5LSB under these conditions. So for an AVR with a 10-bit ADC, the problem reduces to having a delay long enough that the S/H capacitor C charges from zero up to a voltage that is at least 2047/2048 times the input voltage.

The voltage on a capacitor charging from zero through a resistor connected to a constant voltage source is given by the equation V = Vo ( (1 - exp(-t/RC)). Setting V = 2047/2048 * Vo, we get exp(-t/RC) = 1/2048. Rearranging this gives t = RC ln(2048), so t = RC * 7.62.

Let's plug the numbers in, assuming C = 14pF as on the datasheet:

R = 10K -> t = 1.06us
R = 100K -> t = 10.6us
R = 1M -> t = 106us

This is the first time I have done this calculation, however the result for R=100K has turned out to be similar to what I determined empirically, which is that a 10us delay is sufficient to get accurate results with 100K source resistance. If we want to allow for manufacturing tolerances in the value of the S/H capacitor, it may be prudent to increase this delay by about 50%.

Note that at R=1M the ADC accuracy will be degraded, because the datasheet specifies the input resistance of the ADC as being about 100M. But not every application needs 10 bits of accuracy. The 1us delay needed when R=10K is provided by the ADC itself, because the ADC does the sample and hold 1.5 ADC clocks after it sees the start conversion command.

@jtouch
Copy link

jtouch commented Feb 25, 2016

I agree completely with the analysis above, but I'm deeply concerned about the impact on the circuit you're trying to measure.

It's like measuring the swing of a pendulum by putting a large mass on it. Sure, when the pendulum is still, the mass helps transfer energy to the measuring device. But it also interferes with the pendulum swinging.

IMO, dumping the charge on the internal capacitor to a direct wire to ground is the better way to handle this issue - it is the best way to give a clean reading when switching between inputs. I would have thought the ADC circuit designers would have consider that in their on-chip design so we wouldn't have to burn a pin to do this, but so be it.

ollie1400 pushed a commit to ollie1400/Arduino that referenced this pull request May 2, 2022
Improved ADC speed on Arduino Due
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

Successfully merging this pull request may close these issues.

None yet

6 participants