Yahoo Groups archive

AVR-Chat

Index last updated: 2026-04-28 22:41 UTC

Message

Re: [AVR-Chat] Arrays and pointers in GCC

2009-01-15 by Bill Knight

All
   I posted the following message three days ago and have not seen it 
appear in the group.  Did it make it or did I just miss the posting?

Regards
-Bill Knight
R O SoftWare
====================================================================

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
> 
>

Attachments

Move to quarantaine

This moves the raw source file on disk only. The archive index is not changed automatically, so you still need to run a manual refresh afterward.