New to Atmel, looking for help analyzing why this program will not work properly

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

Hello, I am very new to Atmel and microcontrollers in general so please forgive me if this is a dumb question. I am currently working on a project that is introducing interrupts . The main function of the program is supposed to rotate a single lighted LED through a series of four LED's. I have that part working properly. There are also two switches hooked up in this circuit that are supposed to activate the interrupts. The switches/interrupts are supposed to act as described below:

 

Use the interrupt functions of INT0 and INT1 such that the switches attached to PD2 and PD3 cause the following action when the switches are cycled (capture each switch signal’s falling edge to trigger the interrupt):

A press of switch 0 (on INT0) will cause the LED pattern to change to a count up by 1 in binary, for one complete cycle of the four LEDs, with PC0 being the least significant bit, but start from the binary pattern that was being displayed in the main rotating sequence at the time of the interrupt. In this sequence, any given LED pattern must be on for 250 msec before transitioning to the next pattern. After one cycle is complete, return to the main rotating sequence at the point where it was interrupted. Do not allow interrupts of this ISR, or immediate repeat of this routine if INT0 is retriggered during the ISR. However, if INT1 is triggered during this ISR, then it must execute immediately after this ISR is done.  

A press of switch 1 (on INT1) will cause the LEDs to all be turned on and stay on for two seconds. After completion of this ISR, resume the normal rotating LED sequence of main(), picking up where it left off. Do not allow interrupts of this ISR, or immediate repeat of this routine if INT1 is retriggered during the ISR. However, if INT0 is triggered during this ISR, then it must execute immediately after this ISR is done (keep in mind that the INT0 sequence must begin with the pattern being displayed in the main rotating sequence when it was originally interrupted).

 

I have been able to get the patterns of the Interrupts to activate correctly, however I am having two issues that I cannot figure out why they are happening. The first is that whenever the interrupt pattern is done completing, it does not go back into looping the main rotating sequence. Whenever a interrupt is finished, it correctly goes back to the light that it left off on, but it just stays at that light and nothing happens after that. My second issue is that when the switch/button is pressed, the interrupt pattern will sometimes immediately repeat. I tried to find a way to debounce this, but for whatever reason it won't consistently work. If anyone could  give any ideas of why this isn't working properly, I would be very grateful. The code that I have  written so far is below. Everything below the ISR(INT1_vect) was given to me by my professor for the delay functions, so I did not write that. I am using the ATmega328P microcontroller as well.

 

#include <avr/io.h>
#include <avr/interrupt.h>
void wait(volatile int multiple, volatile char time_choice);
void delay_T_msec_timer0(char choice);

int main(void)
{
    DDRC = 0xFF;  // PORTC set to all outputs
    PORTC = 0xFF; // PORTC All off
    DDRD = DDRD & 0b11110011;// defines PORTD2 and PORTD3 as input for use as interrupts INT0 and INT1
    PORTD = DDRD | 0b00001100; //enables PORTD2 and PORTD3 pull-up resistors
    EICRA = 0b00001110; //sets interrupt type as falling
    EIMSK = 0b00000011; // Enable INT0 and INT1
    sei();//Enable Global Interrupt
    int state = 0; // Keep track of what LED is on
    while (1) 
    {
        if(state == 3)// If on the 4th LED, jump back to first
        {
            state = 0;
        }
        else
        {
            state++;
        }

        switch(state)
        {
            case 0:    PORTC = 0b11111110;    //turn on PORTC0
            break;
            case 1:    PORTC = 0b11111101; //turn off PortC0, turns on PORTC1
            break;
            case 2:    PORTC = 0b11111011; //turns off PORTC1, turns on PORTC2
            break;
            case 3:    PORTC = 0b11110111;//turns off PORTC2, turns on PORTC3
            break;
        }
    
        wait(1000, 2);
    }
    return 0;
}

ISR(INT0_vect)
{
    wait(50, 2);
    unsigned char temp;
    temp = PORTC;
    int i;

    for(i=0; i < 16; i++)
    {
        if (PORTC == 0b11110000)
        {
            PORTC = 0b11111111;
        }
        else
        {
            PORTC--;
        }
        wait(250,2);
    }
    PORTC = temp;
}

ISR(INT1_vect)
{
    wait(50, 2);
    unsigned char temp;
    temp = PORTC;

    PORTC = 0b11110000;
    wait(2000,2);

    PORTC = temp;
}

void wait(volatile int multiple, volatile char time_choice){
    while(multiple > 0)
    {
        delay_T_msec_timer0(time_choice);
        multiple--;
    }
}// wait()

void delay_T_msec_timer0(char choice)
{
    TCCR0A = 0x00;
    TCNT0 = 5;

    switch (choice)
    {
        case 1:
        TCCR0B = 1<<CS01;
        break;
        case 2:
        TCCR0B = 1<<CS01 | 1<<CS00;
        break;
        case 3:
        TCCR0B = 1<<CS02;
        break;
        case 4:
        TCCR0B = 1<<CS02 | 1<<CS00;
        break;
        default:
        TCCR0B = 1<<CS00;
        break;
    }
    while ((TIFR0 & (0x1<<TOV0)) == 0);

    TCCR0B = 0x00;
    TIFR0 = 0x1<<TOV0;
}
 

 

Last Edited: Mon. Jun 3, 2019 - 02:08 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Make sure you have AVcc connected to Vcc.
You might want to set up the timers in a one time unit function rather than unit them every time you call the delay. What is the speed of the AVR? Are you using a crystal or the internal oscillator?

Jim

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

Last Edited: Mon. Jun 3, 2019 - 02:07 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Jim,  I am using a  16MHz crystal oscillator. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
ISR(INT0_vect)
{
    wait(50, 2);

The alarm bells went off as soon as I read that. You never want to delay for anything more than an opcode or two in an ISR. I also read:

A press of switch 0 (on INT0) 

Again this is terrible design. You would never usually choose to put a button on an external interrupt. The only reason usually is when INT0 is being used to awaken the device from SLEEP but as soon as it is awake the button scanning should be done in a timer interrupt. To understand why read: http://www.ganssle.com/debouncin...

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

I re-wrote part of the delay/timer function, but that External interrupt function is the only one we have discussed in calss so far, so I believe that he wants us to use that. Here is what the code looks like now.

 

#include <avr/io.h>
#include <avr/interrupt.h>
void wait(int msec);
void delayNms_timer0();

int main(void)
{
    DDRC = 0xFF; //PORTC set to all outputs
    PORTC = 0xFF; //PORTC All off
    DDRD = DDRD & 0b11110011; //defines PORTD2 and PORTD3 as input for use as interrupts INT0 and INT1
    PORTD = DDRD | 0b00001100; // enables PORTD2 and PORTD3 pull-up resistors
    EICRA = 0b00001110; //sets interrupt type as falling
    EIMSK = 0b00000011; //Enable INT0 and INT1
    sei(); //Enables global interrupt
    int state = 0; //Keep track of what LED is on
    while (1) 
    {
        if(state ==3)// If on the 4th LED, jump back to first
        {
            state = 0;
        }
        else
        {
            state++;
        }

        switch(state)
        {
            case 0: PORTC = 0b11111110; // turn on PORTC0
            break;
            case 1: PORTC = 0b11111101; //turn off PORTC0, turn on PORTC1
            break;
            case 2: PORTC = 0b11111011; //turn off PORTC1, turn on PORTC2
            break;
            case 3: PORTC = 0b11110111; //turn off PORTC2, turn on PORTC3
            break;
        }

        wait(1000);
    }
    return 0;
}

ISR(INT0_vect)
{
    unsigned char temp;
    temp = PORTC;
    int i;

    for(i=0; i < 16; i++)
    {
        if (PORTC == 0b11110000)
        {
            PORTC = 0b11111111;
        }
        else
        {
            PORTC--;
        }
        wait(250);
    }
    PORTC = temp;
}

ISR(INT1_vect)
{
    unsigned char temp;
    temp = PORTC;

    PORTC = 0b11110000;
    wait(2000);

    PORTC = temp;
}

void wait(int multiple)
{
    while (multiple > 0)
    {
        delayNms_timer0();
        multiple = multiple -1;
    }
}

void delayNms_timer0()
{
    TCCR0A = 0x00;
    TCNT0 = 5;
    TCCR0B = 1<<CS01 | 1<<CS00;
    while ((TIFR0 & (0x1<<TOV0)) == 0);
        TCCR0B = 0x00;
        TIFR0 = 0x1 << TOV0;
}

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

prk1215 wrote:
My second issue is that when the switch/button is pressed, the interrupt pattern will sometimes immediately repeat. I tried to find a way to debounce this, but for whatever reason it won't consistently work.

 

You can clear pending interrupts by writing 1 to the corresponding bit in the EIFR register. So, if a new interrupt occurs during the ISR, you can prevent reentry by writing these flags before exiting the interrupt.

Note that every interrupt source in the AVR usually will have a corresponding mask register to enable/disable the interrupt, and a flag register to allow polling or clearing pending interrupts.

Last Edited: Mon. Jun 3, 2019 - 02:07 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you, that solution fixed the repeating pattern.

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

Yes, however, as others have stressed, you shouldn't have such delays inside an ISR, it's bad practice.

The correct way is for the ISR to set some global variable to signal the main code sequence. It's the main program that should have all the slow code.

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

Could you give an example of how to do this? Sorry I am very new to this. I think I have an idea of what you mean, but Im not 100%

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

I think this is the scenario of your issue:

1. wait() in your main loop is running

2. An interrupt occurs.

3. The interrupt procedure calls wait()

4. The wait() called by the interrupt procedure exits with timer0 stopped

5. Code execution returns to wait() in main loop, but the timer is stopped and this time the loop in delayNms_timer0() runs infinitely.

The simplest solution is to make two procedures for delays, one using timer0 and called in the main loop and the second one using timer1 and called in interrupts.

Last Edited: Tue. Jun 4, 2019 - 06:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That makes sense, and was my original thought to use two different timers. However, we haven’t discussed how to set up or use the timer1 yet. Based off of what we have learned so far, my teacher said we should save information at the beginning of each interrupt service routine that may change inside the routine(such as timer registers, counting variables, etc.) and reload that saved information at the end of the interrupt routine and restart the timer for the main loop. I just don’t know how to do that

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

Well, removing all the delays is more tricky, timers are involved, etc. but you can remove at least the LED blink sequence code from the ISRs.

Here is some pseudo-code:

 

uint8_t isr_flag = 0;

ISR() {

    // wait 1-5ms for debouncing

    // set isr_flag

    // clear the hardware interrupt flag to prevent ISR reentry due to bouncing

}

 

main() {

    while(1) {

        // call alternate LED sequence if flag for ISR is set, then clear ISR flag

        ...

        // normal LED sequence

        ...

    }

}

 

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

Your interrupt procedures doesn't save/restore timer registers. When you add code for this, the code will be still far from being correctly designed (never use delays in interrupts), but I think it will work.