ok heres a correction

   if(timet[minbug]==68)if(secs==16)
   {timet[minbug]=0;secs++;}

 

I was simply trying to help you learn programming. No need to get so hyped up.

the deviation is 1 second every 4096

That's so off the mark that I would suggest, as I did earlier, that you go back and read the data sheet more thoroughly.

The bugs that u pointed out is quite common amongst people who are beginning to code those devices.

Unfortunately for you, you picked a very difficult device to code.

 

I was simply trying to help you learn programming. No need to get so hyped up.

So you expect a yes ,yes sir, Im all wrong? I just cant verify what you say. You refuse to show your source, but compare some program "X" to mine.

That's so off the mark that I would suggest, as I did earlier, that you go back and read the data sheet more thoroughly.

I did. This is the result.

a) can you explain the deviation with my settings?
Can you answer if its just timer or also prescaler?
Finally, a test is required to see if theres deviation remaining.
If not, your saying would have little point.

Thats what physics are about, postulates, theories, research, verification.
We have been told so in college.
If they know everything from a theory book why testing is required so much?
Its quite normal.

The bugs that u pointed out is quite common amongst people who are beginning to code those devices.

I dont understand the grammar, frankly. "The bugs" now youre giving out, instead process one problem after the other. How do you know my programming abilities?
I wrote 8086 assembler some larger sources. At this time I was a beginner.
Then I learned PIC RISC. I regret digging into it so much.
Started with embedded C 5 years ago or so. I get along with any PIC, tried all, upto PIC32, SD card, USB.

Thanks anyway.

Unfortunately for you, you picked a very difficult device to code.

[/quote]

I really appreciate you take the time to reply. Im not trolling. I really want to improve my C skills.
Used the 16f54 for some years. But yes, its always a challenge anew.

I really want to process all the problems until the script is acceptable.
Including testing.

If you have a better script please show it.

I choose this IC since theres not so much prerequisite, small short datasheet, few configuration options.
The c script will be manageable on a public forum!

I did scrolling message on 16F57. Its not really good I use the 16F1709 now a lot.
I did LCD 3 digit display and I2C with 16f59. You name it, for RTC.

Sure Im always willing to learn, correct my mistakes,
This is why I post my script on a forum.

 

Started with embedded C 5 years ago or so.

There are lots of long tenured beginners.

 

There are lots of long tenured beginners.

Ok but I only wish to process problems one by one, improve c skills. Your replies in this regard are most welcome.

If you reference MISRA fluency as professional then I don't feel like going into it. I see consumer electronics failing all day including the softwares so it's nothing special.

When you see a problem write a solution don't just mock it. I want a led clock that's working for normal purposes, that's pretty much it.

Actually the timer latency bothered me for a long time never needed total accuracy with 16f54 so far.

The other bug or bugs you didn't mention.

Solutions please.

 

Solutions please.

I think you will learn far more by doing it yourself.

Trust me, the bugs are so simple that even a beginner can spot it by reading the datsheet - I will make it easier by saying that it is in the timer section.

With your years of experience coding pics, you should be able to identify it from miles away.

 

Here is the same code running on your wiring scheme. I can produce a .hex file for your chip if you want to play around with it.

All I had to do is to change the wiring definitions and recompile,

#define DIG1_PORT                   PORTB
#define DIG1_DDR                    TRISB
#define DIG1                        (1<<0)

//Pin11/SegA on RA3
#define SEGA_PORT                   PORTA
#define SEGA_DDR                    TRISA
#define SEGA                        (1<<3) 

//Pin10/SegF on RA2
#define SEGF_PORT                   PORTA
#define SEGF_DDR                    TRISA
#define SEGF                        (1<<2) 

//Pin9/Dig2 on RA1
#define DIG2_PORT                   PORTA
#define DIG2_DDR                    TRISA
#define DIG2                        (1<<1) 

//Pin8/Dig3 on RB7
#define DIG3_PORT                   PORTB
#define DIG3_DDR                    TRISB
#define DIG3                        (1<<7)

//Pin7/SegB on RA0
#define SEGB_PORT                   PORTA
#define SEGB_DDR                    TRISA
#define SEGB                        (1<<0) 

//Pin6/Dig4 on RB6
#define DIG4_PORT                   PORTB
#define DIG4_DDR                    TRISB
#define DIG4                        (1<<6)

//Pin5/SegG on RB5
#define SEGG_PORT                   PORTB
#define SEGG_DDR                    TRISB
#define SEGG                        (1<<5) 

//Pin4/SegC on RB4
#define SEGC_PORT                   PORTB
#define SEGC_DDR                    TRISB
#define SEGC                        (1<<4) 

//Pin3/SegDP on RB3
#define SEGDP_PORT                  PORTB
#define SEGDP_DDR                   TRISB
#define SEGDP                       (1<<3) 

//Pin2/SegD on RA2
#define SEGD_PORT                   PORTB
#define SEGD_DDR                    TRISB
#define SEGD                        (1<<2) 

//Pin1/SegE on RA1
#define SEGE_PORT                   PORTB
#define SEGE_DDR                    TRISB
#define SEGE                        (1<<1)

So in the future, if someone changes the wiring scheme again, the code is just a recompilation away from working again. That's the beauty of not hard-wiring your code.

 

Outstanding really.

Something you miss

The tables can be put into eeprom. The defines can't.

Showing some incomplete fragment.

A few bugs they're so outstanding anyone could spot them.

Well when It shows and keeps time, you know how much I give about it. That's the mission.

For this you need ram buffering or its too slow. You found a better or faster way. But keep it secret

 

heres the update including time setting functionality.

For this purpose you must hold the pushbutton (theres only one) at time of startup.
Depending on the time of day (or night), probably for several minutes.

So ive tested the 24 hour overflow, works.

By the way I made this PCB because I needed some in circuit debugging displays, and why wire up these boring 7seg displays all over again, use resistors even. Some day I was so annoyed about a LED matrix I made, it was so dull, I tried without resistors.

This was a circuit using shifting registers. Then some day I tried with a PIC directly as well.
Some day I cooked one in boiling water, no prob.

I dont know 6 or 7 years ago. Actually I have no clock or wristwatch so I really need this one to use every day, when I dont want to search the phone or turn on laptop.

GOTO instruction, yes, I dont care. I wanted a no frills LED clock which is small, works, doesnt make noise or contain a radio (I had many of these).

For sure a serial debugging display is a beginners cobbling "work".

If you look the STM8 capacitive touch libaries maybe its not beginners work. All sources are shown.
Hide your museum pieces now?

Some "sellers" bought the lowcost dev boards from Farnell etc., put on ebay for MUCH higher price.
I havent checked if these are still on trade for inflated prices but for sure its some kind of abuse.
Infringement.

This thread isnt about whats reasonable markup and what not, or what is "unwelcome buying".

I did look at STM controllers, got some dev boards, but considered changeover too costly.

#include  <xc.h> // 32 KHZ LEd 7 segment clock
#pragma config OSC = LP  WDT = OFF CP = OFF // configuration 16F54

const unsigned char dig_PORTA[] @0x01a ={0b1111,0b0011,0b1011,0b1011,\
0b0111,0b1110,0b1110,0b1011,\
0b1111,0b1111,0b1111,0b0110,\
0b1110,0b0011,0b1010,0b1010,\
0b1101,0b0001,0b1001,0b1001,\
0b0101,0b1100,0b1100,0b1001,\
0b1101,0b1101,0b1101,0b0100,\
0b1100,0b0001,0b1000,0b1000,\
0b1111,0b0011,0b1011,0b1011,\
0b0111,0b1110,0b1110,0b1011,\
0b1111,0b1111,0b1111,0b0110,\
0b1110,0b0011,0b1010,0b1010,\
0b1111,0b0011,0b1011,0b1011,\
0b0111,0b1110,0b1110,0b1011,\
0b1111,0b1111,0b1111,0b0110,\
0b1110,0b0011,0b1010,0b1010}; // segment and sink driving table for PORTA

const unsigned char dig_PORTB[] @0x60 ={0b11010110,0b11010000,0b11100110,0b11110100,\
0b11110000,0b11110100,0b11110110,0b11110000,\
0b11110110,0b11110100,0b11110010,0b11100110,\
0b11000110,0b11010110,0b11110110,0b11100110,\
0b11010111,0b11010001,0b11100111,0b11110101,\
0b11110001,0b11110101,0b11110111,0b11110001,\
0b11110111,0b11110101,0b11110011,0b11100111,\
0b11000111,0b11010111,0b11110111,0b11100111,\
0b01010111,0b01010001,0b01100111,0b01110101,\
0b01110001,0b01110101,0b01110111,0b01110001,\
0b01110111,0b01110101,0b01110011,0b01100111,\
0b01000111,0b01010111,0b01110111,0b01100111,\
0b10010111,0b10010001,0b10100111,0b10110101,\
0b10110001,0b10110101,0b10110111,0b10110001,\
0b10110111,0b10110101,0b10110011,0b10100111,\
0b10000111,0b10010111,0b10110111,0b10100111}; // segment and sink driving table for PORTB

unsigned char* tptr;
unsigned char v_phase,v_digphase,secs;
unsigned char ddata[8];
unsigned char timet[5];
#define hrh 0
#define hrl 1
#define minh 2
#define minl 3
#define minbug 4

void update() // precompute the display data, load from ROM table
{unsigned char i,i2;
for(i=0;i<4;i++)
{
  i2=timet[i]+(i<<4);
  *(tptr+i+i)=dig_PORTA[i2];
  *(tptr+1+i+i)=dig_PORTB[i2];
}
}

void display() // update the multiplex display
{
TRISA=0xff;TRISB=0xff;
PORTA=*(tptr+v_digphase);
PORTB=*(tptr+1+v_digphase);
TRISA=0;TRISB=0;
v_phase+=0x10;v_phase&=0x30;
v_digphase+=2;v_digphase&=0x07;
}

void minplus()
{
secs=0;
timet[minl]++;
timet[minbug]++;
if(timet[minl]==10)
{
  timet[minl]=0;if(++timet[minh]==6)
  {
   timet[minh]=0;
   if(++timet[hrl]==10){timet[hrh]+=1;timet[hrl]=0;}
   if(timet[hrl]==4)if(timet[hrh]==2){timet[hrh]=0;timet[hrl]=0;}
  }
}  
}

void secplus()
{
TMR0&=0x1f;
*(tptr+3)^=0x8; // blink rhe LED
secs++;
if(timet[minbug]==68)if(secs==16)
{timet[minbug]=0;secs++;}
}

void main(void)
{
v_phase=0;v_digphase=0; //initialization
secs=0;timet[minbug]=0;
timet[minl]=0;timet[minh]=0;timet[hrl]=0;timet[hrh]=0;
TRISB=0;TRISA=0;OPTION=0x07;
tptr=&ddata[0];
update(); // display 00.00

sett_reloop:;
TRISB=0x02;
if((PORTB&0x02)==0)
{
  TMR0=0;TRISB=0;
  while(TMR0<0x2)display();
  minplus();update();TMR0=0;
  goto sett_reloop;
}else TRISB=0;

while(1)
{
  if(TMR0&0x20) secplus(); else if(secs==60)
  {minplus();update();}  
  display();      
}}

 

One thing you have to be careful of when substituting an '84 for a '54 is the smaller stack on the latter. Just for grins, I coded a clock to @takao21203 's specs using a 16F54 mainly to see how XC8 did with the low-end processors. I had to be careful in the table lookups due to to the miserable 2 level stack but basic timekeeping/muxed display with 32KHz XTAL sims OK. Display IO is re-mappable at compile time as are output levels for CC/CA displays, 12/24hour time etc. It compiles like this in XC8 Free mode:

Memory Summary:
Program space used C1h ( 193) of 200h words ( 37.7%)
Data space used Eh ( 14) of 19h bytes ( 56.0%)
EEPROM space None available
Data stack space used 0h ( 0) of Bh bytes ( 0.0%)
Configuration bits used 1h ( 1) of 1h word (100.0%)
ID Location space used 0h ( 0) of 4h bytes ( 0.0%)

You have compiled in FREE mode.
Using Omnicient Code Generation that is available in PRO mode,
you could have produced up to 60% smaller and 400% faster code.
See http://www.microchip.com/MPLABXCcompilers for more information.

make[2]: Leaving directory 'M:/UP/PIC/Clock54/Clock54.X'
make[1]: Leaving directory 'M:/UP/PIC/Clock54/Clock54.X'

BUILD SUCCESSFUL (total time: 1s)
Loading code from M:/UP/PIC/Clock54/Clock54.X/dist/ReleasePM3/production/Clock54.X.production.hex...
Loading completed

Unfortunately, when I went to program the chip, MPLAB bricked my PM3 so I was not able to test it on actual hardware. I wanted to do that before adding the Time Set functions but have to wait to get the PM3 up again. If you are still interested, I'll proceed (since I wired a breadboard). Otherwise you can have the code as is if you want. It's a decent example of baseline PIC coding and a reminder of why you wouldn't do it if you had a choice. The only reason I took a shot at it was because I just ported some client code from the C54 to the F54 for cost/availability reasons and had some chips laying around.

 

The fundamental flaw in the code is writing to TMR0, especially when a prescaler is involved.

The challenge is to detect roll-over on a chip that doesn't provide hardware support for that. Yes, it is doable.

 

I pointed this earlier in the thread but it is worth repeating again: the OP's code will NOT maintain accurate timing.

The simulation below shows that it is off 0.044s for every 10 seconds, or over 6 minutes off every 24 hours.

note: the simulation was done by setting the break point on the statement that increments secs, on the 10th increment (after ignoring the 1st increment to eliminate the impact of the initialization code).

 

As shown in the code above, I also implemented a mechanism to detect TMR0 overflow (through 0x20), through the TMR0_OVF() macro. You will notice that my code does not write to TMR0.

Because of that, each iteration will take slightly different duration to finish, sometimes 0.999633789 second, and sometimes 1.006714 seconds. However, over the long term, they will maintain the correct timing - for every 18.333 execution of 0.999633789 runs, you get one run of 1.006714.

The chart below shows the first 10 runs (it contains one 1.006714 execution). With a timing error of +0.003418 second for every 10 seconds.

 

The following is the continuation of the previous simulation. It shows a -0.00024 second of error for a 20 second run.

ie., the code is correcting itself to maintain long-term timing accuracy.

 

If you could walk away from this with one lesson, it is "do not write to TMR0" if you care about long-term timing accuracy.

Bugs like

TMR0&=0x1f;

kill any hope of getting the timing right. If you want to develop a clock or a long-duration timer, don't do that.

 

In the above simulation, I only tested 10 seconds and 20 seconds, respectively.

I was asked what if I ran the code a little bit longer. Here are some results, with pictures to provide:

Duration: Error (s) / Error (ppm)
10 s: +0.03418s /341ppm
20 s: -0.00024s / -12ppm
100s: -0.02197s / -219ppm
150s: -0.000244s / - 2ppm
200s: +0.01709s / 85ppm
279s: +0.000122s/ 0ppm

So hopefully it shows that the timing errors from individual runs are not accumulating. ie., the algorithm maintains excellent accuracy over the long-run.

 

Some simulation results for the code posted by OP earlier:

Duration: error(s) / error(ppm)
50s: 0.219727s / ~4300ppm
100s: 0.441528s / ~4400ppm
150s: 0.66333s / ~4400ppm
200s: 0.885132s / ~4400ppm
226s: 0.9994s / ~4400ppm
227s: 1.003784s / ~4400ppm
250s: 1.106934s / ~4400ppm

So the timing errors are cumulative and goes over 1 second about 227seconds after start. -> that translates into about 382 seconds / ~6 minutes off every 24 hours.

Here is a screen shot at 250s -> showing 251s.

enjoy.

 

Thanks your replies

So ive programmed a workaround. Mask out the lower 5 bits.
Keep the previous value and check if any of bits #5 to #7 have changed.
Dont need to write to TMR0 then.

Doable? What is so special about this

if((TMR0&0b11100000)!=tmrx)secplus();

and inside secplus()

tmrx=TMR0&0b11100000;

Correcting once one second has accumulated also can do the job but it adds unneccessary code.
If the value I computed would be wrong change to the correct one.

But its not required when testing if any of the upper bits have changed, compared to previous value of these.

Dont nail me on comments the c source wasnt finished, probably now, it is.
Checked for some minutes, rollover seems to be synchronous.

#include  <xc.h> // 32 KHZ LED 7 segment clock, V1.00 - 72% F:ASH used
#pragma config OSC = LP  WDT = OFF CP = OFF // configuration 16F54

const unsigned char dig_PORTA[] @0x01a ={0b1111,0b0011,0b1011,0b1011,\
 0b0111,0b1110,0b1110,0b1011,\
 0b1111,0b1111,0b1111,0b0110,\
 0b1110,0b0011,0b1010,0b1010,\
 0b1101,0b0001,0b1001,0b1001,\
 0b0101,0b1100,0b1100,0b1001,\
 0b1101,0b1101,0b1101,0b0100,\
 0b1100,0b0001,0b1000,0b1000,\
 0b1111,0b0011,0b1011,0b1011,\
 0b0111,0b1110,0b1110,0b1011,\
 0b1111,0b1111,0b1111,0b0110,\
 0b1110,0b0011,0b1010,0b1010,\
 0b1111,0b0011,0b1011,0b1011,\
 0b0111,0b1110,0b1110,0b1011,\
 0b1111,0b1111,0b1111,0b0110,\ // 16 values for each multiplex phase
 0b1110,0b0011,0b1010,0b1010}; // segment and sink driving table for PORTA

const unsigned char dig_PORTB[] @0x60 ={0b11010110,0b11010000,0b11100110,0b11110100,\
 0b11110000,0b11110100,0b11110110,0b11110000,\
 0b11110110,0b11110100,0b11110010,0b11100110,\
 0b11000110,0b11010110,0b11110110,0b11100110,\
 0b11010111,0b11010001,0b11100111,0b11110101,\
 0b11110001,0b11110101,0b11110111,0b11110001,\
 0b11110111,0b11110101,0b11110011,0b11100111,\
 0b11000111,0b11010111,0b11110111,0b11100111,\
 0b01010111,0b01010001,0b01100111,0b01110101,\
 0b01110001,0b01110101,0b01110111,0b01110001,\
 0b01110111,0b01110101,0b01110011,0b01100111,\
 0b01000111,0b01010111,0b01110111,0b01100111,\
 0b10010111,0b10010001,0b10100111,0b10110101,\
 0b10110001,0b10110101,0b10110111,0b10110001,\
 0b10110111,0b10110101,0b10110011,0b10100111,\
 0b10000111,0b10010111,0b10110111,0b10100111}; // segment and sink driving table for PORTB

unsigned char* tptr; // pointer to display buffer
unsigned char v_phase,v_digphase,secs,tmrx;
unsigned char ddata[8]; // array for display buffer
unsigned char timet[4]; // array for time
#define hrh 0
#define hrl 1
#define minh 2
#define minl 3

void update() // precompute the display data, load from ROM table
{unsigned char i,i2;
 for(i=0;i<4;i++)
 {// RAM buffer holds values for PORTA and PORTB for each multiplex phase
  i2=timet[i]+(i<<4);  
  *(tptr+i+i)=dig_PORTA[i2];
  *(tptr+1+i+i)=dig_PORTB[i2];
 }
}

void display() // update the multiplex display
{
 TRISA=0xff;TRISB=0xff;// required to turn off PORTs because ghosting
 PORTA=*(tptr+v_digphase);
 PORTB=*(tptr+1+v_digphase);  
 TRISA=0;TRISB=0;// turn on again
 v_digphase+=2;v_digphase&=0x07;// increment and mask out upper 5 bits same as rollover at 0x08
}

void minplus()
{// 24 hours clock logic
 secs=0;
 timet[minl]++;
 if(timet[minl]==10)
 {
  timet[minl]=0;if(++timet[minh]==6)
  {
   timet[minh]=0;
   if(++timet[hrl]==10){timet[hrh]+=1;timet[hrl]=0;}
    if(timet[hrl]==4)if(timet[hrh]==2){timet[hrh]=0;timet[hrl]=0;}
  }
 }    
}

void secplus()
{
 tmrx=TMR0&0b11100000; //mask out lower 5 bits
 *(tptr+3)^=0x8; // blink rhe LED  
 secs++;
}

void main(void)
{
 tmrx=0;v_digphase=0;secs=0;
 timet[minl]=0;timet[minh]=0;timet[hrl]=0;timet[hrh]=0;
 TRISA=0;OPTION=0x07;TMR0=0;
 tptr=&ddata[0]; // clear variables
 update(); // display 00.00

 sett_reloop:;// pushbutton logic to set time at startup
 TRISB=0x02;
 if((PORTB&0x02)==0)
 {// pushbutton was hold down, enter
  TMR0=0;TRISB=0;
  while(TMR0<0x2)display();// small delay and increment minutes
  minplus();update();TMR0=0;
  goto sett_reloop;// reloop until button is released
 }else TRISB=0;

 while(1)
 {// check if any of the 3 upper bits have changed = 1 sec elapsed
  if((TMR0&0b11100000)!=tmrx)secplus();if(secs==60){minplus();update();}    
  display();// multiplex display from RAM buffer        
 }}

 

One thing you have to be careful of when substituting an '84 for a '54 is the smaller stack on the latter. Just for grins, I coded a clock to @takao21203 's specs using a 16F54 mainly to see how XC8 did with the low-end processors. I had to be careful in the table lookups due to to the miserable 2 level stack but basic timekeeping/muxed display with 32KHz XTAL sims OK. Display IO is re-mappable at compile time as are output levels for CC/CA displays, 12/24hour time etc. It compiles like this in XC8 Free mode:
Unfortunately, when I went to program the chip, MPLAB bricked my PM3 so I was not able to test it on actual hardware. I wanted to do that before adding the Time Set functions but have to wait to get the PM3 up again. If you are still interested, I'll proceed (since I wired a breadboard). Otherwise you can have the code as is if you want. It's a decent example of baseline PIC coding and a reminder of why you wouldn't do it if you had a choice. The only reason I took a shot at it was because I just ported some client code from the C54 to the F54 for cost/availability reasons and had some chips laying around.

So you say with your much smaller program code, the display isnt flickering?
The table reads are very slow, RAM read via pointer is fast.
In addition you need to care individual sink bits, do 10 divison perhaps.

 

Dont need to write to TMR0 then.

I'm happy for you that you finally got it, after ~40 back-and-forth.