In an attempt to sample the A/D on a fixed interval, and then send result to the LCD and the serial port, I have put most of my code in an interrupt. But I would like to move more code into main for example.

FILE lcd_stream = FDEV_SETUP_STREAM(lcd_putchar, 0, _FDEV_SETUP_WRITE);

But I can’t make lcd_stream volatile, I can’t even initialize lcd_stream as FILE without the rest of the string. My program seems to work OK I would like to know more about volatile and initializing FILE types. I would think

FILE lcd_stream;
lcd_stream = FDEV_SETUP_STREAM(lcd_putchar, 0, _FDEV_SETUP_WRITE);

would work but it has to be one line?

Code so far:

// Analog input.c
// for NerdKits with ATmega328p
// scootergarrett

#define F_CPU 14745600

#include <stdio.h>
#include <math.h>

#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <inttypes.h>

#include "../libnerdkits/io_328p.h"
#include "../libnerdkits/delay.h"
#include "../libnerdkits/lcd.h"
#include "../libnerdkits/uart.h"

// PIN DEFINITIONS:
// PC0 -- Analog input
// PC4 -- LED output for testing

void adc_init()
{
  ADMUX=0;
  ADCSRA=(1<<ADEN)|(1<<ADPS2)|(1<<ADPS1)|(1<<ADPS0);
  ADCSRA|=(1<<ADSC);
}

uint16_t adc_read()
{
    while(ADCSRA&(1<<ADSC)){}
    ADCSRA|=(1<<ADSC);
    return ADCL+(ADCH<<8);
}

int main()
{
    // start up the Analog to Digital Converter
    adc_init();

    // start up the serial port
    uart_init();
    FILE uart_stream = FDEV_SETUP_STREAM(uart_putchar, uart_getchar, _FDEV_SETUP_RW);
    stdin = stdout = &uart_stream;

    // start up the LCD
    lcd_init();

    //Interrupt setup stuff
    TCCR1B|=(1<<CS12)|(1<<CS10)|(1<<WGM12);
    TIMSK1|=(1<<OCIE1A);
    OCR1A=2880;             //Interrupt timming
    sei();

    DDRC|=(1<<PC4);         //LED for testing

    while(1)
    {
    }
    return 0;
}

ISR(TIMER1_COMPA_vect)
{
    // holder variables for data
    double sample_avg;
    uint8_t i;

    //I would like to put these two lines in main
    FILE lcd_stream = FDEV_SETUP_STREAM(lcd_putchar, 0, _FDEV_SETUP_WRITE);
    FILE uart_stream = FDEV_SETUP_STREAM(uart_putchar, uart_getchar, _FDEV_SETUP_RW);

    //Get the A/D data
    sample_avg=0.0;
    for(i=0;i<100;i++)
    {
        sample_avg+=adc_read()/100.0;
    }

    // write message to LCD
    lcd_home();
    fprintf_P(&lcd_stream,PSTR("ADC: %.0f of 1024    "),sample_avg);

    // write message to seri port
    fprintf_P(&uart_stream,PSTR("ADC: %.0f of 1024    \r\n"),sample_avg);

    PORTC^=(1<<PC4);        // LED change for testing
    return;
}

scootergarret -

Since you are not doing anything in your main code "while(1)" loop, you can get away with putting all your code inside an interrupt service routine (ISR). However, as you write more complex programs with perhaps more than one ISR, you should try to make the ISR's as simple and as fast as possible. In the code above you have 100 calls to "adc_read()" which in turn waits each time to complete the ADC reads. This goes against the "general" rule-of-thumb for interrupts.

What I would do with your code, is to set-up all the FILE streams in main (before the while(1) loop), do the "for loop" and the LCD displays inside the "while(1)" loop. The one and only thing I would do inside the ISR block is set a volatile "ok_to_read_ADC" variable to 1. Then the first line after "while(1){" would be

if (ok_to_read_ADC){
  // do all the loops and calls to adc_read() here
  // display the results
  ok_to_read_ADC = 0;
}

Thanks for the input

The issue I’m trying to get at here is to take (average over 100) A/D measurements at set intervals and out put them on the same interval. For example if I want to know the frequency of a signal I need to know my sampling rate accurately. So in an attempt to shorten the ISR code I would like to pull out those two lines, and make them global volatile. Then I will work on the A/D code. The real problem I’m getting at is serial communication between a C program on my computer and the microprocessor; if I can get them measuring sending and receiving and plotting data in sync. And I still don’t understand why a FILE variable can’t just be initialized.

Declare the file variables in front of your main so they will be global and static. Initialize them in your main and then just use them in other routines.

FILE lcd_stream;
FILE uart_stream;

int main()
{
    lcd_stream = FDEV_SETUP_STREAM(lcd_putchar, 0, _FDEV_SETUP_WRITE);
    uart_stream = FDEV_SETUP_STREAM(uart_putchar, uart_getchar, _FDEV_SETUP_RW);
// now you can just use them in your interrupt routine like you would in your main
}

I think that is what you're looking for although it is not good practice to put all that code in your interrupt routine. I suggest you work to change it now and not learn/reinforce bad habits?

Hello scooter,

It is NEVER a good idea to mix "Foreground and Background Operations".

Getting into that habit is simply a recipe for future unnecessary difficulties. (Some may consider that an "opinion", but bad habits are hard to break as well as often overlooked as the actual source of problems.)

1 - Always break down the processes you intend to implement into "Foreground and Background Operations".

Basically for a Microcontroller without a realtime/preemtive OS that means, for the most part, User Interface Funtions and Interrupt Functions.

While indeed 14 million plus operations per second are fantastic, you really do want to avoid doing as many things a possible that "wastes those cycles" so that you have the most amount of cycles available for actual use.

Interrupt Functions, as we are going with the idea that there is NO realtime/preemtive OS, should ALWAYS be as small, efficient, and concise as is possible to do the function that needs to be done. Anything else that doesn't need to be done at the time of the interrupt should be done elsewhere.

Now to implement these concepts in your code.

1 - ALL of the User Interface should be moved to your "while (1)" loop. (Displaying data, formatting data, processing user input are ALL User Interface Functions.)

2 - Your Interrupt Functions should ONLY do what is absolutely necessary, any additional code should be contained in your Foreground Operations.

3 - ALWAYS try to only process data, and get data, when necessary.

I do like your use of the "Timer Interrupt Function" to process, acquire data.

I use that concept as well.

However, there is something that I see as a problem with how you are implementing that.

The "Timer Interrupt Occurs" and then it may have to wait while it "Reads the ADC Results"...

That can have the affect of wasting unnecessary cycles, and in extreme cases cause your Timer Interrupt to "miss an interrupt". (That will probably not happen in this case, but why even have that possibility.

You do not want to wait, (the statement "while(ADCSRA&(1<<ADSC)){}"), while already in an Interrupt Function, for a result to be available.

A better method is to change your adc_read function so that it "ONLY returns the ADC Result" if there is an ADC Result. If there is no ADC Result then simply return 0, as a "NO DATA STATE", and do not process a 0. (Or you can return at 16 bit 0FFFFH, which I personally prefer since often for My Projects a 0 is common for the ADC while a 0FFFFH is NOT.)

Decide on how you want to go to represent a "NO DATA STATE".

To do this, get rid of the "while" and use an "if" to return the ADC Result if there is an ADC Result when the adc_read function is called, otherwise return a "NO DATA STATE".

4 - Change your Timer1 Interrupt Function so that are is some variable, which WILL NEED TO BE DEFINED AS VOLATILE, to be used to tie-into the "Foreground Operations".

Use an "if" to see if that variable is a "NO DATA STATE" and if it is, in a "NO DATA STATE", and there are ADC Results then store the results into that variable. Otherwise, just discard that particular ADC Result. (Unless you are doing some sort of "time critical ADC Calculations, discarding should not be a problem.)

5 - In your "Foreground Operations" check that variable defined in step 4, to see if there is data, if there is then process that ADC Result and then set that variable back to a "NO DATA STATE" so that the Timer1 Interrupt can put data back there to be processed by your Foreground Operations later.

6 - LASTLY, you really only need to write to the display when you first boot your program, and after that only when the data to be displayed has changed. Otherwise, writing to the display is just wasting alot of cycles.

So some coding should be implemented so that your display is only written when necessary. (In this case when there has been a new sample, or maybe only after every so many samples, or so many Timer1 Interrupts.)

If you get used to dividing your Operations into Foreground and Background and enforce some code discipline in that regard you will find that breaking down your projects into algorithms will be easier and easier to implement in code as well.

Practice does make perfect, or at least allows a progression towards that end... ;)

@ scootergarret -

Ah...I see what you are worried about. If you use the main loop method I suggested, the interrupt might happen while you are trying to output to the LCD or UART, then when the interrupt returns control to the main loop, it would resume code execution at those points and there would be a delay before the next ADC read would happen. As Jim suggested, the same sort of thing could happen with the code as it is written above but it would be less likely.

I think the ultimate solution would be to set up the ADC with an automatic trigger. The trigger can be a pin change, a timer or the completion of a previous ADC conversion (this last one turns it into "free-running" or continuous mode). You would need an ADC completion interrupt to store the results, keep track of the sample count (i.e. 100 in this case), start the next ADC conversion (if sample count < 100), not start a new conversion (if sample count >= 100) and finally set a flag so the main loop can output the results. I don't think you would need a timer ISR but you'd still have to set up a timer if you want to use that as your trigger. Keep in mind, all this happens independently of your main code (except for setting the flag variable), so you should get great timing accuracy. I haven't used the ADC triggers before but it seems like an interesting way to do what you need. Hopefully Noter's solution will work for you in the meantime.