Need help improving code for AVR

Question

So there is some code that I wrote for a Clock project I am working on that uses an ATMega328P and an DS1307 RTC.

#include <avr/io.h>
#include <stdlib.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include <util/atomic.h>

typedef unsigned char   u8;
typedef signed short    s16;


#include "lcd.h"
#include "i2cmaster.h"
#define F_CPU 1000000UL

#define RTC 0xD0
#define KEY_PIN     PINB
#define KEY_PORT    PORTB
#define KEY_DDR     DDRB
#define KEY0        0
#define KEY1        1
#define KEY2        2
#define KEY3        3

u8 key_state;               // debounced and inverted key state:
                            // bit = 1: key pressed
u8 key_press;               // key press detect

ISR(TIMER0_OVF_vect)
{
    static u8 ct0 = 0xFF, ct1 = 0xFF;   // 8 * 2bit counters
    u8 i;

    i = ~KEY_PIN;               // read keys (low active)
    i ^= key_state;             // key changed ?
    ct0 = ~( ct0 & i );         // reset or count ct0
    ct1 = ct0 ^ (ct1 & i);      // reset or count ct1
    i &= ct0 & ct1;             // count until roll over ?
    key_state ^= i;             // then toggle debounced state
    key_press |= key_state & i;     // 0->1: key press detect
}
u8 get_key_press( u8 key_mask )
{
    ATOMIC_BLOCK(ATOMIC_FORCEON){       // read and clear atomic !
        key_mask &= key_press;      // read key(s)
        key_press ^= key_mask;      // clear key(s)
    }
    return key_mask;
}
uint8_t dec_to_bcd(uint8_t dec)
{   
    uint8_t x;
    x = ((dec / 10) << 4) + (dec % 10);
    return(x);
}
uint8_t bcd_to_dec(uint8_t bcd)
{
    return (((0xF0 & bcd)  >> 4)* 10) + (0x0F & bcd);
}
uint8_t set_sec_min()               // Set seconds and minutes.
{
    uint8_t sec_min;
    char buff[4];
    while(1)                        // Set seconds/minutes;
    {
        lcd_gotoxy(0,0);
        itoa(sec_min,buff,10);
        lcd_puts(buff);
        if( get_key_press( 1<<KEY1 ))
        {
            sec_min++;
        }
        if( get_key_press( 1<<KEY2 ))
        {
            sec_min--;
        }
        if( get_key_press( 1<<KEY3 ))
        {
            break;
        }
        if( sec_min > 59)
        {
            sec_min =0;
        }
    }

    return(sec_min);
}                               
void init_timer0()
{
    TCCR0B |= (1 << CS01);  // set /8 prescaler
    TIMSK0 |= (1 << TOIE0); // Enable overflow interrupt.
    sei();
}
void set_time()
{
    uint8_t seconds;
    uint8_t minutes;
    uint8_t hours;
    uint8_t day;
    uint8_t date;
    uint8_t month;
    uint8_t year;
    char buff[5];
    while(1)
    {
        lcd_clrscr();
        seconds = set_sec_min();
        lcd_clrscr();
        minutes = set_sec_min();
        lcd_clrscr();
        while(1)                        // Set hours;
        {
            lcd_gotoxy(0,0);
            itoa(hours,buff,10);
            lcd_puts(buff);
            if( get_key_press( 1<<KEY1 ))
            {
                hours++;
            }
            if( get_key_press( 1<<KEY2 ))
            {
                hours--;
            }
            if( get_key_press( 1<<KEY3 ))
            {
                break;
            }
            if( hours   > 12 | hours < 1)
            {
                hours = 1;
            }
        }
        hours = dec_to_bcd(hours);
        hours |= (1 << 6);              //Set 12 hour mode
        lcd_clrscr();
        while(1)                        // Set AM/PM;
        {
            lcd_gotoxy(0,0);
            if( get_key_press( 1<<KEY1 ) | get_key_press( 1<<KEY2 ) )
            {
                hours ^= (1 << 5);
            }
            if( get_key_press( 1<<KEY3 ))
            {
                break;
            }
            if(hours & (1 << 5))
            {
                lcd_puts("PM");
            }else
            {
                lcd_puts("AM");
            }
        }

        lcd_clrscr();
        while(1)                        // Set day;
        {

            lcd_gotoxy(0,0);
            itoa(day,buff,10);
            lcd_puts(buff);
            if( get_key_press( 1<<KEY1 ))
            {
                day++;
            }
            if( get_key_press( 1<<KEY2 ))
            {
                day--;
            }
            if( get_key_press( 1<<KEY3 ))
            {
                break;
            }
            if( day > 7 | day < 1)
            {
                day =1;
            }
        }

        lcd_clrscr();
        while(1)                        // Set date;
        {

            lcd_gotoxy(0,0);
            itoa(date,buff,10);
            lcd_puts(buff);
            if( get_key_press( 1<<KEY1 ))
            {
                date++;
            }
            if( get_key_press( 1<<KEY2 ))
            {
                date--;
            }
            if( get_key_press( 1<<KEY3 ))
            {
                break;
            }
            if( date > 31 | date < 1)
            {
                date =1;
            }
        }

            lcd_clrscr();
            while(1)                        // Set month;
            {

                lcd_gotoxy(0,0);
                itoa(month,buff,10);
                lcd_puts(buff);
                if( get_key_press( 1<<KEY1 ))
                {
                    month++;
                }
                if( get_key_press( 1<<KEY2 ))
                {
                    month--;
                }
                if( get_key_press( 1<<KEY3 ))
                {
                    break;
                }
                if( month > 12 | month < 1)
                {
                    month =1;
                }
            }
                lcd_clrscr();
                while(1)                        // Set year;
                {

                    lcd_gotoxy(0,0);
                    itoa(year,buff,10);
                    lcd_puts(buff);
                    if( get_key_press( 1<<KEY1 ))
                    {
                        year++;
                    }
                    if( get_key_press( 1<<KEY2 ))
                    {
                        year--;
                    }
                    if( get_key_press( 1<<KEY3 ))
                    {
                        break;
                    }
                    if( year > 99)
                    {
                        year =0;
                    }
                }
                    break;
    }


    i2c_init();
    i2c_start_wait(RTC+I2C_WRITE);
    i2c_write(0x00);                            // First register address; the RTC increments the register pointer after every byte write.

    i2c_write(dec_to_bcd(seconds));             // Write seconds.
    i2c_write(dec_to_bcd(minutes));             // Write minutes.
    i2c_write(hours);                           // Write hours.
    i2c_write(day);                             // Write days.
    i2c_write(dec_to_bcd(date));                // Write date.
    i2c_write(dec_to_bcd(month));               // Write month.
    i2c_write(dec_to_bcd(year));                // Write year.

    i2c_stop();
}
void display_time()
{
        uint8_t ret;
        char buff[4];
        char monthNames[][4] = {"Jan","Feb","Mar","Apr","May","Jun","Jul","Aug","Sep","Oct","Nov","Dec"};
        char dayNames[][4] = {"Mon","Tue","Wed","Thu","Fri","Sat","Sun"};

        i2c_start_wait(RTC+I2C_WRITE);          // Establish communication
        i2c_write(0x00);                        // Write Address of first register.
        i2c_rep_start(RTC+I2C_READ);            // Re-establish comm with READ mode.

    //************** PRINT SECONDS ************************
        ret = i2c_readAck();
        ret = bcd_to_dec(ret);
        itoa(ret,buff,10);
        if(ret > 9)
        {
            lcd_gotoxy(6,0);
            lcd_puts(buff);
        }else if(ret == 0)
        {
            lcd_gotoxy(6,0);
            lcd_puts("00");
        }
        else{   
            lcd_gotoxy(7,0);
            lcd_puts(buff);
        }   
    //***************************************************** 
    //************** PRINT MINUTES ************************
        ret = i2c_readAck();
        ret = bcd_to_dec(ret);
        itoa(ret,buff,10);
        if( ret > 9)
        {
            lcd_gotoxy(3,0);
            lcd_puts(buff);

        }else if(ret == 0)
        {
            lcd_gotoxy(3,0);
            lcd_puts("00");
        }
        else{
            lcd_gotoxy(4,0);
            lcd_puts(buff);
        }
    //*****************************************************
    //************** PRINT HOURS ************************
        ret = i2c_readAck();

            if( ret & (1 << 5))
            {
                lcd_gotoxy(9,0);
                lcd_puts("PM");
            }else
            {
                lcd_gotoxy(9,0);
                lcd_puts("AM");
            }
        ret = (((0x10 & ret)  >> 4)* 10) + (0x0F & ret);
        itoa(ret,buff,10);
        lcd_gotoxy(0,1);

            if( ret > 9)
            {
                lcd_gotoxy(0,0);
                lcd_puts(buff);
            }else
            {
                lcd_gotoxy(0,0);
                lcd_puts("0");
                lcd_gotoxy(1,0);
                lcd_puts(buff);
            }
    //*****************************************************
    //************** PRINT DAY ****************************
        ret = i2c_readAck();
        lcd_gotoxy(0,1);
        ret--;
        lcd_puts(dayNames[ret]);
    //******************************************************
    //************** PRINT DATE ****************************
        ret = i2c_readAck();
        ret = (((0x10 & ret)  >> 4)* 10) + (0x0F & ret);
        itoa(ret,buff,10);
        if(ret > 9)
        {
            lcd_gotoxy(4,1);
            lcd_puts(buff);
        }else
        {
            lcd_gotoxy(4,1);
            lcd_putc('0');
            lcd_gotoxy(5,1);
            lcd_puts(buff);
        }   
    //*******************************************************
    //************** PRINT Month ****************************
        ret = i2c_readAck();
        ret = (((0x10 & ret)  >> 4)* 10) + (0x0F & ret);
        lcd_gotoxy(7,1);
        ret--;
        lcd_puts(monthNames[ret]);
    //*****************************************************
    //************** PRINT Year ****************************
        ret = i2c_readNak();
        i2c_stop();
        ret = bcd_to_dec(ret);
        itoa(ret,buff,10);
        lcd_gotoxy(11,1);
        lcd_puts("20");
        lcd_puts(buff);
    //*****************************************************
}
int main(void)
{
    init_timer0();
    KEY_DDR = 0;                // input
    KEY_PORT = 0xFF;            // pullups on
   lcd_init(LCD_DISP_ON);
   lcd_home();
    while (1) 
    {
        if( get_key_press( 1<<KEY0 ))
        {
            set_time();
        }
        display_time();
    }
}

The thing is that now I want to be able to optimize the code. As you can see in the set_time() function, the increment/decrement action is repeated. Is there a way I can write a function for this and use it whenever I need to? Keep in mind that each of the variables has a different limit after which it resets/wraps.

Answer 1

Clarification of Terms

optimize the code

Optimisation in C is a specific term, I am assuming you are using AVR GCC as your compiler which has optimisation flags which will aim to minimise the size of the assembler code it generates from your C code. AVR studio sets the default optimisation level to 1. Have a look at this link for a good explanation of it.

Solution

I think what you mean is you want to reduce the lines of code you are using to make your project look a bit nicer so I have given you an idea of what to do for your set_time() function in the code block at the bottom. This will work for all the date time elements except for the 12hr and AM/PM one which needs rewritten to conform to the others or just left as a separate function.

In each while loop replace the if statements with what is below and then you need to add my function check_action(). Check action accepts a pointer to the date time element that you want to change and its upper and lower limits. (*pointer)++ will evaluate the value of the data that is at the pointer location first and then increment that value. The function will normally return 0 and will stay in the while loop unless key 3 is pressed, then it will return 1 and break will be called.

void set_time(void)
{
    uint8_t hours, *p_hours = &hours;

    .....

    while(1) //set seconds
    {
        //LCD functions
    
        //Replace if statements with this
        if(check_action(p_hours, 12, 1))
            break;
    }

    .....
}

int check_action(uint8_t *date_time_element,
              uint8_t upper_limit,
              uint8_t lower_limit)
{
    if( get_key_press( 1<<KEY1 ))
    {
        (*date_time_element)++;
    }
    if( get_key_press( 1<<KEY2 ))
    {
        (*date_time_element)--;
    }
    if( get_key_press( 1<<KEY3 ))
    {
        return 1;
    }
    if( (*date_time_element) > upper_limit 
      | (*date_time_element) < lower_limit)
    {
        (*date_time_element) = lower_limit;
    }
    return 0;
}

Answer 2

Is there a way I can write a function for this and use it whenever I need to?

you may consider breaking it down to at least two parts:

1) read the key pads; this will return the key pressed; 2) process the key pressed: likely a set of switch / case statements; 3) based on the outcome of the processing in 2), update the display.

what you have now, with all the while loops, is messy.