David
I do not have an AVR compiler accessible to me right now but please
let me offer an alternative piece of code. Also, I did not see your
declaration of 'ADC_Pointer' which is actually an index, not a pointer.
unsigned char ADC_index;
ISR(ADC_vect)
{
ADC_Data[ADC_index] = ADCW;
ADC_index = ((ADC_index + 1) & 7);
ADMUX = (ADMUX & 0xF8) | ADC_index;
ADCSRA |= (1 << ADSC);
}
If you have the time, please give this a try and let me know how (if)
it works out any better for you.
-Bill Knight
R O Software.
PS - ADC_index declared as an 'unsigned int' (instead of an 'unsigned
char') might actually produce smaller code.
David VanHorn wrote:
> Is there a reason that I can't use a char as a pointer in an array of ints?
> My ADC experiment has an array of 8 ints to store the data, and it
> does grind my gears to use a 16 bit pointer into an 8 element array.
>
> In ASM, I'd only use three bits of a byte, saving the other five for
> flags, and absolutely preventing indexing outside the array, but I
> haven't worked out how to do that in C yet.
>
> Also, I did look at the compiled version of my ISR... Eww.
>
> I'm open to the idea that I'm not writing it in the right way for the
> compiler to be efficient yet, but I was impressed with how many
> instructions it took to do the ISR.
>
> Started with this code:
>
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> { // Store in array of locations per channel flag
>
> // Read the channel data MUST BE IN LOW-HIGH order!
> // low byte of ADC_Reading = ADCL
> // high byte of ADC_Reading = ADCH
> ADC_Data[ADC_Pointer] = ADCW; // ADC_Reading;
>
> // Select the next channel
> ADC_Pointer++; //Only I ever increment this!
> if (ADC_Pointer > 7) ADC_Pointer=0;
>
> ADMUX = ADMUX & 0xF8; // Save the other bits
> ADMUX = ADMUX |= ADC_Pointer; // Copy the pointer into ADMUX
> ADCSRA |= (1<<ADSC); // Start the next conversion
> }
>
>
> And got this with maximum optimization for size:
>
> // Reads all 8 channels even though we only use two.
> ISR(ADC_vect)
> { // Store in array of locations per channel flag
> de: 1f 92 push r1
> e0: 0f 92 push r0
> e2: 0f b6 in r0, 0x3f ; 63
> e4: 0f 92 push r0
> e6: 11 24 eor r1, r1
> e8: 2f 93 push r18
> ea: 3f 93 push r19
> ec: 8f 93 push r24
> ee: 9f 93 push r25
> f0: ef 93 push r30
> f2: ff 93 push r31
>
> // Read the channel data MUST BE IN LOW-HIGH order!
> // low byte of ADC_Reading = ADCL
> // high byte of ADC_Reading = ADCH
> ADC_Data[ADC_Pointer] = ADCW; // ADC_Reading;
> f4: 20 91 88 00 lds r18, 0x0088
> f8: 30 91 89 00 lds r19, 0x0089
> fc: f9 01 movw r30, r18
> fe: e2 0f add r30, r18
> 100: f3 1f adc r31, r19
> 102: e6 57 subi r30, 0x76 ; 118
> 104: ff 4f sbci r31, 0xFF ; 255
> 106: 84 b1 in r24, 0x04 ; 4
> 108: 95 b1 in r25, 0x05 ; 5
> 10a: 91 83 std Z+1, r25 ; 0x01
> 10c: 80 83 st Z, r24
>
> // Select the next channel
> ADC_Pointer++; //Only I ever increment this!
> 10e: c9 01 movw r24, r18
> 110: 01 96 adiw r24, 0x01 ; 1
> 112: 90 93 89 00 sts 0x0089, r25
> 116: 80 93 88 00 sts 0x0088, r24
> if (ADC_Pointer > 7) ADC_Pointer=0;
> 11a: 08 97 sbiw r24, 0x08 ; 8
> 11c: 20 f0 brcs .+8 ; 0x126 <__vector_16+0x48>
> 11e: 10 92 89 00 sts 0x0089, r1
> 122: 10 92 88 00 sts 0x0088, r1
>
> ADMUX = ADMUX & 0xF8; // Save the other bits
> 126: 87 b1 in r24, 0x07 ; 7
> 128: 88 7f andi r24, 0xF8 ; 248
> 12a: 87 b9 out 0x07, r24 ; 7
> ADMUX = ADMUX |= ADC_Pointer; // Copy the pointer into ADMUX
> 12c: 87 b1 in r24, 0x07 ; 7
> 12e: 90 91 88 00 lds r25, 0x0088
> 132: 89 2b or r24, r25
> 134: 87 b9 out 0x07, r24 ; 7
> 136: 87 b1 in r24, 0x07 ; 7
> 138: 87 b9 out 0x07, r24 ; 7
> ADCSRA |= (1<<ADSC); // Start the next conversion
> 13a: 36 9a sbi 0x06, 6 ; 6
> 13c: ff 91 pop r31
> 13e: ef 91 pop r30
> 140: 9f 91 pop r25
> 142: 8f 91 pop r24
> 144: 3f 91 pop r19
> 146: 2f 91 pop r18
> 148: 0f 90 pop r0
> 14a: 0f be out 0x3f, r0 ; 63
> 14c: 0f 90 pop r0
> 14e: 1f 90 pop r1
> 150: 18 95 reti
>
>
> Normally in any app I write in ASM, I dedicate two registers for usage
> in the ISR, ITEMP and ITEMP2, and one to hold SREG, "STEMP"
> So getting into this would be something like:
>
> ADC_ISR:
> in STEMP,SREG
>
> code...
>
> out SREG,STEMP
> reti
>
>Message
Re: [AVR-Chat] Arrays and pointers in GCC
2009-01-12 by Bill Knight
Attachments
- No local attachments were found for this message.