Prblm with printf() and global variables, [solved IE no ISR]

Go To Last Post
9 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello guys,
I'm currently working in system that simulates a fire sensor system but I can't get it to work just fine. I'm relatively new to the AVR MCU and I would really appreciate any advice,

Here's some background of the system, but feel free to skip it,

What the system should do:
- Read a voltage from a temp sensor using the ADC converter every 2 seconds.
- Based on the reading it should turn on X number of LED's, the bigger the reading more LED's should turn on.
- Send a serial message to inform the PC of the current temperature reading
- Send an Alarm message if the temperature is above certain value
- If the user sends a value N (from 1 to 9), the system should report the previous N values

Implementation:
I used the 16-bit timer to trigger the overflow ISR every second, this routine starts the ADC by settng the ADSC flag. The subroutine also sets a flag every other second which enables a msg to be sent to the user in main().

The ADC ISR stores the value in an array and sets a flag so that the main() can analyze the value and turn on the required number of LEDs

A USART_RX routine enables a flag so that the received char can be analized in main().

main() will just loop waiting for the flags to be enabled and act appropietly.

Problem:
I've set up the ISR for the ADC like this,

// Global Variables
unsigned volatile int flag_adc; 	// Flag to identify ADC conversion
unsigned volatile int adc_data;			// Stores ADC conversion
unsigned volatile int ADC_store[10];	// Stores 10 ADC conversions
unsigned volatile int ii;

ISR(ADC_vect){
	adc_data = ADCW;				// Copy reading to memory
	ADC_store[ii] = adc_data;		// Store the reading in array

	ii++;							// Prepare array for next reading

	if(ii == 10){					// If max size 10, restart filling.
		ii = 0;
	}

	if(read < 10){					
		read++;
	}

	flag_adc = 1;					// Enable flag to service ADC in main routine
}

Everytime the ISR runs it should increase the value of ii so that the new data will then be stored in the next array element.

In my main routine,

int main(void){
	unsigned int adc_data1;
	init_hardware();
	sei();					// Enable global interrupts

	while (1){
//SERVICE TIMER
		if(flag_timer){
			flag_timer = 0;
			cli();
			results = convert*adc_data;
			sei();
// This line was commented for debugging purposes.
//			printf("\rSensor Current Temperature: @d degrees\n", (int) results);
			printf("\rRead @d\t@d\n", ii, read);
		}
   }
}

The flag_timer is set by the Timer Overflow ISR about 1 second after the ADC. So according to this the printf should display:

Read 1   1
Read 2   2
Read 3   3

but instead it just displays

Read 1   1
Read 1   1
Read 1   1

I'm not sure why the variable values are not increased, something I noticed is that if I remove the printf() line the code works just fine, but I can't figure out why the function is screwing things up.
Here's all code related to the printf() function:

static int uart_putchar(char c, FILE *stream);
static int uart_getchar(FILE *stream);

static FILE mystdout = FDEV_SETUP_STREAM(uart_putchar, NULL, _FDEV_SETUP_WRITE);
static FILE mystdin  = FDEV_SETUP_STREAM(NULL, uart_getchar, _FDEV_SETUP_READ);

static int uart_putchar(char c, FILE *stream){
	if (c == '\n'){
		uart_putchar('\r', stream);
	}
	loop_until_bit_is_set(UCSR0A, UDRE0);
	UDR0 = c;
	return 0;
}

int uart_getchar(FILE *stream){
	loop_until_bit_is_set(UCSR0A, RXC0);
	return UDR0;
}
//In main...
	stdout = &mystdout;		// Printf and scanf functions
	stdin  = &mystdin;

Thanks in advance for any advice, Here's the code I'm using:

// Simulation of a remotely located fire sensor

#include 
#include 
#include 
#include 

// Program definitions
#define   convert   0.053763         // Number to change scale from 1023 to 55


// Declare global variables
unsigned volatile int flag_rcv;    // Flag to identify USART received character
unsigned volatile int flag_adc;    // Flag to identify ADC conversion
unsigned volatile int flag_timer;    // Flag to identify Timer Overflow

unsigned volatile int adc_data;         // Stores ADC conversion
unsigned volatile int ADC_store[10];   // Stores 10 ADC conversions
unsigned volatile int ii;

unsigned volatile char ch;          // Stores a character from USART
unsigned volatile int read;



// printf() and scanf() in WINAVR
static int uart_putchar(char c, FILE *stream);
static int uart_getchar(FILE *stream);

static FILE mystdout = FDEV_SETUP_STREAM(uart_putchar, NULL, _FDEV_SETUP_WRITE);
static FILE mystdin  = FDEV_SETUP_STREAM(NULL, uart_getchar, _FDEV_SETUP_READ);

static int uart_putchar(char c, FILE *stream){
   if (c == '\n'){
      uart_putchar('\r', stream);
   }
   loop_until_bit_is_set(UCSR0A, UDRE0);
   UDR0 = c;
   return 0;
}

int uart_getchar(FILE *stream){
   loop_until_bit_is_set(UCSR0A, RXC0);
   return UDR0;
}

// Interrupt ISR
// This interrupt occurs whenever the USART completes receiving a character
ISR(USART0_RX_vect){
   ch = UDR0;       // Gets character from USART
   flag_rcv = 1;    // Enable flag to service USART in main routine
}

/*This interrupt occurs when the ADC is done*/
ISR(ADC_vect){
   adc_data = ADCW;            // Copy reading to memory
   ADC_store[ii] = adc_data;      // Store the reading in array

   ii++;                     // Prepare array for next reading

   if(ii == 10){               // If max size 10, restart filling.
      ii = 0;
   }

   if(read < 10){               
      read++;
   }

   flag_adc = 1;               // Enable flag to service ADC in main routine
}

/*This interrupt occurs when timer reaches max of 65,535*/
ISR(TIMER1_OVF_vect){
   static unsigned volatile int time = 0;

   TCNT1H = 0x85;               // 1 second overflow
   TCNT1L = 0xED;
   time++;

   if(time == 1){               // Start ADC
      ADCSRA |= 1 << ADSC;
   }

   if(time == 2){
      flag_timer = 1;            // Enable flag to service timer in main routine
      time = 0;
   }
}

void init_hardware(void){
// USART initialization
// Communication Parameters: 8 Data, 1 Stop, No Parity
// USART Receiver: On
// USART Transmitter: On
// USART Mode: Asynchronous   
   UCSR0A = (1 << UDRE0);
   UCSR0B = (1 << RXCIE0) | (1 << TXCIE0) | (1 << RXEN0) | (1 << TXEN0);
   UCSR0C = (1 << UMSEL01) | (1 << UCSZ01) | (1 << UCSZ00);
   UBRR0H = 0x00;      // Set Baud Rate to 2400
   UBRR0L = 0xCF;

//PortB configuration
   DDRB = 0xFF;
   PORTB = 0xFF;

//Timer initialization
   TCCR1A = 0x00;
   TCCR1B = 0x04;         // Clock Division 256
    TCNT1H = 0x85;         // 1sec delay
   TCNT1L = 0xED;
   TIMSK1 = 0x01;         // Enable overflow Interrupt

//ADC setup using the registers
   ADMUX  = 0x00; // AREF, Right ajust result, PortA0 MUX input
   ADCSRA = 0x8F; // 1000 1111 Enable ADC, Disable Autotrigger, Enable Interrupts, Clock Div 128
   ADCSRB = 0x00; // No effect since Autotrigger is disabled.

}

int main(void){
    volatile unsigned int j;
   volatile unsigned int k;
   volatile unsigned int adc_data1;
   volatile unsigned  int temp;
   volatile float results;

//This mapping is used to connect stdio variables to our global variables
//for printf and scanf
   stdout = &mystdout;      // Printf and scanf functions
   stdin  = &mystdin;
   init_hardware();
   sei();               // Enable global interrupts

   while (1){

//SERVICE ADC
      if(flag_adc){
         flag_adc = 0;

         cli();               // Atomic reading
         adc_data1 = adc_data;
         sei();

         // LED bar display, the bigger the reading more LEDs on
         if (adc_data1 == 1023)
            PORTB = 0x00;
         else if ((adc_data1 > (7*1023)/8) && (adc_data1 <= 1023))
            PORTB = 0x80;
         else if ((adc_data1 > (6*1023)/8) && (adc_data1 <= (7*1023)/8))
            PORTB = 0xC0;
         else if ((adc_data1 > (5*1023)/8) && (adc_data1 <= (6*1023)/8))
            PORTB = 0xE0;
         else if ((adc_data1 > (4*1023)/8) && (adc_data1 <= (5*1023)/8))
            PORTB = 0xF0;
         else if ((adc_data1 > (3*1023)/8) && (adc_data1 <= (4*1023)/8))
            PORTB = 0xF8;
         else if ((adc_data1 > (2*1023)/8) && (adc_data1 <= (3*1023)/8))
            PORTB = 0xFC;
         else if ((adc_data1 > (1*1023)/8) && (adc_data1 <= (2*1023)/8))
            PORTB = 0xFE;
         else if ((adc_data1 > (0)) && (adc_data1 <= (1*1023)/8))
            PORTB = 0xFF;

         results = convert*adc_data1;         // Change scale from 0-1023 to 0-55

         if((results) > 50){
            printf("\rALERT\n \rSensor Current Temperature: @d degrees\n", (int) results);
            printf("\rALERT: UNUSUALLY HIGH TEMPERATURE DETECTED\n");
         }
      }
//SERVICE TIMER
      if(flag_timer){
         flag_timer = 0;
         cli();
         results = convert*adc_data;
         sei();
   // This line was commented for debugging purposes.
//         printf("\rSensor Current Temperature: @d degrees\n", (int) results);
         printf("\rRead @d\t@d\n", ii, read);
      }

//SERVICE UART
      if(flag_rcv){
         flag_rcv = 0;
         cli();
         k = ii;
         
         if(ch >= '1' && ch <= '9'){            // Only 1-9 valid input
            temp = ((int) ch) - 48;            // ASCII to decimal conversion

            if(temp > read){         // If requested number of readings exceeds those made
   // This line was commented for debugging purposes.
//               temp = read;
            }
            // Display the requested number of sensor readings
            for(j = 1; j <= temp; j++){
               results = convert * ADC_store[k];
               printf("\rSensor Reading: t - @d Temperature:@d degrees\n", j, (int) results);
            
               if(k == 0){
                  k = 10;
               }

               k--;
            }

         }
         sei();
      }
   }
} 

[note to Bob - I know the thread title mentions WinAVR and as such this should move to GCC Forum but this looks like a generic question to me - nothing specific about WinAVR/GCC - cliff]

Last Edited: Sat. Mar 19, 2011 - 06:42 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But where is the rest of the code? You haven't shown how the ADC or the timer are initialized, nor have you shown the declaration of 'read'.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What do you mean by this:

A USART_RX routine enables a flag so that the received char can be analized in main().

Does this mean you've enabled receive interrupts for the usart? If so, then you'll have problems since you poll the usart for rx characters.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
What do you mean by this:

A USART_RX routine enables a flag so that the received char can be analized in main().

Does this mean you've enabled receive interrupts for the usart? If so, then you'll have problems since you poll the usart for rx characters.

The ISR set a flag, the flag is then polled by main and acts accordingly. Thanks

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here's the code
I apologize for the formating, it is way more understandable in AVR Studio,

- % have benn change to @

// Simulation of a remotely located fire sensor

#include 
#include 
#include 
#include 

// Program definitions
#define	convert	0.053763			// Number to change scale from 1023 to 55


// Declare global variables
unsigned volatile int flag_rcv; 	// Flag to identify USART received character
unsigned volatile int flag_adc; 	// Flag to identify ADC conversion
unsigned volatile int flag_timer; 	// Flag to identify Timer Overflow

unsigned volatile int adc_data;			// Stores ADC conversion
unsigned volatile int ADC_store[10];	// Stores 10 ADC conversions
unsigned volatile int ii;

unsigned volatile char ch; 			// Stores a character from USART
unsigned volatile int read;



// printf() and scanf() in WINAVR
static int uart_putchar(char c, FILE *stream);
static int uart_getchar(FILE *stream);

static FILE mystdout = FDEV_SETUP_STREAM(uart_putchar, NULL, _FDEV_SETUP_WRITE);
static FILE mystdin  = FDEV_SETUP_STREAM(NULL, uart_getchar, _FDEV_SETUP_READ);

static int uart_putchar(char c, FILE *stream){
	if (c == '\n'){
		uart_putchar('\r', stream);
	}
	loop_until_bit_is_set(UCSR0A, UDRE0);
	UDR0 = c;
	return 0;
}

int uart_getchar(FILE *stream){
	loop_until_bit_is_set(UCSR0A, RXC0);
	return UDR0;
}

// Interrupt ISR
// This interrupt occurs whenever the USART completes receiving a character
ISR(USART0_RX_vect){
	ch = UDR0; 		// Gets character from USART
	flag_rcv = 1; 	// Enable flag to service USART in main routine
}

/*This interrupt occurs when the ADC is done*/
ISR(ADC_vect){
	adc_data = ADCW;				// Copy reading to memory
	ADC_store[ii] = adc_data;		// Store the reading in array

	ii++;							// Prepare array for next reading

	if(ii == 10){					// If max size 10, restart filling.
		ii = 0;
	}

	if(read < 10){					
		read++;
	}

	flag_adc = 1;					// Enable flag to service ADC in main routine
}

/*This interrupt occurs when timer reaches max of 65,535*/
ISR(TIMER1_OVF_vect){
	static unsigned volatile int time = 0;

	TCNT1H = 0x85;					// 1 second overflow
	TCNT1L = 0xED;
	time++;

	if(time == 1){					// Start ADC
		ADCSRA |= 1 << ADSC;
	}

	if(time == 2){
		flag_timer = 1;				// Enable flag to service timer in main routine
		time = 0;
	}
}

void init_hardware(void){
// USART initialization
// Communication Parameters: 8 Data, 1 Stop, No Parity
// USART Receiver: On
// USART Transmitter: On
// USART Mode: Asynchronous	
	UCSR0A = (1 << UDRE0);
	UCSR0B = (1 << RXCIE0) | (1 << TXCIE0) | (1 << RXEN0) | (1 << TXEN0);
	UCSR0C = (1 << UMSEL01) | (1 << UCSZ01) | (1 << UCSZ00); 
	UBRR0H = 0x00;		// Set Baud Rate to 2400
	UBRR0L = 0xCF;

//PortB configuration
	DDRB = 0xFF;
	PORTB = 0xFF;

//Timer initialization
	TCCR1A = 0x00;
	TCCR1B = 0x04;			// Clock Division 256
    TCNT1H = 0x85;			// 1sec delay
	TCNT1L = 0xED;
	TIMSK1 = 0x01;			// Enable overflow Interrupt

//ADC setup using the registers
	ADMUX  = 0x00; // AREF, Right ajust result, PortA0 MUX input
	ADCSRA = 0x8F; // 1000 1111 Enable ADC, Disable Autotrigger, Enable Interrupts, Clock Div 128
	ADCSRB = 0x00; // No effect since Autotrigger is disabled.

}

int main(void){
 	volatile unsigned int j;
	volatile unsigned int k;
	volatile unsigned int adc_data1;
	volatile unsigned  int temp;
	volatile float results;

//This mapping is used to connect stdio variables to our global variables
//for printf and scanf
	stdout = &mystdout;		// Printf and scanf functions
	stdin  = &mystdin;
	init_hardware();
	sei();					// Enable global interrupts

	while (1){

//SERVICE ADC
		if(flag_adc){
			flag_adc = 0;

			cli();					// Atomic reading
			adc_data1 = adc_data;
			sei();

			// LED bar display, the bigger the reading more LEDs on
			if (adc_data1 == 1023)
				PORTB = 0x00;
			else if ((adc_data1 > (7*1023)/8) && (adc_data1 <= 1023))
				PORTB = 0x80;
			else if ((adc_data1 > (6*1023)/8) && (adc_data1 <= (7*1023)/8))
				PORTB = 0xC0;
			else if ((adc_data1 > (5*1023)/8) && (adc_data1 <= (6*1023)/8))
				PORTB = 0xE0;
			else if ((adc_data1 > (4*1023)/8) && (adc_data1 <= (5*1023)/8))
				PORTB = 0xF0;
			else if ((adc_data1 > (3*1023)/8) && (adc_data1 <= (4*1023)/8))
				PORTB = 0xF8;
			else if ((adc_data1 > (2*1023)/8) && (adc_data1 <= (3*1023)/8))
				PORTB = 0xFC;
			else if ((adc_data1 > (1*1023)/8) && (adc_data1 <= (2*1023)/8))
				PORTB = 0xFE;
			else if ((adc_data1 > (0)) && (adc_data1 <= (1*1023)/8))
				PORTB = 0xFF;

			results = convert*adc_data1;			// Change scale from 0-1023 to 0-55

			if((results) > 50){
				printf("\rALERT\n \rSensor Current Temperature: @d degrees\n", (int) results);
				printf("\rALERT: UNUSUALLY HIGH TEMPERATURE DETECTED\n");
			}
		}
//SERVICE TIMER
		if(flag_timer){
			flag_timer = 0;
			cli();
			results = convert*adc_data;
			sei();
	// This line was commented for debugging purposes.
//			printf("\rSensor Current Temperature: @d degrees\n", (int) results);
			printf("\rRead @d\t@d\n", ii, read);
		}

//SERVICE UART
		if(flag_rcv){
			flag_rcv = 0;
			cli();
			k = ii;
			
			if(ch >= '1' && ch <= '9'){				// Only 1-9 valid input
				temp = ((int) ch) - 48;				// ASCII to decimal conversion

				if(temp > read){			// If requested number of readings exceeds those made
	// This line was commented for debugging purposes.
//					temp = read;
				}
				// Display the requested number of sensor readings
				for(j = 1; j <= temp; j++){
					results = convert * ADC_store[k];
					printf("\rSensor Reading: t - @d Temperature:@d degrees\n", j, (int) results);
				
					if(k == 0){
						k = 10;
					}

					k--;
				}

			}
			sei();
		}
	}
} 
  
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,
ok I finally figured out the problem, had to learn a little of ASM and some debugging techniques (lots of learning here)

The program was actully reseting after every printf, the reason being because the the TX ISR was not defined (god thanks for these shower enlightments)

To fix the code change the following line:

   UCSR0B = (1 << RXCIE0) | (1 << TXCIE0) | (1 << RXEN0) | (1 << TXEN0); 

To this one:

   UCSR0B = (1 << RXCIE0) | (1 << RXEN0) | (1 << TXEN0); 

or by adding a dummy ISR for the TX, which there's no point of doing....

Thanks guys

Note/question to mods: I don't know if thie post should be closed or if there is some sort of procedure I have to do now...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Note/question to mods: I don't know if thie post should be closed or if there is some sort of procedure I have to do now...

We encourage people to go back to the first post, edit it and change the Subject: to include "[solved]". When I do this on threads (as I will with this one) I try to include in a few words a short summary of what the issue was.

Moderator.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And send everyone free tshirts, you forgot that part. 8)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dksmall wrote:
And send everyone free tshirts, you forgot that part. 8)

dang, this will get expensive =D,

this will get offtopic but, it would be cool to have an AVR freak t-shirt...