0
\$\begingroup\$

I'm fairly new to programming PIC Microcontrollers, but am getting real stuck into it and loving it. Today my little project was to make a 7 segment display count from 0 to 9, multiplexing each segment as the numbers progressed.

I ended up with this huge chunk of code. Now since I'm fairly new to this, I'm 100% sure there is a better way to make a 7 segment display count, using multiplexing, because I just bogged together what I knew. I got timers and loops and idk stuff all over the place - it works though. I'd love to put everything into folders, make it neat and make sure my code is as efficient as it gets (and not have function declared implicit warnigs in the build!)

Please help a brother out!

Thanks.

Here is what main.c looked like:

(definitions.h contains _XTAL_FREQ and PORTbits, and config.h contains config bits)

#include <xc.h>
#include "config.h"
#include "definitions.h"

void delay(void)
{
        INTCONbits.T0IF = 0;        //Clear the Timer 0 interrupt flag
        TMR0 = 0b11111010;          
        INTCONbits.T0IE = 1;        //Enable the Timer 0 interrupt

        while(INTCONbits.T0IF == 0) //Wait for the interrupt to occur. This
        {                           //happens when the TMR0 register rolls over.
            NOP();
        }
}

int main(void)
{
    TRISD=0x00;

    OPTION_REGbits.PSA = 0;     //Prescaler assigned to Timer 0 (other option is to
                                //the Watchdog timer (WDT))

    OPTION_REGbits.PS = 0b111;  //Set the prescaler to 1:256
    OPTION_REGbits.T0CS = 0;    //Use the instruction clock (Fcy/4) as the timer
                                //clock. Other option is an external oscillator
                                //or clock on the T0CKI pin.

    while(1)
    {
        no0();
        __delay_ms(1);
        no1();
        __delay_ms(1);
        no2();
        __delay_ms(1);
        no3();
        __delay_ms(1);
        no4();
        __delay_ms(1);
        no5();
        __delay_ms(1);
        no6();
        __delay_ms(1);
        no7();
        __delay_ms(1);
        no8();
        __delay_ms(1);
        no9();
        __delay_ms(1);
    }
}

int no0()
{
    unsigned int a;

    for(a=0;a<100;a++)
        {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=1;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=1;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=1;
        delay();
        }
}

int no1()
{
    unsigned int b;

    for(b=0;b<300;b++)
        {
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
        }
}

int no2()
{
    unsigned int c;

    for(c=0;c<120;c++)
    {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=1;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=1;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=1;
        delay();
    }
}

int no3()
{
    unsigned int d;

    for(d=0;d<120;d++)
    {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=1;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=1;
        delay();
    }
}

int no4()
{
    unsigned int e;

    for(e=0;e<150;e++)
    {
        SEG1=0;
        SEG2=1;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=1;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();       
    }
}

int no5()
{
    unsigned int f;

    for(f=0;f<120;f++)
    {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=1;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=1;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=1;
        delay();
    }
}

int no6()
{
    unsigned int g;

    for(g=0;g<100;g++)
    {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=1;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=1;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=1;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=1;
        delay();
    }
}

int no7()
{
    unsigned int h;

    for(h=0;h<200;h++)
    {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
    }
}

int no8()
{
    unsigned int i;

    for(i=0;i<86;i++)
    {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=1;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=1;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=1;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=1;
        delay();
    }
}

int no9()
{
    unsigned int j;

    for(j=0;j<100;j++)
    {
        SEG1=1;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=1;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=1;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=1;
        SEG5=0;
        SEG6=0;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=1;
        SEG7=0;
        delay();
        SEG1=0;
        SEG2=0;
        SEG3=0;
        SEG4=0;
        SEG5=0;
        SEG6=0;
        SEG7=1;
        delay();
    }
}
\$\endgroup\$
2
\$\begingroup\$

There are a few things that I can see straight away that I would do - but I feel this is more of a general programming question.

You have lots of code repeats

This fragment:

SEG1=1;
SEG2=0;
SEG3=0;
SEG4=0;
SEG5=0;
SEG6=0;
SEG7=0;

appears again and again, along with others like it. Put it in its own function and call it where necessary. This will help you two ways - one is that your code will be easier to read, and secondly if you wish to change this fragment you only have to do it in one place. You could also put this function into an external included file.

You have magic numbers in your code

for(a=0;a<100;a++)

What if you want to display the number for a longer or shorter duration of time? Make the repeat number variable, and either pass it to the function or (because it varies by the number of segments you are illuminating) pass a unit of time (for example milliseconds) you wish to delay to the function and calculate the loop count in the function.

Your code is not well commented

How long does your delay function run for? What about when you come back to this code in 2 months time? Or a year?

You sit stuck idling in delays a lot

This is a bigger question, but your code is not very portable to use it something like a clock (for example) because it can't respond to external events. I would set up a timer linked to an interrupt service routine (ISR) which ticked at intervals I defined. I would implement a state machine with all the behaviour I needed and update this machine at each ISR tick. For example, you could have a state machine which took two inputs - the number of ticks (which you increment each time the ISR runs) and the seven-segment number to output. Every time you check the machine, depending on these two you decide which segment to illuminate and then return - not wasting time sat in the delay loop. This would allow you to respond to events like button presses etc in your main loop, and update the display only via the ISR.

I realise that this last point is a big leap and uses a couple of programming techniques you may not be familiar with. I do suggest making yourself familiar with all of the following (these are Wikipedia links that introduce the general principles - you will want to seek out more information and examples that are more specific to the type of processor you are using).

Finite State Machine

Interrupt Service Routine

\$\endgroup\$
  • \$\begingroup\$ Thanks for the tips. From your first point, say to display the number zero, how would you put the fragment into its own function? The same way I put the numbers into their functions? Also, is it possible to put these functions into another file and link them in (instead of having them all inside main.c)? And correct me if i'm wrong, but for the last point you're saying set up an ISR based timer to allow the device to be open to input, instead of using the delay loop? Thanks again. \$\endgroup\$ – ezra_vdj Sep 6 '15 at 6:41
  • \$\begingroup\$ Yes - the function works the same way as other functions you have written. You can call functions from within other functions too, provided they are within scope (try Googling C scope for more info on this). Yes you can link external files, using the same #include you are already using for other files. Take a look at this Stack Overflow answer for an example. And yes, that is what I am suggesting with the ISR technique. \$\endgroup\$ – stefandz Sep 6 '15 at 8:47

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.