Re: RE: RE: [avr-gcc-list] buggy variable naming with underscores

[Dave Hansen]

From: Jamie Morken <address@hidden>
[...]
> Ok, I simplified the code, it just reads all 8 ADC channels on the atmega8 
> and sends them over the UART to the PC.  Please try it out and see if you 
> can figure out why the channels are all shifted by 1 index in the array!  
> Here is the code.
[...]
> SIGNAL(SIG_ADC)
> {
>         u08 lt;
>         u08 ht;
>         lt = ADCL;
>         ht = ADCH & 0x03;
>
> 	ADChannels[nNextAdc]=(((u16)ht)<<8)|lt;		//all channels shifted by one 
> array index when printed with printf below
>
> 	//ADChannels[nNextAdc-1]=(((u16)ht)<<8)|lt;	//this line fills array 
> correctly, but may corrupt memory
>
>         nNextAdc++;
>         if (nNextAdc==8) nNextAdc=0;
>         ADMUX = nNextAdc;
> }
>
>
> void INIT_Adc(void)
> {
>         nNextAdc=0;
>         DDRC = 0;
>         PORTC = 0;
>         ADMUX = nNextAdc;
>         ADCSRA = 0xEF;
> }

You've selected an auto trigger mode for the ADC, but have modified SFIOR, 
so you're in free-running mode (I'm looking at a mega16 datasheet, but they 
should be similar).  Thus, by the time your ADC ISR runs, the next 
conversion has already started.  Your updated MUX value won't take effect 
until that conversion is complete.  So you've shifted the values yourself.

Simplest solution would be to disable free-running mode (ADCSRA = 0xCF), and 
start each conversion in the ISR _after_ you've written the mux (ADMUX = 
nNextAdc; ADCSRA |= _BV(ADSC);).  This will slow you down slightly, but not 
much.

If you just gotta gotta gotta have free-running mode, keep two channel 
variables, current and next.  In your ADC init routine, set current and the 
mux to zero, start the conversion, then set next and the mux to 1.  In your 
ISR, save the result in array[current], set current to next, increment next 
modulo 8, and set the mux to next.

HTH,
  -=Dave