-2

I was trying to interface a seven-segment-display with Atmega16 chip with its decoder (74ls47), and increase the value it displays using ISR. The ISR should turn a led on and off then increase the value of the SSD, but it only makes the led blink and nothing happens to the SSD.

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include "DIO.h"


unsigned int counter=0;


int main(void)
{
    SREG |= (1<<7); //The Holy Gate
    GICR |= (1<<7); //Enableing INT1
    MCUCR |=(1<<2);     //for INT1
    MCUCR |=(1<<3);     //for INT1  
    DDRC =0xFF;
    PORTC =0;
    DDRB |=(1<<0);
    while (1) 
    {   

    }
}

ISR (INT1_vect)
{

    digitalWrite('B', 0, 1);
    _delay_ms(500);
    digitalWrite('B', 0, 0);
    if (counter <= 9) {
        PORTC=counter;
        counter++;
    } else {
        counter=0;
    }
}

Note: digitalWrite is a function to turn the led on and off which is pre-defined in "DIO.h" file

Thanks in advance.

Ôrel
  • 7,044
  • 3
  • 27
  • 46
Mohamed Mostafa
  • 582
  • 1
  • 3
  • 12
  • 5
    500ms delay is not the best option for interrupts. – unalignedmemoryaccess Aug 23 '18 at 15:34
  • 1
    Indeed, better to invert the LED state in every interrupt, which you can then pass once as `digitalWrite('B', 0, LEDstate);` – Weather Vane Aug 23 '18 at 15:37
  • Still, you say the LED flashes but there is no output to the 7-segment display. It depends on how you have connected the 7-segment driver 74LS47. If control lines `/LT` and `/RBI` and `/BI` are also wired to port C then you should set them as `1` level output as shown on the data sheet's truth table for the IC, otherwise clamp them ditto. – Weather Vane Aug 23 '18 at 16:12
  • Actually when I've replaced the line : ( PORTC=counter; ) with ( PORTC=any number ) it works perfectly – Mohamed Mostafa Aug 23 '18 at 21:44
  • So Atmega16's PORTs *do* work, but that's just a random result: please focus on my last comment. Have you tried `PORTC = counter | 0xF0;` ? – Weather Vane Aug 23 '18 at 22:32
  • Unfortunately I've tried it too, and it didn't work. The SSD only shows "0" and even the Atmega16's port doesn't change its value when the interrupt happens. – Mohamed Mostafa Aug 24 '18 at 02:26
  • counter needs to be `volatile`. And what you do when you get problems like this is to bring out your scope/logic analyser and measure the pins. If you haven't done that yet, please do. – Lundin Aug 24 '18 at 06:46
  • @Lundin - no, it does not need to be volatile as it is only accessed within the ISR. – Chris Stratton Aug 25 '18 at 04:55

1 Answers1

0

Ok, actually I've solved it and have no idea how did it work. I defined "counter" volatile.

My mistake :

volatile unsigned char counter = 0;
....
ISR (INT0_vect)
{
      if (counter <= 9) {
        PORTC=counter;
        counter++;}
        else {
        counter=0;}
}
int main (void)
{
    ....
    while(1)
    {

    }
}

The corrected code :

volatile unsigned char counter ;
....
ISR (INT0_vect)
{
      if (counter <= 9) {
        PORTC=counter;
        counter++;}
        else {
        counter=0;}
}
int main (void)
{
    ....
    counter=0;
    while(1)
    {

    }
}
Mohamed Mostafa
  • 582
  • 1
  • 3
  • 12
  • This should not matter, as the variable is assigned in the same scope where it is used. You would only need to use `volatile` if you used the variable in both the ISR and something else, such as your foreground code. It's more likely your issue was something related to the highly erroneous original use of a **half second delay within an ISR**. – Chris Stratton Aug 26 '18 at 16:41