>On Wed, Mar 07, 2001 at 11:52:40AM -0500, Dan Michaels wrote:
>> James Cameron wrote:
>> >Looking for archived discussions or sample code to adjust for consistent
>> >drift in my PIC 16F877's 4MHz crystal oscillator.
>> .......
>> >The problem is that my clock is losing one second every three hours.  It
>> >is at least consistently doing this.  It is well within specifications
>> >for the crystal.
>> Your error is ~1/3600, while xtal error is <= 50/1000000 = 1/20000,
>> so the problem is no doubt in your code. 4 mhz is not an even power
>> of 2, so your method of adjusting for timer overflows probably needs
>> to be fixed. Look there.
>
>Ah, maybe I've got one of those off-by-one problems.  I checked most
>carefully ... TMR0 at 1:256, should give one T0IF every 65536uS exactly,
>which I add into a 24-bit counter and then subtract one second.  If the
>result is positive, I keep the new counter value and activate my 1Hz
>code path.  TMR0 runs free, never reset.  Does this logic seem sound?
>
>http://quozl.netrek.org/uptime/uptime.asm is the code.
>http://quozl.netrek.org/uptime/ has the rest of the code.
>

> Hi James,
>
> The only thing that I wasn't sure about was the lines:
>
>  ;; subtract 1s from copy of counter
>  movlw d'1000000'&0xff
>  subwf t0,f
>  btfss status,c
>  decf t1,f
>
> This is an eight bit subtract of a 24 bit value, but you are not bringing
> the carry to the most significant byte ("t2").  I haven't done any kind of
> analysis on what this can do to you, but I'm sure every once in a while it
> will bite you.
>
> I would suggest something like:
>
>  ;; subtract 1s from copy of counter
>  movlw d'1000000'&0xff
>  subwf t0,f
>  btfsc status,c
>   goto $ + 5  ;; If No Borrow, Subtract from the Middle Byte
>  movlw d'1'
>  subwf t1,f
>  btfss status,c
>   decf t2,f ;;  "t1" was Zero upon Entry into Subtraction Routine, Decrement
> t2

> If you wanna do this why not do it like:
> ....
>
>   movlw d'1000000'&0xff
>   subwf t0,f
>   movlw 1
>   skpnc
>    subwf t1,f
>   skpnc
>    subwf t2,f

> myke predko wrote:
>
> > This is an eight bit subtract of a 24 bit value, but you are not
> > bringing the carry to the most significant byte ("t2").
>
> and Scott Dattalo <PICLISTEraseME.....MITVMA.MIT.EDU> replied:
>
> > If you wanna do this why not do it like:
> > ....
> >
> >   movlw d'1000000'&0xff
> >   subwf t0,f
> >   movlw 1
> >   skpnc
> >    subwf t1,f
> >   skpnc
> >    subwf t2,f
>
> Scott:
>
> After reading the recent "commenting practices" thread that was
> sparked by Myke Predko's use, with no explanatory comments, of the
> FSR as a generic temporary variable (and worse, if you recall, he
> used the temporary variable in ALMOST the same way that the FSR is
> usually used), I think I'd rewrite your code like this:
>
>     LIST    P=16C54,R=DEC
>
>     MOVLW   1<<6        ;No need for "&0xff", since 1000000 = 1E6.
>     SUBWF   T0,TMR0     ;TMR0 has just overflowed.
>     MOVLW   F           ;W register = F (just F, not 0x0F).
>     BTFSC   STATUS,W    ;Skip if C (just C, not 0x0C) = W.
>     SUBWF   T1,W+1      ;(W register = F still, but W = F-1).
>     BTFSC   STATUS,INDF ;We're using INDF and FSR together
>     SUBWF   T2,(FSR-3)  ; here, but not in the usual way.
>
> This code assembles identically to yours... But since it's fully
> commented, it's clearly better.

> After reading the recent "commenting practices" thread that was
> sparked by Myke Predko's use, with no explanatory comments, of the
> FSR as a generic temporary variable (and worse, if you recall, he
> used the temporary variable in ALMOST the same way that the FSR is
> usually used), I think I'd rewrite your code like this:
>
>     LIST    P=16C54,R=DEC
>
>     MOVLW   1<<6        ;No need for "&0xff", since 1000000 = 1E6.
>     SUBWF   T0,TMR0     ;TMR0 has just overflowed.
>     MOVLW   F           ;W register = F (just F, not 0x0F).
>     BTFSC   STATUS,W    ;Skip if C (just C, not 0x0C) = W.
>     SUBWF   T1,W+1      ;(W register = F still, but W = F-1).
>     BTFSC   STATUS,INDF ;We're using INDF and FSR together
>     SUBWF   T2,(FSR-3)  ; here, but not in the usual way.
>
> This code assembles identically to yours... But since it's fully
> commented, it's clearly better.
>
> -Andy