4
\$\begingroup\$

I have a device on ATMega16 microcontroller that is supposed to constantly send ADC measurement results over USART. The controller is operating at 16MHz with an external crystal and ADC prescaler is set to 128. I have tried two methods of performing ADC conversion and sending the results.

  1. The first method is based on interrupts
    int main(void) {
        ...
        while (true) {}
    }

    ISR(ADC_vect) {
        USARTSendByte(ADCL);
        USARTSendByte(ADCH);
        ADCSRA |= 1 << ADSC;
    }
  1. The second method is based on manually starting the conversion
    int main(void) {
        ... 
        // main loop
        while (true) {
            if (adcEnabled) {
                ADMUX |= channel;
                ADCSRA |= (1 << ADSC);
                while (!(ADCSRA & (1 << ADIF))) {
                    // Do nothing
                }
                ADCSRA |= (1 << ADIF); // Clear ADIF            
                USARTSendByte(ADCL);
                USARTSendByte(ADCH);
            }
        }
    }

I've performed a number of tests which consisted of sending 32 blocks of 512 bytes (16384 bytes in total) over USART and measuring transmission time. In the first case (interrupts) the average time was 1623.13ms. The smallest result was 1535ms and the largest - 1712ms. In the second case the result was 1600.38ms which is 22.75ms less than in previous case, with a smallest result of 1530ms and the largest result of 1679ms.

So the question is: will interrupts actually decrease ADC performance and why is that happening or the results of my tests were inconclusive?

\$\endgroup\$
  • \$\begingroup\$ note, that you can check for ADSC flag in ADCSRA - the micro changes it to 0 after conversion is done, this way you don't have to clear the interrupt flag. \$\endgroup\$ – miceuz Jun 27 '13 at 5:57
6
\$\begingroup\$

What you're probably measuring is the overhead of entering and exiting the ISR in software. Because you haven't decorated the ISR it is saving the entire register state and restoring it on return. Your ISR can be declared to be naked and you can manage the preservation of the SREG and other critical registers yourself (e.g. in inline assembly) and that will get you where you want to be.

You don't really want to be calling functions from the ISR either. Just move bytes into a global buffer and do stuff with the buffered data from your main loop (like send them out over the UART).

From the avr-gcc manual:

define ISR_NAKED

ISR is created with no prologue or epilogue code. The user code is responsible for preservation of the machine state including the SREG register, as well as placing a reti() at the end of the interrupt routine.

\$\endgroup\$
  • \$\begingroup\$ Thanks, unfortunately I'm inexperienced with AVR's assembly, so I guess I'll have to find some more info on preserving the sreg register. \$\endgroup\$ – Ashton H. Jun 27 '13 at 0:03
  • \$\begingroup\$ it's easy enough to write a minimal example in C, compile it, look at the generated assembly listing, and copy it into your interrupt handler 'verbatim', augmented with a minimalist prologue and epilogue \$\endgroup\$ – vicatcu Jun 27 '13 at 12:12
3
\$\begingroup\$

Are you planning to do anything else in the microprocessor, other than send the ADC readings? If not, forget using interrupts and just go with everything in the main loop. Interrupts are really only useful if you're trying to do something else in the "background".

\$\endgroup\$
  • \$\begingroup\$ "Interrupts are really [...] useful if you're trying to do something else in the "background"" ... or if you want/need to use power-saving features. \$\endgroup\$ – JimmyB Jul 4 '13 at 12:02
1
\$\begingroup\$

Don't bother (yet) to use assembler. You don't need that.

However an important (general) point is: Don't call blocking functions from within an ISR! An ISR should never have to wait for anything.

Vicatcu is right: Use some kind of 'buffer', which can be as simple as a single global uint16_t variable, to which the ISR writes its result and the main loop picks it up when it can.

Another note: In your code you acquire an ADC result, send it out over the USART, and then start the next conversion. You should be able to get the ADC result, start the next conversion, and only then transfer the data, while the ADC is already processing the next sample at the same time. Or, you may want to look at the "free-running" mode of the ADC where it automatically starts the next conversion as soon as the previous one finished.

To illustrate how a generic buffering approach could be implemented, this may serve as an example:

volatile uint16_t adcValueBuffer;
volatile uint8_t adcValueBufferValidFlag;

ISR(ADC_vect) {

    adcValueBuffer = ADC;

    adcValueBufferValidFlag = 1; // This signals that the ADC provided a new value for the code outside the ISR.

    ADCSRA |= 1 << ADSC;

}


int main() {
  uint16_t adcValueCache; // Local variable which will hold the ADC value until it is completely transmitted.

  while( true ) {

    // Wait for the ISR to signal that a new value is available:
    while ( adcValueBufferValidFlag == 0 ) {
    }

    adcValueBufferValidFlag = 0; // Re-set flag. Will be set again by the ISR when a new ADC value becomes available.

    // Make sure that we read the buffered value atomically:
    cli();

    adcValueCache = adcValueBuffer;

    sei();

    USARTSendByte( (uint8_t)adcValueCache );
    USARTSendByte( (uint8_t)(adcValueCache >> 8));

  }

}

If/when the ADC is constantly sampling faster than the USART can transmit the data there's no way not to lose samples. In that case you'll need to synchronize both processes (sampling and transmitting) as you already did, starting the next sampling only after the (slower) transmission operation finishes.

Apart from that, queues or circular buffers (a.k.a. ring buffers) are often used to exchange data at varying rates between asynchronous components of the program: the ISR appends data to the buffer and the main-loop then takes (and removes) as many elements as are available in the buffer when it has the time.

These buffers can be implemented in two flavors regarding overflow conditions: one type discards any new data when the buffer is full, the other one just overwrites the oldest data in the buffer.

If that's what you need, just search for "avr circular buffer"; there are plenty of implementations out there.

\$\endgroup\$
  • \$\begingroup\$ Calling a function from an ISR is not bad by itself, but the function should not run longer than say some µs. The rule simply is: Keep the code that's run in an ISR as short as possible. -- You should not have to disable interrupts during transmission via USART, but, and I think that's your point, you should disable interrupts while reading the data provided by the ISR. You can easily 'cache' that data in main() like: cli(); temp = adc_value; sei(); processAdcValue( temp ); \$\endgroup\$ – JimmyB Jul 1 '13 at 17:24
  • \$\begingroup\$ My USART transfer routine looks like this: while ((UCSRA & (1 << UDRE)) == 0) {/*Do nothing*/} UDR = temp;. So if an interrupt occurs before the temp data is sent I will end up sending a different value due to temp being overwritten. This is actually why I wanted to disable the interrupts during data transfer. \$\endgroup\$ – Ashton H. Jul 1 '13 at 19:20
  • 1
    \$\begingroup\$ Please have a look at my edit. \$\endgroup\$ – JimmyB Jul 2 '13 at 8:52
  • 1
    \$\begingroup\$ Added another edit :) \$\endgroup\$ – JimmyB Jul 3 '13 at 12:43
  • 1
    \$\begingroup\$ That's great :) You may consider to accept my answer then. \$\endgroup\$ – JimmyB Jul 4 '13 at 11:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.