Interrupts on M48

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

Hi Freaks,

I am trying to use a simple switch on PB0 interrupt pin on my M48. When the switch is pressed, I have a speaker connected to PC0 which should beep. I have already proven the beeping code and it works. However, my interrupts are not working. Here is my code:

#include 
#include 
#include 
#include 
#include 

 //switch on PB0 PCINT0 interrupt
//10K pull up from PB0 to supply
//0.1uF cap to ground for switch debouncing
//Disable CKOUT for PB0

volatile unsigned char switch_on = 0;
volatile unsigned char temp;
volatile unsigned char temp1;

void beep()
   {

     for(int i=0;i<500;i++)
      {
         PORTC = 0x01;
        _delay_ms(0.5);
         PORTC = 0x00;
        _delay_ms(0.5);
      }

   }

void delay() //delay for beeping

   {
     _delay_ms(500);
   }


void ext_interrupt_init()
    {
     PCMSK0 |= (1 << PCINT0);
     PCICR |= (1 << PCIE0);//enable interrupt flag
     sei();
    }

ISR(PCINT0_vect)
   {
  
    unsigned char switch_on = 0;
    //use debouncing
    temp = PINB & (0x01); 
    _delay_ms(100);  
    temp1 = PINB & (0x01);
    //set switch_on flag here
    if (temp == temp1)
    switch_on = 1;
    }

int main(void)

   {
         DDRB = 0xFF;
	 DDRC = 0xFF;

         ext_interrupt_init();
          for(;;)
            {
              if (switch_on)
                {  

                  for(int j=0; j<10; j++)

                    {
                       delay();
                       beep();
                       switch_on = 0;
                     }

                 }
             }    
     } 

I am using a very simple debouncing logic.Other than that, what could be wrong in the above code?
(I am working on making a more efficient timer based beep, but for now please ignore the beeping logic, I have another post for that)
Any help appreciated.

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

Do you realise that you get 2 interupts for each key press, one on closing and one on releasing? Also it is not wise to spend 200ms in an interrupt, especcially seeing that you have hadware debounce. You should add something like 10k-100K in series with the input pin and put the cap at the input pin. May want to have a look here: http://www.members.optusnet.com....

Do you have a DW debugger? Have you tested your code in the simulator at least?

edit you also have 2 switch_on so the ISR may be using it's own copy rather the the one you have declare outside the ISR. (C guru's opinion required here :? )

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

js wrote:
you also have 2 switch_on so the ISR may be using it's own copy rather the the one you have declare outside the ISR. (C guru's opinion required here :? )
Yes, the ISR definitely does not change the global variable switch_on, only its local variable with the same name.

PS:
"copy" is wrong. The local switch_on is a entirely separate variable.

Stefan Ernst

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

Also I just realized, portB has to be configured as input to interrupt on PB0 to be recognized. Will try with the changes and see what happens.

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

How can I connect the switch to be active high (if I want normally OFF and HIGH (5V)) when I press it? Will connecting the 10K to GND work and the switch between the pin and VCC like the RESET pin?

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

Ok it is working. I changed the code to the following:

#include 
#include 
#include 
#include 
#include 

 //switch on PB0 PCINT0 interrupt
//10K pull up from PB0 to supply
//0.1uF cap to ground for switch debouncing
//Disable CKOUT for PB0

volatile unsigned char switch_on = 0;
volatile unsigned char temp;


void beep()
   {

     for(int i=0;i<500;i++)
      {
         PORTC = 0x01;
        _delay_ms(0.5);
         PORTC = 0x00;
        _delay_ms(0.5);
      }

   }

void delay() //delay for beeping

   {
     _delay_ms(500);
   }


void ext_interrupt_init()
    {
     PCMSK0 |= (1 << PCINT0);
     PCICR |= (1 << PCIE0);//enable interrupt flag
     sei();
    }

ISR(PCINT0_vect)
   {
 
    //use debouncing
    temp = PINB & (0x01);
    _delay_ms(100); 
    temp1 = PINB & (0x01);
    //set switch_on flag here
    if (temp)
    switch_on = 1;
    }

int main(void)

   {
         DDRB = 0xFF;
    DDRC = 0xFF;

         ext_interrupt_init();
          for(;;)
            {
              if (switch_on)
                { 

                  for(int j=0; j<10; j++)

                    {
                       delay();
                       beep();
                       switch_on = 0;
                     }

                 }
             }   
     } 

By default with the switch off the resistor is connected to VCC. Pressing the switch connects the PB0 to GND. I would like to reverse this logic (by default I want to connect PB0 to GND and high when switch is pressed). Is this possible?

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

Quote:
Is this possible?
Yes by reversing your input components and the logic in your code.
Quote:
the switch between the pin and VCC like the RESET pin?
What do you mean by this? A switch to VCC on the reset pin will do nothing?

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

I meant reversing the input components like you suggested.
I just don't like the idea that a passive component is always connected to VCC esp.for a low power app. I prefer it to be connected to GND and give VCC to it when needed. Plus, I think I am going to have to use software debouncing to eliminate the extra R and C.

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

The usual convention is to have active low enables. Just look at 9 out of 10 integrated circuits.

I agree that the logic is inverted but you will soon get used to it.

Your main consideration is what happens when the AVR is un-powered or in RESET. You do not want motors to start or bombs to explode. So you design your hardware accordingly. And just make the software to suit.

Rememer tha you can flip a bit by using the exclusive-or operator.

David.

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

Thanks, David. That makes sense.

One more question:
I don't understand how my logic is working. I have the following in my ISR:

temp = PINB & (0x01); 
if (temp)
    switch_on = 1; 

I have not yet inverted my logic. When the switch is not pressed, PB0 input is connected to VCC. Upon pressing the switch PB0 is connected to GND, so PINB & (0x01) should be 0 right which means the if loop condition should evaluate to false and the speaker should not beep?

But right now with this logic, the speaker is beeping when I press the switch. How is that possible?

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

What happens if you keep the button on at all times? You may also want to investigate what I said above:

Quote:
Do you realise that you get 2 interupts for each key press, one on closing and one on releasing?

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

When I keep the button on at all times, I don't get anything out of the speaker.Also I will check about the 2 interrupts although right now it seems to work fine if I press and release the switch once and no matter how long I keep the switch depressed, the speaker beeps when I release it. If it is working the way it is (with the RC deboucing), do I still need to use an extensive, deeply convoluted software debouncer?

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

Quote:

do I still need to use an extensive, deeply convoluted software debouncer?

Why? Just use the famous (fairly simple) code from Danni or Lee.

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

Do you have a quick link to the code, Cliff? Appreciate your help.

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

https://www.avrfreaks.net/index.p...

Enter the words "danni lee debounce" and tick the "all words" button (maybe limit it to only this forum?)

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

IIRC, Danni has also put his debouncing code up in in the projects area.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

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

Quote:
the speaker beeps when I release it.
Because you are picking up both the low going and high going pulses and, accidentally, it happens to turn on switch_on.

Have a good look at the logic in the ISR, put a filter in there so that ONLY either the low going pulse or the high going pulse does anything.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

I guess I have a question: If you are learning about interrupts, and want to "practice" with one edge, why didn't you pick an interrupt source that has edge selection?

(But debouncing still applies when using most (buttons; switches) devices of that type)

Lee

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

Aren't level triggered interrupts more resistant to glitches or unwanted noise triggers? Since the AVR has to now see only a level vs. an edge which is more susceptible to noise? I mean isn't level triggered interrupt similar to a latch?

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

Are you responding to my post?

If so, the thread to this point seems to be using a pin-change interrupt, which is edge-triggered. Thus, I don't know what you are getting at with your comment. The AVR "External Interrupt" (and several others such as Analog Comparator and Input Capture) has edge selection. (Yes, you can also have a level-triggered external interrupt. Pick a mode.)

If you are struggling with the button push, then clearing the level-triggered to avoid cascading is surely not a place to start.

Lee

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

Anyway here is an updated code that does things in the ISR only the low state of the pin.

#include  
#include  
#include  
#include  
#include  

 //switch on PB0 PCINT0 interrupt 
//10K pull up from PB0 to supply 
//0.1uF cap to ground for switch debouncing 
//Disable CKOUT for PB0 

volatile unsigned char switch_on = 0; 
volatile unsigned char temp; 

void beep() 
   { 

     for(int i=0;i<500;i++) 
      { 
         PORTC = 0x01; 
        _delay_ms(0.5); 
         PORTC = 0x00; 
        _delay_ms(0.5); 
      } 

   } 

void delay() //delay for beeping 

   { 
     _delay_ms(500); 
   } 


void ext_interrupt_init() 
    { 
     PCMSK0 = (1 << PCINT0); 
     PCICR = (1 << PCIE0);	//enable interrupt flag 
     sei(); 
    } 

ISR(PCINT0_vect) 
   { 
    switch_on = 0; 

	if (!(PINB & (1<<PB0)))	//Do something on low level only
		{
      	switch_on = 1;
		}
    } 

int main(void) 

   { 
	
//	DDRB = 0xFF; 		//WRONG need to be inputs!!
	PORTB = 0xFF;		//Use pullups
    DDRC = 0xFF; 

         ext_interrupt_init(); 
          for(;;) 
            { 
              if (switch_on) 
                { 

                  for(int j=0; j<10; j++) 

                    { 
                       delay(); 
                       beep(); 

//    					switch_on = 0; 

                     } 

                 } 
             }    
     } 

Don't really know how it works for you as you have portb as output!! I spent a good hour trying to figure out why I was getting PC int continuosly even though the swicth was not touched. It turned out the PB0 was blown on the chip. :evil: Maybe it blew up in another project, who knows. :(

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

Thanks for giving it a try, John. Appreciate it. I will try it myself.

One question I had on my original code:

Do you see any problems (in general like noise,reliability, etc.) that would affect it long term or when I move over to a PCB? Right now I am using a breadboarded circuit and it seems to work but the plan is to build it on a PCB moving forward. (I would think a breadboarded circuit is probably like a field test, being the noisiest at least compared with a PCB).

I kind of tested it by pressing the switch faster, longer etc. Maybe I should try different switches.

I will definitely try the more efficient code you worked on along with danni's debouncing, but for the meantime, I was of the opinion that if it ain't broke don't fix it.

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

theusch wrote:
Are you responding to my post?

If so, the thread to this point seems to be using a pin-change interrupt, which is edge-triggered. Thus, I don't know what you are getting at with your comment.
Lee

Sorry Lee, didn't realize this was an edge triggered interrupt. I will check and see if I can use INT0.

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

To avoid the edge phase problem with PCINT, check for the unwanted level immediately after entering the PCINT ISR. If it is the wrong edge, leave the ISR immediately.
If possible, use the INT0 or INT1 on the edge that is desired.

PORTB and DDRB initialize on reset to all zero logic. This means that the pull-up resistors are off and the port bits are all inputs.

I recommend turning on the internal pull-up resistor and having the switch pull the input pin to ground when pressed. Then, to save power, when the PCINT happens for that pin (switch pressed to ground), have the ISR turn off the PCINT and turn on the timer for about 20 milliseconds. Enable the timer interrupt and go back to sleep.
When the timer ISR happens, read the PortB pin and verify that it is still at ground. If yes, then turn off the timer interrupt and the timer. If no then set the timer for one more period of 20 milliseconds, turn the timer interrupt back on, exit the ISR and go back to sleep.
If, after the second 20mS period, the switch is not at ground, then it wasn't pressed. If it is at ground, leave the ISR and do the switch routine from the main code.
One problem with having delays in ISRs is that the code can't respond to other ISRs when in an ISR. You can set the general interrupt flag from inside the ISR to allow this, but the second ISR might extend your delay of the first ISR.
ISR code should be very short. It should set a flag for the main code, load a data byte between a peripheral register and memory, reload a timer, do a single left or right shift of a register, etc... All very short activities. No timed delays, no peripheral register polling, no number crunching. The shorter the ISR, the less likely that it is to cause side effects with the main code or other ISRs. The more ISRs that you have running, the more complex and useful your final application code can be.
If two interrupts happen at the same time, then there is a priority for which ISR will be executed first. After the first ISR, the main code will execute one instruction and then jump to the second ISR. Take care with UDRE ISR, (USART Data Register Empty). If it is enabled, and SEI is set, then the ISR will be re-entered continuously if no data is being sent out of the USART. Even if the ISR is only RETI, the main code will run at about 1/12 of its normal speed because only one main code instruction is executed before the ISR runs again. Usually, though, if the code is running at 1/8 of its expected speed, the CLK/8 fuse bit has been programmed. This is the factory-default condition on many newer AVRs, and it causes lots of grief for AVR beginning students. Ramble On.

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

Thanks for the great info,Simonetta. Actually, this is part of a much bigger program with multiple ISR's and timers.I know this is a lot for a newbie like me, but I want to try and get into it. I can see I am going to stumble a lot but in the end, it will be a good learning exercise.

Do you have any links to a complex program using multiple interrupts (like ADC, switch, UART,etc.) and timers? I have looked at a lot of flowcharts, but would like to see how these interrupt and timer based programs are coded to get an idea.

I will try using a timer and change my code as all of you have suggested. Thanks for your patience on this.

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

Now, maybe I've been at this too long but in general with short ISRs and signal rates not approaching AVR saturation, there isn't much "complex" about a program with lots of enabled interrupt sources. Yes, there are cases where there is interaction between ISRs but in general each does its thing and when each is solid they work together nicely.

A typical app for me has 4-8 ADC channels doing continuous conversions, so that would be an ADC interrupt every 100-200us. (They are then summed and periodically averaged for each channel to get the value to "close" the application loop.)

I'll always have a "heartbeat" timer at ~10ms. That is the basis for all internal timing. The ISR is [almost always] a one-instruction set of the "tick" flag. This interval is used as the gathering for all "dirty" inputs--buttons, keypad, switches, flowmeter pulses, inputs from PLC--that are then entered into the debounce mechanism that results in the debounced current state as well as the edges.

A typical app will have one (or more) USART links. While they may not always be active, when they are (e.g., controlling PC hooked up for monitor, setup, data dump) they are going full-tilt-boogie. Circular buffers for TX and RX.

There might be external int or pin-change or ICP or analog comparator or the like for "clean" signals--encoder, high-speed counter, etc.

There might be some other timer interrupts for, say, re-adjusting PWM periods each cycle.

All in all if your ISRs take a few us each and the max rate is say an interrupt every 100 us, it is no big deal. If each mechanism works properly then they will play nicely together.

Lee

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

Thanks, Lee that is very useful info. for a newbie like me.

The idea of a heartbeat timer sounds really useful. I will try to incorporate this in my code.

However, if I do have a heartbeat timer, will I still be able to fully put the AVR to sleep?

@Simonetta:

I have this sleep_cpu() command in my program right now. Does this save me any power?


void go_to_sleep()
 {

  sleep_cpu();

 }

int main(void)

  {

     DDRB = 0x00;//set PortB as input since PB0 is used as interrupt
	 DDRC = 0xFF;//set port C for output to beep speaker
	 DDRD = 0xFF;//set port D for output to flash LED
     ext_interrupt_init();

        for(;;)

           {

             go_to_sleep();

              if ((switch_on & 0x01) == 1)

               {  

                 for(int j=0; j

Here is how I interpreted it:
Processor goes to sleep in main, till it is woken up by a PCINT0 pin which then sets the switch_on flag. Is this true? Is the sleep_cpu() function buying me anything?