[PIC] Problem with coding for "digital clock" (pic16f877a,20Mhz,LCD)

manojkl · Aug 22, 2015

LCD doesn't display the time!
command to display variable in lcd?

Code:

 // LCD module connections
sbit LCD_RS at RD2_bit;
sbit LCD_EN at RD3_bit;
sbit LCD_D4 at RD4_bit;
sbit LCD_D5 at RD5_bit;
sbit LCD_D6 at RD6_bit;
sbit LCD_D7 at RD7_bit;

sbit LCD_RS_Direction at TRISD2_bit;
sbit LCD_EN_Direction at TRISD3_bit;
sbit LCD_D4_Direction at TRISD4_bit;
sbit LCD_D5_Direction at TRISD5_bit;
sbit LCD_D6_Direction at TRISD6_bit;
sbit LCD_D7_Direction at TRISD7_bit;
// End LCD module connections

volatile int     //VOLATILE statement used to change
                 //value after the optimisation aslo
change=0,
count=0,
sec=0,           //declaring the variable
minu=0,
hr=0;
 void timer_init()       //initialisation of timer1 module
 {
  TMR1H=TMR1L=0;         //clearing 16 bit register to start from first itself
  T1CON.T1CKPS1=1;      //choosing the 1:4 prescalr
  T1CON.T1CKPS0=0;
  T1CON.T1OSCEN=0;    //Don"t use LP oscillator
  T1CON.TMR1CS=0;     //Use microcontroller internal clock for counting the pulse
  PIE1.TMR1IE=1;      //Interrupt enable pin of the timer1 module
  T1CON.TMR1ON=1;     //Enable timer1 ON
  INTCON.PEIE=1;      //Enable peripheral interrupt enable bit
  INTCON.GIE=1;       //Enable Global interrupt enable bit
 }
    //Interrupt Service routine(ISR)
     void Interrupt_ISR()
{
    if(PIR1.TMR1IF==1)   //check really interrupt is fired by the timer1 module
   {
    count++;              //increase count by one(count++=(count=count+1))
     PIR1.TMR1IF=0;       //clear the interrupt flag
    }
 }
  //display the clock on the LCD
  void displayclock()
 {

    //HOURS
    if(hr<10)
    {
     LCD_out(1,5,"0");          //move the cursor to 1st row 5th column
     _LCD_MOVE_CURSOR_RIGHT;
     Lcd_Out(1,6,hr);
     _LCD_MOVE_CURSOR_RIGHT;
     }
     else
     {
     LCD_out(1,5,hr);
     _LCD_MOVE_CURSOR_RIGHT;
     }
     LCD_out(1,7,":");
     _LCD_MOVE_CURSOR_RIGHT;
     //MINUTE
      if(minu<10)
    {
     LCD_out(1,8,"0");          //move the cursor to 1st row 8th column
     _LCD_MOVE_CURSOR_RIGHT;
     Lcd_Out(1,9,minu);
     _LCD_MOVE_CURSOR_RIGHT;
     }
     else
    {
     LCD_out(1,8,minu);
     _LCD_MOVE_CURSOR_RIGHT;
     }
       LCD_out(1,10,":");
     _LCD_MOVE_CURSOR_RIGHT;
        //SECONDS
      if(sec<10)
    {
     LCD_out(1,11,"0");          //move the cursor to 1st row 11th column
     _LCD_MOVE_CURSOR_RIGHT;
     Lcd_Out(1,12,sec);
     _LCD_MOVE_CURSOR_RIGHT;
     }
     else
    {
     LCD_out(1,11,sec);
     _LCD_MOVE_CURSOR_RIGHT;
     }
  }
    int main()
   {
      LCD_init();  //Initialise the LCD display
      //Set the initial time
      hr=18;
      minu=34;
      sec=10;

      timer_init(); //initialise the timer1
      displayclock();
       while(1)
      {
        if(count==19)
        {
          sec++;
          count=0;
          change=1;
         }
         if(sec==60)
        {
          minu++;
          sec=0;
          change=1;
         }
         if(minu==60)
        {
          hr++;
          minu=0;
          change=1;
         }
         if(hr==24)
         {
          hr=0;
          minu=0;
          sec=0;
          }
             // if current time has changed, update the clock
          if(change) 
          {
           displayClock();
           change = 0;
           }
       }
    }

pic.programmer · Aug 22, 2015

You are using mikroC PRO PIC Compiler. Would you mind zipping and posting the complete mikroC PRO PIC project files and Proteus file ?

manojkl · Aug 22, 2015

here it is.

pic.programmer · Aug 22, 2015

I have re-written the code completely. I don't know what timer interrupt (delay) you used. I have used 100 ms timer interrupt. So, 10 x 100ms timer interrupt is equal to 1 sec.

The forum is not allowing me to attach files. Hence I am posting the code. Compile the code for 20 MHz HS Oscillator.

Code C - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
sbit LCD_RS at RD2_bit;
sbit LCD_EN at RD3_bit;
sbit LCD_D4 at RD4_bit;
sbit LCD_D5 at RD5_bit;
sbit LCD_D6 at RD6_bit;
sbit LCD_D7 at RD7_bit;
 
sbit LCD_RS_Direction at TRISD2_bit;
sbit LCD_EN_Direction at TRISD3_bit;
sbit LCD_D4_Direction at TRISD4_bit;
sbit LCD_D5_Direction at TRISD5_bit;
sbit LCD_D6_Direction at TRISD6_bit;
sbit LCD_D7_Direction at TRISD7_bit;
// End LCD module connections
 
char count = 0, sec = 10, minute = 34, hr = 18;
 
//Timer1
//Prescaler 1:8; TMR1 Preload = 3036; Actual Interrupt Time : 100 ms
//Place/Copy this part in declaration section
void InitTimer1() {
    T1CON = 0x31;
    TMR1IF_bit = 0;
    TMR1H = 0x0B;
    TMR1L = 0xDC;
    TMR1IE_bit = 1;
    INTCON = 0xC0;
}
 
void Interrupt() {
    if(TMR1IF_bit) {
        TMR1IF_bit = 0;
        TMR1H = 0x0B;
        TMR1L = 0xDC;
        //Enter your code here
        ++count;
        if(count == 10) {
              ++sec;
              if(sec == 60) {
                   ++minute;
                   if(minute == 60) {
                         ++hr;
                         if(hr == 24) {
                              hr = 0;
                         }
                         minute = 0;
                   }
                   sec = 0;
              }
              count = 0;
        }
    }
}
 
void main() {
 
    TRISA = 0xC0;
    TRISB = 0x00;
    TRISC = 0x00;
    TRISD = 0x00;
    TRISE = 0x00;
    
    PORTA = 0x00;
    PORTB = 0x00;
    PORTC = 0x00;
    PORTD = 0x00;
    PORTE = 0x00;
    
    LCD_Init();
    LCD_Cmd(_LCD_CURSOR_OFF);
    LCD_Cmd(_LCD_CLEAR);
    
    InitTimer1();
    
    while(1) {
 
          LCD_Chr(1,1,hr/10 + 48);
          LCD_Chr(1,2,hr%10 + 48);
          LCD_Chr(1,3,':');
          LCD_Chr(1,4,minute/10 + 48);
          LCD_Chr(1,5,minute%10 + 48);
          LCD_Chr(1,6,':');
          LCD_Chr(1,7,sec/10 + 48);
          LCD_Chr(1,8,sec%10 + 48);
    }
}

Aussie Susan · Aug 24, 2015

pic.programmer: In the OP's version, they (correctly) make all of the variables that were updated in the ISR 'volatile' but the code presented in #4 above does not. This is a backwards step.
Also, the OP initialised the timer in a way that is self-documenting but the code above relies on 'magic numbers' and is therefore much harder to understand, debug and maintain. Further, the accepted wisdom is to completely initialise a peripheral before enabling it (unless the data sheet tells you otherwise) but you have gone against this. Again a backwards step.
The OP's code only updated the LCD when the time changed which would allow the program to be extended (or made a separate function) later on to do other things. Your code updates the LCD continuously even when nothing has altered. I consider this a backwards step.
I have no idea of your code works and I accept that the OPs question is not very well asked (it does not say what works, exactly what doesn't etc. so it is hard to understand the actual problem). However, from a learning perspective of the OP, simply re-writing it might give him something to submit for an assignment (bit assumption on my part) but does not really teach him anything.
If the OP can describe his actual problem better, then perhaps we can all help him understand where he went wrong so he can write better code in the future.
Susan

xenos · Aug 24, 2015

The capacitors in the crystal section, should not be 1nF.

I see that the compiler you use is not case sensitive in your project (this is not the problem).

I am not using (and not going to use) microC, so I am not familiar with the syntax of the LCD_Out function, but I doubt that the third parameter can be either character array or an integer (not even ascii) value.
This is a C++ feature.

In other words, you have a lot of calls on LCD_out in the displayclock() function,
most of them looks like:

Code:

LCD_out(1,11,"0"); //move the cursor to 1st row 11th column

and the rest them looks like:

Code:

Lcd_Out(1,12,sec);

Check the Lcd_out declaration and use it accordingly.

pic.programmer · Aug 24, 2015

@xenos

Yes. He was using the LCD_Out() function improperly.

Code:

Lcd_Out(1,12,sec);

LCD_Out() third argument should be a string. He was passing integer value.

@Susan

pic.programmer: In the OP's version, they (correctly) make all of the variables that were updated in the ISR 'volatile' but the code presented in #4 above does not. This is a backwards step.

In my code the sec, minute, hr variables only update inside ISR and it is not modified anywhere else. So, I have not used volatile.

Also, the OP initialised the timer in a way that is self-documenting but the code above relies on 'magic numbers' and is therefore much harder to understand, debug and maintain. Further, the accepted wisdom is to completely initialise a peripheral before enabling it (unless the data sheet tells you otherwise) but you have gone against this. Again a backwards step.

Above the InitTimer1() function definition the comment says taht Timer1 is configured for 100 ms interrupt.
So, I have used a variable count and 10 timer1 interrupts are counted to get 1 sec. If my InitTimer() function is harder to read then excuse me please because I have used mikroE Timer Calculator Tool to get that piece of code.

The OP's code only updated the LCD when the time changed which would allow the program to be extended (or made a separate function) later on to do other things. Your code updates the LCD continuously even when nothing has altered. I consider this a backwards step.

I can easily add a flag which is set whenever 1 sec is elapsed and update the LCD once every sec.

I have no idea of your code works and I accept that the OPs question is not very well asked (it does not say what works, exactly what doesn't etc. so it is hard to understand the actual problem). However, from a learning perspective of the OP, simply re-writing it might give him something to submit for an assignment (bit assumption on my part) but does not really teach him anything.
If the OP can describe his actual problem better, then perhaps we can all help him understand where he went wrong so he can write better code in the future.

I think the OPs problem is solved and he wont be back until he gets another probelm.

Aussie Susan · Aug 25, 2015

pic.programmer said:
In my code the sec, minute, hr variables only update inside ISR and it is not modified anywhere else. So, I have not used volatile.

That is the whole point of 'volatile' - to tell the compiler that it cannot assume that the value of a variable not alter.
Normally the compiler believes that it can see all of the relevant code so that if it copies a value from a variable to a register, then the value in the register correctly reflects the value of the variable unless the compiler issues instructions to alter one or both. When the value CAN be altered in an ISR, the compiler needs to be told that the assumption is not valid and that it must refer back to the variable value every time.
For example, consider the (hypothetical) code:

Code:

while(2 != hr){}   // Wait until the 2nd hour

Without begin told that 'hr' is volatile, the compiler is completely within its rights to generate code that copies whatever the 'hr' value is to a register and then repeatedly check the value of the register - in fact this is exactly what many compilers will do. Further, some levels of optimisation will let the compiler "know" that the 'hr' variable was initialised to soem other value and therefore optimise out the entire loop.
In your code, the compiler could detect that none of the 'hr', 'minute' or 'sec' variables are altered in the main while loop and store all of them in registers for rapid access. Therefore the displayed value will never change.

pic.programmer said:
Above the InitTimer1() function definition the comment says taht Timer1 is configured for 100 ms interrupt.
So, I have used a variable count and 10 timer1 interrupts are counted to get 1 sec. If my InitTimer() function is harder to read then excuse me please because I have used mikroE Timer Calculator Tool to get that piece of code.

That was not my point. Your code enables the timer before you set the other configuration values. With some devices you can get away with this but others will not operate as you expect. This is a major source of programming bugs.
If the code was automatically generated, then this should be a lesson to check the boilerplate code is valid in the circumstances where you use it.
Susan

Welcome to EDAboard.com

[PIC] Problem with coding for "digital clock" (pic16f877a,20Mhz,LCD)

manojkl

Newbie level 5

Attachments

pic.programmer

Advanced Member level 3

manojkl

Newbie level 5

Attachments

pic.programmer

Advanced Member level 3

manojkl

Aussie Susan

Advanced Member level 5

bigdogguru

manojkl

xenos

Full Member level 4

bigdogguru

pic.programmer

Advanced Member level 3

Aussie Susan

Advanced Member level 5

bigdogguru

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Connect with us

Online statistics

Forum statistics