Loops error

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

#define F_CPU 1200000
#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>

int count;

ISR(INT0_vect)
{
    cli();
    _delay_ms(25); //Debounce Switch
    count++;
    if (count >4, count = 0) //Reset count after Mode 4
    sei();
}

int main(void)
{
    MCUCR |= (1<<ISC00);
    GIMSK |= (1<<INT0);
    DDRB = 0x1F;
    PORTB |= (1<<PINB5); //enable Internal Pull Up
    sei();
    int delay1 = 75;
    int delay2 = 500;
    int delay3 = 1000;
    count = 0;
    while(1)
    {
        _delay_ms(500);
        while(count == 0) //Standby Mode
        {
            PORTB = 0x00;
        }
        
        while(count == 1) //Mode 1 Double Alternating Flash Fast
        {
            PORTB = 0x01; //Turn LED 1 on
            _delay_ms(delay1);
            PORTB = 0x00; //Turn LED 1 off
            _delay_ms(delay1);
            PORTB = 0x01; //Turn LED 1 on
            _delay_ms(delay1);
            PORTB = 0x02; //Turn LED 2 on
            _delay_ms(delay1);
            PORTB = 0x00; //Turn LED 2 off
            _delay_ms(delay1);
            PORTB = 0x02; //Turn LED 2 on
            _delay_ms(delay1);
        }
    }
}            
        
        while(count == 2) //Mode 2 Single Alternating Flash Fast
        {
            PORTB = 0x01; //Turn LED 1 on
            _delay_ms(delay2);
            PORTB = 0x02; //Turn LED 2 on
            _delay_ms(delay2);
        }
        
        while(count == 3) //Mode 3 Single Alternating Flash Slow
        {
            PORTB = 0x01; //Turn LED 1 on //Turn LED 1 on
            _delay_ms(delay3);
            PORTB = 0x02; //Turn LED 2 on
            _delay_ms(delay3);
        }
        
        while(count == 4) //Mode 4 Both LEDs Simultaneous
        {
            PORTB = 0x03;
            _delay_ms(delay2);
            PORTB = 0x00;
            _delay_ms(delay2);
        }
        
    }
}

 

I get this error when compiled

any ideas or suggestions I have looked at tutorials and articles and cant see any thing wrong

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

It looks as if you have indented nicely. It would be clearer if you had pasted your code inside code tags.
The braces are balanced UNTIL you get to the while(count ==2) line.
Remove one right brace.
You have several other problems. You must use constant numbers with _delay_ms()

David.

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

david.prentice wrote:
It would be clearer if you had pasted your code inside code tags.

Instructions here: https://www.avrfreaks.net/wiki/ad...

 

The braces are balanced UNTIL you get to the while(count ==2) line.

The OP seems to be using Atmel Studio; IIRC, it highlights the matching brace when the cursor is on a brace - so mis-matched braces should be easy to spot ...

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And, count should be declared as volatile...

:: Morten

 

(yes, I work for Atmel, yes, I do this in my spare time, now stop sending PMs)

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

Remove one right brace.

Two.  Here's the OP's code washed through bcpp:

#define F_CPU 1200000
#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>

int count;

ISR(INT0_vect) {
  cli();
  _delay_ms(25);                        //Debounce Switch
  count++;
  if (count >4, count = 0)              //Reset count after Mode 4
    sei();
}


int main(void) {
  MCUCR |= (1<<ISC00);
  GIMSK |= (1<<INT0);
  DDRB = 0x1F;
  PORTB |= (1<<PINB5);                  //enable Internal Pull Up
  sei();
  int delay1 = 75;
  int delay2 = 500;
  int delay3 = 1000;
  count = 0;
  while(1) {
    _delay_ms(500);
    while(count == 0) {                 //Standby Mode
      PORTB = 0x00;
    }

    while(count == 1) {                 //Mode 1 Double Alternating Flash Fast
      PORTB = 0x01;                     //Turn LED 1 on
      _delay_ms(delay1);
      PORTB = 0x00;                     //Turn LED 1 off
      _delay_ms(delay1);
      PORTB = 0x01;                     //Turn LED 1 on
      _delay_ms(delay1);
      PORTB = 0x02;                     //Turn LED 2 on
      _delay_ms(delay1);
      PORTB = 0x00;                     //Turn LED 2 off
      _delay_ms(delay1);
      PORTB = 0x02;                     //Turn LED 2 on
      _delay_ms(delay1);
    }
  }
}


while(count == 2) {                     //Mode 2 Single Alternating Flash Fast
  PORTB = 0x01;                         //Turn LED 1 on
  _delay_ms(delay2);
  PORTB = 0x02;                         //Turn LED 2 on
  _delay_ms(delay2);
}


while(count == 3) {                     //Mode 3 Single Alternating Flash Slow
  PORTB = 0x01;                         //Turn LED 1 on //Turn LED 1 on
  _delay_ms(delay3);
  PORTB = 0x02;                         //Turn LED 2 on
  _delay_ms(delay3);
}


while(count == 4) {                     //Mode 4 Both LEDs Simultaneous
  PORTB = 0x03;
  _delay_ms(delay2);
  PORTB = 0x00;
  _delay_ms(delay2);
}


}
}

The highlighted braces are superfluous.  Without them:

#define F_CPU 1200000
#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>

int count;

ISR(INT0_vect) {
  cli();
  _delay_ms(25);                        //Debounce Switch
  count++;
  if (count >4, count = 0)              //Reset count after Mode 4
    sei();
}


int main(void) {
  MCUCR |= (1<<ISC00);
  GIMSK |= (1<<INT0);
  DDRB = 0x1F;
  PORTB |= (1<<PINB5);                  //enable Internal Pull Up
  sei();
  int delay1 = 75;
  int delay2 = 500;
  int delay3 = 1000;
  count = 0;
  while(1) {
    _delay_ms(500);
    while(count == 0) {                 //Standby Mode
      PORTB = 0x00;
    }

    while(count == 1) {                 //Mode 1 Double Alternating Flash Fast
      PORTB = 0x01;                     //Turn LED 1 on
      _delay_ms(delay1);
      PORTB = 0x00;                     //Turn LED 1 off
      _delay_ms(delay1);
      PORTB = 0x01;                     //Turn LED 1 on
      _delay_ms(delay1);
      PORTB = 0x02;                     //Turn LED 2 on
      _delay_ms(delay1);
      PORTB = 0x00;                     //Turn LED 2 off
      _delay_ms(delay1);
      PORTB = 0x02;                     //Turn LED 2 on
      _delay_ms(delay1);
    }

    while(count == 2) {                 //Mode 2 Single Alternating Flash Fast
      PORTB = 0x01;                     //Turn LED 1 on
      _delay_ms(delay2);
      PORTB = 0x02;                     //Turn LED 2 on
      _delay_ms(delay2);
    }

    while(count == 3) {                 //Mode 3 Single Alternating Flash Slow
      PORTB = 0x01;                     //Turn LED 1 on //Turn LED 1 on
      _delay_ms(delay3);
      PORTB = 0x02;                     //Turn LED 2 on
      _delay_ms(delay3);
    }

    while(count == 4) {                 //Mode 4 Both LEDs Simultaneous
      PORTB = 0x03;
      _delay_ms(delay2);
      PORTB = 0x00;
      _delay_ms(delay2);
    }

  }
}

 

As mentioned, there are other issues with your code.  Some have been mentioned, some have not.  Come back when you run into them.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

sei() within an ISR() often means "slow suicide" and invariably represents poor program design. 

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

clawson wrote:
sei() within an ISR() often means "slow suicide" and invariably represents poor program design.
The OP has (many) bigger fish to fry:

  if (count >4, count = 0)              //Reset count after Mode 4
    sei();

 

count will always be  0.  The sei() will never run.

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

One should never use delay in an ISR(),  best to keep ISR's as fast as possible!

 

ISR(INT0_vect) {
  cli();
  _delay_ms(25);                        //Debounce Switch
  count++;
  if (count >4, count = 0)              //Reset count after Mode 4
    sei();
}

Click Link: Get Free Stock: Retire early! PM for strategy

share.robinhood.com/jamesc3274
get $5 free gold/silver https://www.onegold.com/join/713...

 

 

 

 

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

clawson wrote:
sei() within an ISR() often means "slow suicide" and invariably represents poor program design.

ki0bk wrote:
One should never use delay in an ISR()...

Y'all are into the superlatives today.  Certainly there are situations where either of those techniques might be used, but indeed unlikely in this type of program.

 

So let's first point OP at the datasheet on interrupt handling for the basis behind "no cli or sei in ISRs":

When an interrupt occurs, the Global Interrupt Enable I-bit is cleared and all interrupts are disabled. The user software can write logic one to the I-bit to enable nested interrupts. All enabled interrupts can then interrupt the current interrupt routine. The I-bit is automatically set when a Return from Interrupt instruction – RETI – is executed.

So there is no need for the cli, and nested interrupts aren't desired and the compiler will get the RETI in there.

 

Now on to my main reason for posting:

 

_delay_ms(25);                        //Debounce Switch

Debounce, eh?  Well, kindof/maybe.  I'd say "not".   But if the switch bounces then the interrupt flag will again be set and the interrupt re-entered -- no matter if you delay 25us or 25 seconds.  (and the sei means that the interrupts would be nested...)

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Cheers guys, Had a read through this and the suggestions have all been a huge help,