Pages: [1] 2   Go Down
Author Topic: analogRead problem with IDE 1.5.5 libraries  (Read 3489 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 19
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I have a DUE test board where I use most of the analog inputs. For that test setup, I have written an automated test that reads the analog inputs and compares it with expected values.
When compiling with IDE 1.5.4, all runs fine. Compiling the same sketch with 1.5.5, I get mostly wrong analogRead results. I suspect a problem with the ADC initialization change that was introduced with 1.5.5. While appreciating a higher speed for A/D conversion, for sure no such problem must result  from that change.

Please find below a few lines of code that I use.
I do not know if that could be important, but I apply the test voltages via 10kOhm series resistors (which should not be a problem since the analog inputs have a very high impedance). And another detail: I use an external 2.5V reference.

For now, I will revert to IDE 1.5.4, hopefully a solution will be found for 1.5.6.
Can anyone meanwhile confirm the bug, and is there a patch possible in the libraries of IDE 1.5.5?

#define A_U_IN 11

static const unsigned char AinPins[] = {
    0,1,2,3,4,5,6,7
};

unsigned short Ain[sizeof(AinPins)];
unsigned short Apower;
boolean AinOk[sizeof(AinPins)];
boolean ApowerOk;

// Note: external reference voltage is 2.5V
static const unsigned short AinTest[] = {
    1870, 1900, //=DAC0, set to value 1800, at A0
    2130, 2170, //=DAC1, set to value 2200, at A1
    795, 843,   //=0.50V at Ain2
    1203, 1255, //=0.75V at Ain3
    1610, 1667, //=1.00V at Ain4
    2018, 2078, //=1.25V at Ain5
    2425, 2490, //=1.50V at Ain6
    2833, 2902  //=1.75V at Ain7
};

// Following code is in a loop:

// Read the analog inputs and compare with pre-defined values
        for (i = 0; i < MIN(sizeof(AinPins), smiley-cool; i++) {
            Ain = analogRead(AinPins);
            AinOk = (Ain >= AinTest[i << 1] && Ain <= AinTest[(i << 1) + 1]);
        }
        Apower = analogRead(A_U_IN);
        ApowerOk = (Apower >= 2701 && Apower <= 3243);

// ... other I/O and text output via USB
Logged

0
Offline Offline
Shannon Member
****
Karma: 199
Posts: 11649
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Faster ADC settings require lower impedance source, that's just how it is, the sample/hold
capacitor must be charged in the time available....  With 10k and the input capacitance
of 8pF quoted in the datasheet 12 bit settling time is  665ns, which if you are configuring
the ADC for 1MHz operation is impossible, you have to charge the sample cap in a fraction
of the conversion time.

It sounds like what would be good is a conversion speed parameter, possibly three values,
slow/medium/fast, with documented source impedance requirements.
Logged

[ I won't respond to messages, use the forum please ]

Offline Offline
Newbie
*
Karma: 0
Posts: 19
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I am not sure if the source impedance is the only problem. I also partially see very similar converted values (e.g. 2898) for different analog inputs that are read in a fast sequence, while the input voltages differ as listed in the table in my source code.
If the source impedance was the only problem, we would just talk about decreased accuracy.
Is it possible that there needs to be an additional delay if the analog read input changes?
Logged

Kansas City area
Offline Offline
Newbie
*
Karma: 0
Posts: 18
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


It appears 1.5.5 lost the ability to use analog input numbers and only accepts analog input names.  Take a look at the short sketch below.  I'm going back to 1.5.4 until the bug is fixed.

Code:
void setup() {
  Serial.begin(115200);
  analogReadResolution(12);

  //These read properly
  Serial.println(analogRead(A0));
  Serial.println(analogRead(A1));
  Serial.println(analogRead(A2));
  Serial.println(analogRead(A3));
  Serial.println(analogRead(A4));
  Serial.println(analogRead(A5));
  Serial.println(analogRead(A6));
  Serial.println(analogRead(A7));
  Serial.println(analogRead(A8));
  Serial.println(analogRead(A9));
  Serial.println(analogRead(A10));
  Serial.println(analogRead(A11));
  Serial.println();

  //These read incorrectly
  for(byte n = 0; n <= 11; n++)  {
    Serial.println(analogRead(n));
  }
  Serial.println();

  while(1);  {}  //halt here

}

void loop() {
}


Jon
Logged

0
Offline Offline
Shannon Member
****
Karma: 199
Posts: 11649
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

No, I don't think that's the issue, the code for analogRead converts pin numbers the same
in 1.5.4 and 1.5.5, from analogRead():
Code:
  if (ulPin < A0)
    ulPin += A0;


One strange thing I've noted is that the ADC conversion time seems to change depending
on apparently unrelated code (calling Serial.println in setup(), for instance, after
analogReadResolution (12) smiley-wink.   Perhaps the delay causes the ADC to go into some sort
of over-run condition that affects the speed of subsequent conversions.  I'm seeing
a benchmark vary between about 1.7us and 2.0us dependent on this.

I haven't noticed any obvious difference in behaviour between using 0 and A0 in
analogRead.
Logged

[ I won't respond to messages, use the forum please ]

Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi guys,

The bug was introduced after the optimization of the ADC reads (shame on me  smiley-sad)

May I ask you to try with the following patch and check if that fixes the problem?
https://github.com/arduino/Arduino/issues/1740#issuecomment-30649822

replace the file Arduino/hardware/arduino/sam/cores/arduino/wiring_analog.c
with the file here: https://raw.github.com/arduino/Arduino/a1c48091051bfc7f3366cac6fc23212c4e4fb4a3/hardware/arduino/sam/cores/arduino/wiring_analog.c

Thanks
Logged

C.

Kansas City area
Offline Offline
Newbie
*
Karma: 0
Posts: 18
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


That fixes the read values, although conversion times are back to the same as 1.5.4.

http://www.djerickson.com/arduino/ has some information about speeding up ADC read times.  Using the line
Code:
REG_ADC_MR = (REG_ADC_MR & 0xFFF0FFFF) | 0x00020000;
reduces twenty reads in my sketch from 840uS to 240uS.

Jon
Logged

Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi stratosfear,

That fixes the read values, although conversion times are back to the same as 1.5.4.

Just checked and you're right, If you multiplex over many pins the ADC Startup time is applied at every ADC conversion and things slows up again. The speed increases only if you read from a single pin.

http://www.djerickson.com/arduino/ has some information about speeding up ADC read times.  Using the line
Code:
REG_ADC_MR = (REG_ADC_MR & 0xFFF0FFFF) | 0x00020000;
reduces twenty reads in my sketch from 840uS to 240uS.

The problem with this line is that it sets incorrect Tracking and Conversion timings for the ADC, see the discussion here for more details: https://github.com/arduino/Arduino/issues/1418

Now, the sad thing is that I've already fixed that but I've lost the fix with a clumsy move on git.
And I perfectly remember that I did the same steps we are taking here, i.e.:

1) I applied a patch similar to the one in my previous message
https://github.com/arduino/Arduino/commit/a1c48091051bfc7f3366cac6fc23212c4e4fb4a3
2) I've tested ADC again, and I've seen the slowdown like you did
3) I've applied another change that I can't remember now, but at the time was quite obvious.

So I know for sure that there is a solution and is simple, I just don't remember how smiley-sad

« Last Edit: December 17, 2013, 09:39:57 am by cmaglie » Logged

C.

0
Offline Offline
Shannon Member
****
Karma: 199
Posts: 11649
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Current issue seems to be that when you switch between different pins to analogRead()
all the ones you've already used remain enabled, so that the readings are completely
spurious (an adc_start() triggers multiple conversions).

I found out that simply disabling the current channel before enabling the new one
causes the ADC to shutdown and thus you incur a STARTUP delay.

So you have to disable the old channel _after_ enabling the new one.

Change this code in analogRead()
Code:
     if (ulChannel != latestSelectedChannel) {
        adc_enable_channel( ADC, ulChannel );
        latestSelectedChannel = ulChannel;
      }
to
Code:
     if (ulChannel != latestSelectedChannel) {
        adc_enable_channel( ADC, ulChannel );
       if (latestSelectedChannel != -1)
 adc_disable_channel (ADC, latestSelectedChannel) ;
        latestSelectedChannel = ulChannel;
      }

(In the section conditional on
#if defined __SAM3X8E__ || defined __SAM3X8H__
of course)

[ edit:  I think what is happening is that the conversion sequencer powers down the ADC between conversions if sleep mode is active, but also powers down if no channels are
currently enabled.   The datasheet has to be read between the lines here! ]
« Last Edit: December 17, 2013, 09:43:01 pm by MarkT » Logged

[ I won't respond to messages, use the forum please ]

Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

You find it! Thanks MarkT!

It's the second good fix in a row that you propose, I'm really impressed smiley

https://github.com/arduino/Arduino/commit/b530742603439c87a06d2da9ddd60dd9f82f5082

If someone wants to test it the updated wiring_analog.c is here:

https://raw.github.com/arduino/Arduino/b530742603439c87a06d2da9ddd60dd9f82f5082/hardware/arduino/sam/cores/arduino/wiring_analog.c

Quote
I think what is happening is that the conversion sequencer powers down the ADC between conversions if sleep mode is active, but also powers down if no channels are currently enabled.   The datasheet has to be read between the lines here!

As I said, at the time it seemed almost obvious, but I agree, the datasheet should be a little bit clear on that.

C
Logged

C.

0
Offline Offline
Shannon Member
****
Karma: 199
Posts: 11649
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thanks!  It took a little guesswork to finesse this issue - currently working
on interrupt-driven sampling and digital filtering audio with the Due as it seems
powerful enough to handle quite a lot of interesting stuff (although 12 bits isn't
enough)

I would like to have a go at interfacing a pair of I2S chips (ADC and DAC) using the
synchronous serial controller, but that's down the line a bit (thinking of an I2S
shield with 12.288MHz crystal oscillator, some Wolfson Micro 24 bit ADCs and DACs,
SD slot...)
Logged

[ I won't respond to messages, use the forum please ]

Offline Offline
Newbie
*
Karma: 0
Posts: 1
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I tried out both versions of wiring_analog.c with 1.5.5 and still have the same problem with analogRead not working properly.  I reverted to 1.5.4 and it works fine with my code.

I'm new to arduino, but have some embedded programming experience.  I'm happy to help debug the code if you can point me to the parts of the code that have changed between 1.5.5 and 1.5.4.
Logged

0
Offline Offline
Shannon Member
****
Karma: 199
Posts: 11649
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Erm: http://en.wikipedia.org/wiki/Diff
Logged

[ I won't respond to messages, use the forum please ]

Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

@donesolnas

the change applied are the following:

first (high speed, but breaks reading on multiple analog channels), EDIT:this one is in 1.5.5:

   https://github.com/cmaglie/Arduino/commit/1fc54f5003ccd7f9fc50d52698ad174c6f9ddd5e

second (reading are correct again but slow if you read on multiple channels), this one is in 1.5.5:

   https://github.com/arduino/Arduino/commit/a1c48091051bfc7f3366cac6fc23212c4e4fb4a3

third (reading are correct and at high speed as expected):

   https://github.com/arduino/Arduino/commit/b530742603439c87a06d2da9ddd60dd9f82f5082

Another way could be to download the nightly build that should have the patch now:

http://arduino.cc/en/Main/Software

« Last Edit: December 20, 2013, 05:13:21 am by cmaglie » Logged

C.

Offline Offline
Newbie
*
Karma: 0
Posts: 19
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I today tried the latest patch from the Nightly Build and it basically works in my application. However, accuracy is significantly reduced when compared to the analogRead from the 1.5.4 IDE.
I found a solution for that. The wiring_analog.c code has to be supplemented in analogRead:
Code:
delayMicroseconds(2); // this line new
// Start the ADC
adc_start( ADC );
I tried several delay times and 2microseconds seem to be sufficient to have comparable accuracy than before, and that time should not hurt too much on overall conversion time.
I suggest to take that patch over for the Nightly Builds and then into IDE 1.5.6.
Logged

Pages: [1] 2   Go Up
Jump to: