1
\$\begingroup\$

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.

\$\endgroup\$
1
\$\begingroup\$

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;
}
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Yes I got around to writing a function like that. Except that I did not use pointers and had to explicitly return a value. \$\endgroup\$ – hacker804 Jul 22 '17 at 11:28
0
\$\begingroup\$

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.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.