This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Digital clock program using AT892051

Hi, I'm writing a code for digital clock in Keil C, but I'm not getting the expected results. Can anybody please help me with the program? The code is as follows..

#include <AT892051.H>
#include <stdio.h>
#include <intrins.h>

sbit PIN_P10=P1^0;/* Character to be transfered through this bit in parallel*/
sbit PIN_P11=P1^1;/* Clock*/
sbit PIN_P12=P1^2;/* Latch*/
sbit CS4=P1^3; /* 1st 7-segment Display*/
sbit CS3=P1^4; /* 2nd 7-segment Display*/
sbit CS2=P1^5; /* 3rd 7-segment Display*/
sbit CS1=P1^6; /* 4th 7-segment Display*/

unsigned char DataWrite;
unsigned char _crol_(unsigned char Temp,unsigned char DataWrite);
char Display_Buffer[6];
unsigned char Dispcount;
unsigned char mscount;

int Hours=1;
int Minutes=0;
int Seconds=0;
int i=0;
char Temp;

code unsigned char Lookup[11]=
{ 0xEE,0x28,0xCD,0x6C,0x2B,0x67,0xE7,0x2C,0xEF,0x2F
};

void main()
{

TMOD=0x01; TH0=0xF8; /* Initializing timer0*/ TL0=0x30; /*for 1ms*/ IE=0x82; TR0=1;

do
{

sprintf(Display_Buffer,"%02d%02d",Hours,Minutes); /* converting int into char*/

}while(1);

}

void Timer0(void) interrupt 1
{ TF0=0; TR0=0; TH0=0xF8; TL0=0x30; TR0=1;

{ mscount++; Dispcount++; if(mscount>=1000) { mscount=0; Seconds++; } if(Seconds>=59) { Seconds=0; Minutes++; } if(Minutes>=59) { Minutes=0; Hours++; } if(Hours>=12) { Hours=1; }
}

{ if(Dispcount==29) /* This code is for refresh rate*/ { CS1=0; CS2=0; CS3=0; CS4=0; } if(Dispcount==30) {

DataWrite=Lookup[Display_Buffer[0]-0x30];

}

if(8>i) { i++; if (Dispcount==34) { Temp=DataWrite; } if (Dispcount==32) { PIN_P11=1; }

if (Dispcount==36) { PIN_P10=Temp^7; } if (Dispcount==38) { PIN_P11=0; } if (Dispcount==40) { DataWrite=_crol_(Temp,1); }

} PIN_P12=0;

if(Dispcount==48) { CS1=1; }
}

{ if(Dispcount==49) { CS1=0; CS2=0; CS3=0; CS4=0; } if(Dispcount==50) {

DataWrite=Lookup[Display_Buffer[1]-0x30];

}

{

if(8>i)

{ i++; if (Dispcount==54) { Temp=DataWrite; } if (Dispcount==52) { PIN_P11=1; }

if (Dispcount==56) { PIN_P10=Temp^7; } if (Dispcount==58) { PIN_P11=0; } if (Dispcount==60) { DataWrite=_crol_(Temp,1); }

} PIN_P12=0; }

if(Dispcount==68) { CS2=1; }
}

{ if(Dispcount==69) { CS1=0; CS2=0; CS3=0; CS4=0; } if(Dispcount==70) {

DataWrite=Lookup[Display_Buffer[2]-0x30];

}

{

if(8>i) { i++; if (Dispcount==74) { Temp=DataWrite; } if (Dispcount==72) { PIN_P11=1; }

if (Dispcount==76) { PIN_P10=Temp^7; } if (Dispcount==78) { PIN_P11=0; } if (Dispcount==80) { DataWrite=_crol_(Temp,1); }

} PIN_P12=0; }

if(Dispcount==88) { CS3=1; }
}

{ if(Dispcount==89) { CS1=0; CS2=0; CS3=0; CS4=0; } if(Dispcount==90) {

DataWrite=Lookup[Display_Buffer[3]-0x30];

}

{

if(8>i) { i++; if (Dispcount==94) { Temp=DataWrite; } if (Dispcount==92) { PIN_P11=1; }

if (Dispcount==96) { PIN_P10=Temp^7; } if (Dispcount==98) { PIN_P11=0; } if (Dispcount==100) { DataWrite=_crol_(Temp,1); }

} PIN_P12=0; }

if(Dispcount==108) { CS4=1; }
}

if(Dispcount==821) { CS1,CS2,CS3,CS4=0;

}
} RETI;

Parents
  • The code is hard to read. Both because it wasn't correctly posted, and because you have made it very complicated.

    If you program in C - why do you then think you should have an assembler instruction somewhere in the code? Have you ever seen any Keil C program with a RETI for an interrupt handler? Please in that case show a link.

    You have off-by one errors - how will you get your clock to display the 7:59:59? When secnds reaches 59, you reset to zero - should wait to 60. Same for the other variables.

    You seem to have multiple blocks:

    if (8>i) {i++; ...}
    


    How many times can i be incremented in a single visit to your ISR?

    What is the full range you expect Dispcount to run to until restarted? You have Dispcount declared as unsigned char but are trying to store values way outside the numeric range. And should Dispcount really be overflowing? I would have thought that the display output should happen once/second, meaning your state machine should restart every 1000ms.

    And it isn't easy to read the code when you have many if() statements processing Dispcount in varying order - sometimes a lower value gets tested after a higher value.

    When you test an integer for a large number of different values, there is a specific language construct - switch statement - available. It was added for a purpose. Can you guess what purpose?

    There are other errors too. You really have not spent much time looking at your code and tried to single-step it in your head. You post to a forum to ask for help unless you have already made sure that you have caught all the easy errors? Because you think that a prfessional developer who have this as occupation would send out all his code on forums to find someone to catch all the errors? But who would the legendary/magicall beings be who reads forum posts and catches trivial errors if the developers themselves don't spend time catching them?

    Another thing - if this clock is battery-operated, it would consume 100% CPU power and hence a lot of batteries by the main loop that is constantly converting the individual time components into a string. Is it meaningful to try to build this string hundreds or thousands of times every second?

Reply
  • The code is hard to read. Both because it wasn't correctly posted, and because you have made it very complicated.

    If you program in C - why do you then think you should have an assembler instruction somewhere in the code? Have you ever seen any Keil C program with a RETI for an interrupt handler? Please in that case show a link.

    You have off-by one errors - how will you get your clock to display the 7:59:59? When secnds reaches 59, you reset to zero - should wait to 60. Same for the other variables.

    You seem to have multiple blocks:

    if (8>i) {i++; ...}
    


    How many times can i be incremented in a single visit to your ISR?

    What is the full range you expect Dispcount to run to until restarted? You have Dispcount declared as unsigned char but are trying to store values way outside the numeric range. And should Dispcount really be overflowing? I would have thought that the display output should happen once/second, meaning your state machine should restart every 1000ms.

    And it isn't easy to read the code when you have many if() statements processing Dispcount in varying order - sometimes a lower value gets tested after a higher value.

    When you test an integer for a large number of different values, there is a specific language construct - switch statement - available. It was added for a purpose. Can you guess what purpose?

    There are other errors too. You really have not spent much time looking at your code and tried to single-step it in your head. You post to a forum to ask for help unless you have already made sure that you have caught all the easy errors? Because you think that a prfessional developer who have this as occupation would send out all his code on forums to find someone to catch all the errors? But who would the legendary/magicall beings be who reads forum posts and catches trivial errors if the developers themselves don't spend time catching them?

    Another thing - if this clock is battery-operated, it would consume 100% CPU power and hence a lot of batteries by the main loop that is constantly converting the individual time components into a string. Is it meaningful to try to build this string hundreds or thousands of times every second?

Children
  • Thanks Mr.Westermark for your thoughts on my program. The thing is that I'm new to Keil C, so finding it difficult in the beginning to cope up with it.
    As you said, the shouldn't be used in this program?
    And is it that my variable minutes, seconds must have 60 counts even if i use (Minutes>=59)?
    I don't know how many times i can be incremented in the ISR. It must be incremented only once? if yes then where in particular in my program.
    and my last question, what should be Dispcount declared as?

  • So, this is a school assignment, then?

    "The thing is that I'm new to Keil C"

    But your school knows that - and they expect that, at this point in your course, you should have learned enough 'C' (most of it is not Keil-specific) to enable you to complete this task.

    Perhaps it's time to review your course notes, textbook, etc...?