Electronic – Preventing Stack overflows in PIC MCU using C language MPLAB X

cmicrocontrollerpicstack

I am writing a C language code for PIC16f877a for an Alarm clock. In short, the alarm clock shows Time, Set alarm and a temperature & humidity reading on an LCD.

I have written interrupts for updating the time, for setting the alarm on and off and few others I plan to write.

The issue is, I am at half point of my code and the Stack overflow problem has risen in the proteus simulation. I have identified the part of code causing the problem. But I can't seem to understand how should I change it to prevent stack overflow.

Can someone help me debug this code? And please note down bad practices in C microcontroller programming which cause similar bugs? I can't seem to find such list of bad practices on the internet.

Here is the code I have written for setting the alarm/time on the clock. The overflow starts happening once I select the option of setting time. Then keeps happening until PIC crashes even when I come out of the setTime routine.

#define UP RD0   // Push buttons active high
#define DOWN RD1
#define SELECT RD2
#define BACK RD3
#define SNOOZE RD4

void UpdateSetTimeScreen() {
    if (ptr > 1)
        LCD_cursor(ptr + 1, 1);
    else
        LCD_cursor(ptr, 1);

    if (set[ptr] == 0)
        LCD_puts("_");
    else
        LCD_display_value(set[ptr]);
}

void SetTime() {
    LCD_clear();
    LCD_puts("Set Time");

    LCD_cursor(0, 1);
    LCD_puts("__:__");
    LCD_cursor(0, 1);

    set[0] = 0;
    set[1] = 0;
    set[2] = 0;
    set[3] = 0;

    ptr = 0;

    unsigned int i =1;

    while (i) {
        if (UP == 1) {
            while (UP);
            switch (ptr) {
                case 0:
                {
                    if (set[ptr] < 2) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
                case 1:
                {
                    if (set[ptr] < 3) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
                case 2:
                {
                    if (set[ptr] < 5) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
                case 3:
                {
                    if (set[ptr] < 9) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
            }
        }
        if (DOWN == 1) {
            while (DOWN);
            if (set[ptr] > 0) {
                set[ptr]--;
                UpdateSetTimeScreen();
            }
        } else if (BACK == 1) {
            while (BACK);
            ptr--;
        } else if (SELECT == 1) {
            while (SELECT);
            if (ptr == 3)
                i=0;
            else
                ptr++;
        }
    }

    hour = (set[0]*10) + set[1];
    minute = (set[2]*10) + set[3];
}

void SetAlarm() {
    LCD_clear();
    LCD_puts("Set Alarm Time");
    LCD_cursor(0, 1);
    LCD_puts("__:__");
    LCD_cursor(0, 1);

    set[0] = 0;
    set[1] = 0;
    set[2] = 0;
    set[3] = 0;

    ptr = 0;
    unsigned int j = 1;

    while (j) {
        if (UP == 1) {
            while (UP);
            switch (ptr) {
                case 0:
                {
                    if (set[ptr] < 2) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
                case 1:
                {
                    if (set[ptr] < 3) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
                case 2:
                {
                    if (set[ptr] < 5) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
                case 3:
                {
                    if (set[ptr] < 9) {
                        set[ptr]++;
                        UpdateSetTimeScreen();
                    }
                    break;
                }
            }
        }
        if (DOWN == 1) {
            while (DOWN);
            if (set[ptr] > 0) {
                set[ptr]--;
                UpdateSetTimeScreen(ptr);
            }
        } else if (BACK == 1) {
            while (BACK);
            if(ptr > 0)
                ptr--;
        } else if (SELECT == 1) {
            while (SELECT);
            if (ptr == 3)
                j=0;
            else
                ptr++;
        }
    }

    AlarmHr = (set[0]*10) + set[1];
    AlarmMin = (set[2]*10) + set[3];

}

void SetTemp() {
}

void print_configuration_screen() {
    LCD_clear();
    switch (scr) {
        case 1:
        {
            LCD_puts("* Set Time");
            LCD_cursor(0, 1);
            LCD_puts("  Set Alarm");
            break;
        }
        case 2:
        {
            LCD_puts("  Set Time");
            LCD_cursor(0, 1);
            LCD_puts("* Set Alarm");
            break;
        }
        case 3:
        {
            LCD_puts("  Set Alarm");
            LCD_cursor(0, 1);
            LCD_puts("* Temp Mode");
            break;
        }
        case 4:
        {
            LCD_puts("* Set Alarm");
            LCD_cursor(0, 1);
            LCD_puts("  Temp Mode");
            break;
        }
    }
}

void ConfigurationMode() {
    scr = 1;
    print_configuration_screen();
    while (mode == 2) {
        if (BACK == 1) {
            while (BACK);
            mode = 1;
        } else if (UP == 1) {
            while (UP);
            if (scr > 1) {
                if (scr == 3)
                    scr = 4;
                else if (scr == 4)
                    scr = 1;
                else
                    scr--;
            }
            print_configuration_screen();
        } else if (DOWN == 1) {
            while (DOWN);
            if (scr == 4)
                scr == 3;
            else if (scr < 3)
                scr++;
            print_configuration_screen();
        } else if (SELECT == 1) {
            while (SELECT);
            switch (scr) {
                case 1: SetTime(); // Set time
                    break;
                case 2:
                {
                    SetAlarm(); // Set Alarm
                    mode = 1;
                    break;
                }
                case 3: SetTemp();
                    break;
                case 4:
                {
                    SetAlarm();
                    mode = 1;
                    break;
                }
            }
            scr = 1;
            print_configuration_screen();
        }
    }
}

I am attaching screenshots to better help you guys understand the point when issue arises:

Everything fine

Everything is fine until I go inside the SetTime or SetAlarm routine.
enter image description here

As I press UP/DOWN to set time, on each press, a few 7-8 stack overflows occur.
enter image description here

After I come out of the ConfigurationMode() using BACK, the MCU goes haywire and stack overflows start happening in thousands until it crashes.
enter image description here

Best Answer

You can try, in the ConfigurationMode(), combining case 2 and case 4 and instead of calling SetAlarm cut the code and paste it into case 2/4.

Same for case 3 if SetTemp is going to have some code code it into case 3 rather than calling SetTemp(). For now you can remove SetTemp() from case 3.

The same for case 1 move the setTime()

If ConfigurationMode() is called from only one place, move the code where it is called.

The rule being, do not call procedures unless there is a justifiable reason.