LED it's not blinking as expected on ATmega164

robi10101298 · Dec 1, 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:

C:

/*********************************************
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:

C:

/* 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:

What am I doing wrong?

--- Updated Dec 1, 2020 ---

This is my scheme

KlausST · Dec 1, 2020

Hi,

in the ISR you toggle LED1. But LED1 here is just a variable - as much as I can see. I can´t see how LED1 is linked to PortD.6.
In your circuit there is a LED connected at PortD.6.
--> Here you urgently need a current limiting resistor! Else something may get damaged.

PortD.6 is just initialized, but never used in software.

--> I expect: in ISR: toggle portD.6.

Klaus

robi10101298 · Dec 1, 2020

Here it's a simplified version of the main function:

C:

/*********************************************
Project : Test software
**********************************************
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)


/*
* 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
        for(i=0; i<3; i++) {
            TESTP = 1;
            delay_us(1);
            TESTP = 0;   // may check accuracy of 1us interval on oscilloscope    
        }
    }

           
}// end main loop

--- Updated Dec 1, 2020 ---

KlausST said:
Hi,

in the ISR you toggle LED1. But LED1 here is just a variable - as much as I can see. I can´t see how LED1 is linked to PortD.6.
In your circuit there is a LED connected at PortD.6.
--> Here you urgently need a current limiting resistor! Else something may get damaged.

PortD.6 is just initialized, but never used in software.

--> I expect: in ISR: toggle portD.6.

Klaus

Hello, it's initialized in the defs.h file:

C:

/* 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"

KlausST · Dec 1, 2020

Hi,

O.K.

I´m not familiar with C ... but
"LED1 = ~LED1;"
can the compiler translate it to:
* read portD --> pdVal
* XOR pdVal with 0x40
* write pdVal to portD
?

Klaus

robi10101298 · Dec 1, 2020

KlausST said:
Hi,

O.K.

I´m not familiar with C ... but
"LED1 = ~LED1;"
can the compiler translate it to:
* read portD --> pdVal
* XOR pdVal with 0x40
* write pdVal to portD
?

Klaus

Yes, it can translate this

KlausST · Dec 3, 2020

Hi,

try declaring variables that are used in ISR as well as in MAIN as "volatile". rx_counter0 for example.

Klaus

robi10101298 · Dec 3, 2020

KlausST said:
Hi,

try declaring variables that are used in ISR as well as in MAIN as "volatile". rx_counter0 for example.

Klaus

This is my final code but it's blinking really weird...3 times, then 4 times, then again 3 times, then 4 times, and so on....:

C:

#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);
        }               
}

KlausST · Dec 3, 2020

Hi,

I guess it does what you told it to do.

In main() add a static variable last_sw1
then do (pseudo code, not a very clean solution):

Code:

If (last_sw1 == 1) && (sw1 ==0)"{  // code for detecting key press edge
...switch frequency}
last_sw1 = sw1
wait_ms(20)

Klaus

Welcome to EDAboard.com

LED it's not blinking as expected on ATmega164

robi10101298

Junior Member level 2

KlausST

Advanced Member level 7

robi10101298

Junior Member level 2

KlausST

Advanced Member level 7

robi10101298

Junior Member level 2

KlausST

Advanced Member level 7

robi10101298

Junior Member level 2

KlausST

Advanced Member level 7

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Connect with us

Online statistics

Forum statistics