Connect with us

LED it's not blinking at the expected rate

Discussion in 'Microcontrollers, Programming and IoT' started by robi10101298, Dec 1, 2020.

Scroll to continue with content
  1. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    Hello guys, I'm trying to make a led blink at 3 different frequencies, upon successive keypresses. My problem right now it's that it's only blinking 2 times and I don't understand why. D6 is LED, D4 is the test output. Here it's my main function:
    [Mod edit: tidied up the code by removing blank line for better readability]
    Code:
    /*********************************************
    Chip type: ATmega164A
    Clock frequency: 20 MHz
    Compilers:  CVAVR 2.x
    *********************************************/
    #include <mega164a.h>
    #include <stdio.h>
    #include <delay.h>
    #include <string.h>
    #include <stdlib.h>
    #include "defs.h"
    //*************************************************************************************************
    //*********** BEGIN SERIAL STUFF (interrupt-driven, generated by Code Wizard) *********************
    //*************************************************************************************************
    
    #ifndef RXB8
    #define RXB8 1
    #endif
    
    #ifndef TXB8
    #define TXB8 0
    #endif
    
    #ifndef UPE
    #define UPE 2
    #endif
    
    #ifndef DOR
    #define DOR 3
    #endif
    
    #ifndef FE
    #define FE 4
    #endif
    
    #ifndef UDRE
    #define UDRE 5
    #endif
    
    #ifndef RXC
    #define RXC 7
    #endif
    #define FRAMING_ERROR (1<<FE)
    #define PARITY_ERROR (1<<UPE)
    #define DATA_OVERRUN (1<<DOR)
    #define DATA_REGISTER_EMPTY (1<<UDRE)
    #define RX_COMPLETE (1<<RXC)
    // USART0 Receiver buffer
    #define RX_BUFFER_SIZE0 8
    char rx_buffer0[RX_BUFFER_SIZE0];
    
    #if RX_BUFFER_SIZE0 <= 256
    unsigned char rx_wr_index0,rx_rd_index0,rx_counter0;
    #else
    unsigned int rx_wr_index0,rx_rd_index0,rx_counter0;
    #endif
    // This flag is set on USART0 Receiver buffer overflow
    bit rx_buffer_overflow0;
    // USART0 Receiver interrupt service routine
    interrupt [USART0_RXC] void usart0_rx_isr(void)
    {
    char status,data;
    status=UCSR0A;
    data=UDR0;
    if ((status & (FRAMING_ERROR | PARITY_ERROR | DATA_OVERRUN))==0)
       {
       rx_buffer0[rx_wr_index0++]=data;
    
    #if RX_BUFFER_SIZE0 == 256
       // special case for receiver buffer size=256
       if (++rx_counter0 == 0) rx_buffer_overflow0=1;
    #else
       if (rx_wr_index0 == RX_BUFFER_SIZE0) rx_wr_index0=0;
       if (++rx_counter0 == RX_BUFFER_SIZE0)
         {
         rx_counter0=0;
         rx_buffer_overflow0=1;
         }
    #endif
       }
    }
    
    #ifndef _DEBUG_TERMINAL_IO_
    // Get a character from the USART0 Receiver buffer
    #define _ALTERNATE_GETCHAR_
    #pragma used+
    char getchar(void)
    {
    char data;
    while (rx_counter0==0);
    data=rx_buffer0[rx_rd_index0++];
    
    #if RX_BUFFER_SIZE0 != 256
    if (rx_rd_index0 == RX_BUFFER_SIZE0) rx_rd_index0=0;
    #endif
    #asm("cli")
    --rx_counter0;
    #asm("sei")
    return data;
    }
    #pragma used-
    #endif
    // USART0 Transmitter buffer
    #define TX_BUFFER_SIZE0 8
    char tx_buffer0[TX_BUFFER_SIZE0];
    
    #if TX_BUFFER_SIZE0 <= 256
    unsigned char tx_wr_index0,tx_rd_index0,tx_counter0;
    #else
    unsigned int tx_wr_index0,tx_rd_index0,tx_counter0;
    #endif
    // USART0 Transmitter interrupt service routine
    interrupt [USART0_TXC] void usart0_tx_isr(void)
    {
    if (tx_counter0)
       {
       --tx_counter0;
       UDR0=tx_buffer0[tx_rd_index0++];
    
    #if TX_BUFFER_SIZE0 != 256
       if (tx_rd_index0 == TX_BUFFER_SIZE0) tx_rd_index0=0;
    #endif
       }
    }
    
    #ifndef _DEBUG_TERMINAL_IO_
    // Write a character to the USART0 Transmitter buffer
    #define _ALTERNATE_PUTCHAR_
    #pragma used+
    void putchar(char c)
    {
    while (tx_counter0 == TX_BUFFER_SIZE0);
    #asm("cli")
    if (tx_counter0 || ((UCSR0A & DATA_REGISTER_EMPTY)==0))
       {
       tx_buffer0[tx_wr_index0++]=c;
    
    #if TX_BUFFER_SIZE0 != 256
       if (tx_wr_index0 == TX_BUFFER_SIZE0) tx_wr_index0=0;
    #endif
       ++tx_counter0;
       }
    else
       UDR0=c;
    #asm("sei")
    }
    #pragma used-
    #endif
    //*************************************************************************************************
    //********************END SERIAL STUFF (USART0)  **************************************************
    //*************************************************************************************************
    //*******   if you need USART1, enable it in Code Wizard and copy coresponding code here  *********
    //*************************************************************************************************
    /*
     * Timer 1 Output Compare A interrupt is used to blink LED
     */
    interrupt [TIM1_COMPA] void timer1_compa_isr(void)
    {
    LED1 = ~LED1; // invert LED
    }                              
    /*
     * main function of program
     */
    void main (void)
    {      
       Init_initController();  // this must be the first "init" action/call!
       #asm("sei")             // enable interrupts
       LED1 = 1;               // initial state, will be changed by timer 1
       while(TRUE)
       {
           wdogtrig();         // call often else processor will reset        
           if(SW1 == 0)        // pressed
           {
               delay_ms(30);   // debounce switch
               if(SW1 == 0)
               {                // LED will blink slow or fast
                   while(SW1==0)
                       wdogtrig();    // wait for release
                   // alternate between values and values/4 for OCR1A register
                   // 4C40H / 4 = 1310H
                   // new frequency = old frequency * 4
                   if(OCR1AH == 0x4C)
                       {TCNT1H=0; TCNT1L=0; OCR1AH = 0x13; OCR1AL = 0x10;}
                   else  
                       {TCNT1H=0; TCNT1L=0; OCR1AH = 0x4C; OCR1AL = 0x40;}        
               }            
           }                                    
        
           // measure time intervals on oscilloscope connected to pin TESTP
      
       }
            
    }// end main loop
    


    And here it's the init.c function:

    Code:
    /* initialization file */
    #include <mega164a.h>
    #include "defs.h"
    
    /*
     * most intialization values are generated using Code Wizard and depend on clock value
     */
    
    void Init_initController(void)
    {
    // Crystal Oscillator division factor: 1
    #pragma optsize-
    CLKPR=0x80;
    CLKPR=0x00;
    #ifdef _OPTIMIZE_SIZE_
    #pragma optsize+
    #endif
    
    // Input/Output Ports initialization
    // Port A initialization
    // Func7=In Func6=In Func5=In Func4=In Func3=In Func2=In Func1=In Func0=In
    // State7=T State6=T State5=T State4=T State3=T State2=T State1=T State0=T
    PORTA=0x00;
    DDRA=0x00;
    
    // Port B initialization
    PORTB=0x00;
    DDRB=0x00;
    
    // Port C initialization
    PORTC=0x00;
    DDRC=0x00;
    
    // Port D initialization
    PORTD=0b00100000; // D.5 needs pull-up resistor
    DDRD= 0b01010000; // D.6 is LED, D.4 is test output
    
    // Timer/Counter 0 initialization
    // Clock source: System Clock
    // Clock value: Timer 0 Stopped
    // Mode: Normal top=FFh
    // OC0 output: Disconnected
    TCCR0A=0x00;
    TCCR0B=0x00;
    TCNT0=0x00;
    OCR0A=0x00;
    OCR0B=0x00;
    
    // Timer/Counter 1 initialization
    // Clock source: System Clock
    // Clock value: 19.531 kHz = CLOCK/256
    // Mode: CTC top=OCR1A
    // OC1A output: Discon.
    // OC1B output: Discon.
    // Noise Canceler: Off
    // Input Capture on Falling Edge
    // Timer 1 Overflow Interrupt: Off
    // Input Capture Interrupt: Off
    // Compare A Match Interrupt: On
    // Compare B Match Interrupt: Off
    TCCR1A=0x00;
    TCCR1B=0x0D;
    TCNT1H=0x00;
    TCNT1L=0x00;
    ICR1H=0x00;
    ICR1L=0x00;
    
    // 1 sec = 19531 counts = 4C41H counts, from 0 to 4C40
    // 4C40H = 4CH (MSB) and 40H (LSB)
    OCR1AH=0x4C;
    OCR1AL=0x40;
    OCR1BH=0x00;
    OCR1BL=0x00;
    
    // Timer/Counter 2 initialization
    // Clock source: System Clock
    // Clock value: Timer2 Stopped
    // Mode: Normal top=0xFF
    // OC2A output: Disconnected
    // OC2B output: Disconnected
    ASSR=0x00;
    TCCR2A=0x00;
    TCCR2B=0x00;
    TCNT2=0x00;
    OCR2A=0x00;
    OCR2B=0x00;
    
    // External Interrupt(s) initialization
    // INT0: Off
    // INT1: Off
    // INT2: Off
    // Interrupt on any change on pins PCINT0-7: Off
    // Interrupt on any change on pins PCINT8-15: Off
    // Interrupt on any change on pins PCINT16-23: Off
    // Interrupt on any change on pins PCINT24-31: Off
    EICRA=0x00;
    EIMSK=0x00;
    PCICR=0x00;
    
    // Timer/Counter 0,1,2 Interrupt(s) initialization
    TIMSK0=0x00;
    TIMSK1=0x02;
    TIMSK2=0x00;
    
    // USART0 initialization
    // Communication Parameters: 8 Data, 1 Stop, No Parity
    // USART0 Receiver: On
    // USART0 Transmitter: On
    // USART0 Mode: Asynchronous
    // USART0 Baud rate: 9600
    UCSR0A=0x00;
    UCSR0B=0xD8;
    UCSR0C=0x06;
    UBRR0H=0x00;
    UBRR0L=0x81;
    
    // USART1 initialization
    // USART1 disabled
    UCSR1B=0x00;
    
    
    // Analog Comparator initialization
    // Analog Comparator: Off
    // Analog Comparator Input Capture by Timer/Counter 1: Off
    ACSR=0x80;
    ADCSRB=0x00;
    DIDR1=0x00;
    
    // Watchdog Timer initialization
    // Watchdog Timer Prescaler: OSC/2048
    #pragma optsize-
    #asm("wdr")
    // Write 2 consecutive values to enable watchdog
    // this is NOT a mistake !
    WDTCSR=0x18;
    WDTCSR=0x08;
    #ifdef _OPTIMIZE_SIZE_
    #pragma optsize+
    #endif
    
    }
    And this is how my minimalistic setup looks like:

    [​IMG]


    What am I doing wrong?
     
    Last edited: Dec 1, 2020
  2. Bluejets

    Bluejets

    4,649
    981
    Oct 5, 2014
    You could start with a current limit resistor on series with the LED.
    Most uC ports will handle only 20mA.
    Anyones guess on how much excess current an LED without a resistor will try to pull from the port pin.
    Possibly even damage the port or the complete chip.
     
    hevans1944 and Harald Kapp like this.
  3. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    You cannot invert a single pin for the LED with a statement like
    You'll have to do some bit arithmetic like this:
    Code:
    #define LED PD4 //will be D6 in your application
    
    ISR (TIMER1_OVF_vect)    // Timer1 ISR
    {
        PORTD ^= (1 << LED);    // bitwise EXOR
        TCNT1 = 63974;   // for 1 sec at 16 MHz // not required, I think
    }
    
    (taken from this website).

    Your code possibly overrides the input on D5 as it inverts all bits on port D (not clear as I couldn't locate the definition of LED1 in the code) thus setting D5 to "0" which will lead to an endless while loop in main here:
    Code:
                   while(SW1==0)
                       wdogtrig();    // wait for release
    Another possible issue is here:
    Code:
    // Port D initialization
    PORTD=0b00100000; // D.5 needs pull-up resistor
    DDRD= 0b01010000; // D.6 is LED, D.4 is test output
    For one this should read:
    Code:
    // Port D initialization
    PORTD=0b00100000; // D.5 needs pull-up resistor
    DDRD= 0b001010000; // D.6 is LED, D.4 is test output
    D7 was missing in DDRD= statement. This is not the issue, just for the sake of clarity.
    Here's the catch: all other pins of port D are declared as inputs but left floating in the circuit. This leads to unpredictable behaviour. All unused inputs should be explicitly declared as Pull-Up. Or declare the unused pins as outputs.
     
    Last edited: Dec 2, 2020
  4. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    What I've found is that this code could possibly work

    Code:
    if(SW1 == 0)        // pressed
           {
               delay_ms(30);   // debounce switch
               if(SW1 == 0)   
               {                // LED will blink slow or fast
                   while(SW1==0)
                       wdogtrig();    // wait for release
                      
                   // alternate between values and values/4 for OCR1A register
                   // 4C40H / 4 = 1310H
                   // new frequency = old frequency * 4
                   if(OCR1AH == 0x4C)
                       {TCNT1H=0; TCNT1L=0; OCR1AH = 0x13; OCR1AL = 0x10;}    //0x1310
                   else if(OCR1AH == 0x13)
                       {TCNT1H=0; TCNT1L=0; OCR1AH = 0x2F; OCR1AL = 0xA9;}    //0x2fa9
                   else
                       {TCNT1H=0; TCNT1L=0; OCR1AH = 0x4C; OCR1AL = 0x40;}    //0x4C40           
              
               }
    Also, LED's are declared in the defs.h file:
    Code:
    /* definitions / defines file */
    #define DEFS_H
    
    #define SW_VERSION        13   /* i.e. major.minor software version nbr. */
    
    #ifndef NULL
    #define NULL  0
    #endif
            
    // logix ...
    #define TRUE    1
    #define FALSE    0
    #define DUMMY    0
    
    #define wdogtrig()            #asm("wdr") // call often if Watchdog timer enabled
    
    #define CR                0x0D
    #define LF                0x0A 
    
    #define LED1 PORTD.6        // PORTx is used for output
    #define SW1 PIND.5          // PINx is used for input
    #define TESTP PORTD.4       // test bit durations
    #include "funct.h"
    
    
     
  5. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    This notation is unknown to me for AVR C programming. Where can I read more on it?

    The other remarks about open inputs and missing resistors still are open.
     
  6. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    This is Arduino like port declaration
    Arduino - PortManipulation
     
  7. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    Thanks for the link. But there is no mentioning of syntax like "PIND.5". On the contrary, the Arduino manual recommends using DigitalRead() and DigitalWrite().
    However, obviously the CVAVR compiler supports this syntax, otherwise you'd have received an error.
    It is o.k. as long as you stick to this compiler, but note that using such specialties limits portability of your code to other compilers (e.g. GCC).
     
  8. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    You're welcome! I think right now I'm fine with this CVAVR compiler, I don't think I'll use other
     
  9. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    Now back to your code: does it work or do you still have issues?
     
  10. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    Thanks for your help! It's blinking, but not at the expected rate, I mean it's having a chaotic behaviour... For example, I turn on the switch, it's blinking 2 times. I turn off the switch, it's still blinking a few times then it's not blinking anymore. I turn again on the switch it's blinking faster a lot of times
     
  11. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    Lets try to sort this out.
    First let's see whether the basic blinking routine works properly. To do this, change main() to a most basic code:
    Code:
    void main (void)
    {     
       Init_initController();  // this must be the first "init" action/call!
       #asm("sei")             // enable interrupts
       LED1 = 1;               // initial state, will be changed by timer 1
       while(TRUE)
       {
           wdogtrig();         // call often else processor will reset       
         }
            
    }// end main loop
    
    This should now have the LED blinking with a rate determined by the parameters from the init file:
    Code:
    // 1 sec = 19531 counts = 4C41H counts, from 0 to 4C40
    // 4C40H = 4CH (MSB) and 40H (LSB)
    OCR1AH=0x4C;
    OCR1AL=0x40;
    
    Does this work as supposed? If so, does the alternate setting work too?
    Code:
    {TCNT1H=0; TCNT1L=0; OCR1AH = 0x2F; OCR1AL = 0xA9;}
    Do the rates of blinking match what you expect? If so, the timer setup is o.k. If not, check your timer settings.
     
  12. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    The problem is that when I'm using a switch, if it's on 0 position the circuit it's still blinking...So the switch it's obsolete...Right now it's blinking 3 times but I can't see so cleary if it's 3 times or 4 times.. How can I check this?
     
  13. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    Start by eliminating the switch as I suggested.
    Lower the blink rate so you can easily count.
     
  14. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    Nope, still no effect... Can you please provide me a complete code from scratch?
     
  15. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    Sorry, I don't have time to write the code myself. But you find examples on the internet, e.g. this one.
     
  16. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    This is my final code but it's having a really weird behavior... 3 times blinking, then 4 times, 3 times blinking, then 4 times, and so on...
    Code:
    #include <mega164a.h>
    
    #include <stdio.h>
    #include <delay.h> 
    #include <string.h>
    #include <stdlib.h>
    #define LED1 PORTD.6        // PORTx is used for output
    #define SW1 PIND.5          // PINx is used for input
    // 1 sec = 19531 counts = 4C41H counts, from 0 to 4C40
    // 4C40H = 4CH (MSB) and 40H (LSB)
    //1 sec
    OCR1AH=0x4C;
    OCR1AL=0x4B;
    // Port D initialization
    PORTD=0b00100000; // D.5 needs pull-up resistor
    DDRD= 0b01010000; // D.6 is LED, D.4 is test output
    while(1){
    if(SW1 == 0)        // pressed
            {
            for(i=0; i<2; i++){
                     if(OCR1AH == 0x4C) 
                        {TCNT1H=0; TCNT1L=0; OCR1AH = 0x13; OCR1AL = 0x10;}    //0x1310
                    else if(OCR1AH == 0x13)
                        {TCNT1H=0; TCNT1L=0; OCR1AH = 0x2F; OCR1AL = 0xA9;}    //0x2fa9
                    else
                        {TCNT1H=0; TCNT1L=0; OCR1AH = 0x4C; OCR1AL = 0x40;}    //0x4C40
                    
                     } 
                     delay_ms(1000);
            }               
    }
     
  17. Harald Kapp

    Harald Kapp Moderator Moderator

    11,276
    2,583
    Nov 17, 2011
    The for loop doesn't make sense.
    When you first enter the loop PCR1AH == 0x4C as per declaration at the start of the program.
    In the first iteration of the loop (i == 0) this will lead to OCR1AH = 0x13 (first if statement is true).
    In the next iteration of the loop (i==1) this will lead to OCR1AH = 0x2F (else if statement is now true since OCR1AH == 0x13 from the first iteration).
    Remove the for loop completely, it is not required here.
    Also: no need to set TCNT1H=0; TCNT1L=0; every time. As these values don't change, it is sufficient to set them once at the beginning of the program.
     
  18. robi10101298

    robi10101298

    21
    0
    May 25, 2020
    Thanks for your help but I've solved in a much easier way:
    Code:
    void handle_press() {
            int i;
            int state = 0;
    int delay[] = { 150, 250, 500 };
      /* blink three times at the rate specified for the current state */
      for (i = 0; i < 3; ++i) {
        LED1 = 1;
        delay_ms(delay[state]);
        LED1 = 0;
        delay_ms(delay[state]);
      }
     
      /* then wait another second */
      delay_ms(1000);
     
      /* update state */
      state = (state + 1);
      if (state == 3) {
        state = 0;
      }
    }
    void main (void)
    {         
    unsigned char temp,i;
    char a=1;
        int teamNr = 13;
    
        Init_initController();  // this must be the first "init" action/call!
        #asm("sei")             // enable interrupts
        LED1 = 1;               // initial state, will be changed by timer 1
        while(TRUE)
        {
            wdogtrig();            // call often else processor will reset
        
    
           if (SW1 == 0) {
          handle_press();
            }                 
    
        }
    
                
    }// end main loop
    
     
Ask a Question
Want to reply to this thread or ask your own question?
You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.
Electronics Point Logo
Continue to site
Quote of the day

-