Why doesn't the switch command in this code work?

Question

I want to create a clock using a timer (TMR2) to do the calculation and display current time.

I have a problem dealing with the switch statement and couldn’t identify where the problem is. When I run the code in Proteus to test and write the variable sec0 on the screen, I expect it to display 1 to 9 but it keeps on displaying “9” on the LCD? Looks like the switch function is not working. Below is the code I am using and the circuit diagram.

#pragma config FOSC = XT
#pragma config WDTE = OFF
#pragma config PWRTE = OFF
#pragma config BOREN = OFF
#pragma config LVP = OFF
#pragma config CPD = OFF
#pragma config WRT = OFF
#pragma config CP = OFF

#include <xc.h>
#include <stdlib.h>

#define _XTAL_FREQ 4000000
#define rs RA1
#define rw RA2
#define e RA3

int Count = 0;
int sec = 0;
int sec0 = 0;
int sec1 = 0;
int min = 0;
int min0 = 0;
int min1 = 1;
int hr = 0;
int hr0 = 0;
int hr1 = 1;

char name[] = { ' ', 'h', 'r', ' ', 'm', 'i', 'n', ' ', 's', 'e', 'c' };

void initialize();
void lcd_int();
void write();
void LCD_set();

void main()
{
    ADCON1 = 0x00;
    TRISA = 0x00;
    TRISD = 0x00;
    T2CON = 0b01111101;
    PR2 = 0x7F;
    TMR2 = 2;
    GIE = 1;
    PEIE = 1;

    while (1)
    {
        TMR2 = 2;
        while (!TMR2IF);
        TMR2IF = 0;
        Count++;

        if (Count == 125)
        {
            initialize();
            Count = 0;
            sec += 1;
            sec0 = sec % 10;
            sec1 = sec / 10;

            if (sec == 60)
            {
                sec = 0;
                sec0 = sec % 10;
                sec1 = sec / 10;
                min += 1;
                min0 = min % 10;
                min1 = min / 10;
                if (min >= 60)
                {
                    min = 0;
                    min0 = min % 10;
                    min1 = min / 10;
                    hr += 1;
                    hr0 = hr % 10;
                    hr1 = hr / 10;
                    if (hr >= 24)
                    {
                        hr = 0;
                        hr0 = hr % 10;
                        hr1 = hr / 10;
                    }
                }
            }
            write(sec0);
        }
        PORTD = 0xC0;
    }
}

void initialize()
{
    PORTD = 0x01;
    LCD_set();
    PORTD = 0x00;
    LCD_set();
    PORTD = 0x0F;
    LCD_set();
    PORTD = 0x06;
    LCD_set();
    PORTD = 0x80;
    LCD_set();
}

void LCD_set()
{
    rs = 0;
    rw = 0;
    e = 0;
    __delay_ms(100);
    e = 1;
}

void write(int i)
{
    switch (i)
    {
    case 0:
        PORTD = 0b00110000;
    case 1:
        PORTD = 0b00110001;
    case 2:
        PORTD = 0b00110010;
    case 3:
        PORTD = 0b00110011;
    case 4:
        PORTD = 0b00110100;
    case 5:
        PORTD = 0b00110101;
    case 6:
        PORTD = 0b00110110;
    case 7:
        PORTD = 0b00110111;
    case 8:
        PORTD = 0b00111000;
    case 9:
        PORTD = 0b00111001;
    }
    rs = 1;
    rw = 0;
    e = 0;
    __delay_ms(1000);
    e = 1;
}

you could use addition instead of the multiple case statements ... `PORTD = 0b00110000 + i;` — jsotola, Jun 18 '21 at 16:54

score 22 · Answer 1 · answered Jun 18 '21 at 07:35

22

I didn't inspect the whole code but you forgot the "break" statement for each "case" : without it your code will perpetually exit the switch statement by executing "case 9". By the way it is also good to include a "default" statement to handle unexpected inputs.

answered Jun 18 '21 at 07:35

Genorme

701
3
5

thanks very much it working, just learn about the switch statement from internet without realize the usage of break command. – chuackt Jun 18 '21 at 07:40
Good to add a default statement even if not needed - it shows anyone later maintaining your code that you thought of it and accounted for the possibility – Dirk Bruere Jun 18 '21 at 09:36

score 3 · Answer 2 · answered Jun 19 '21 at 10:57

As pointed in the other answer, the missing break in the switch cause the code for case 9 to be executed for all inputs from 0 to 9. Also, the whole switch is equivalent to

PORTD = '0'+i;

which uses that the characters 0123456789 use consecutive values in the dominant encoding, ASCII. That's actually guaranteed by the C standard, IIRC.

If it was necessary to have arbitrary value, e.g. for a 7-segment display, the most appropriate code would not be a switch, but a table lookup.

Why doesn't the switch command in this code work?

2 Answers2