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();
}
}
Best Answer
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:
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
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