From 79b75860bcec108ec4cd3fcdbed63e1c866cd769 Mon Sep 17 00:00:00 2001 From: ian Date: Tue, 4 Jan 2005 21:55:57 +0000 Subject: [PATCH] more comments --- cebpic/nmra-stream.asm | 18 ++++++++++++++++-- detpic/nmra-stream.asm | 18 ++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/cebpic/nmra-stream.asm b/cebpic/nmra-stream.asm index 00e2005..97716aa 100644 --- a/cebpic/nmra-stream.asm +++ b/cebpic/nmra-stream.asm @@ -289,6 +289,10 @@ read_from_buffer movwf FSR1H,0 ; set high byte of IND1 pointer movff INDF1,W,0 andwf TOTRACKBIT,0,0 ; mask out bit to be transmitted +;*** to `mask out' means to clear, eg +;*** if you were to `mask out bit 0 of 0xff' you'd get 0xfe +;*** I suggest `test bit to be transmitted' or `check bit ...' or maybe +;*** `select bit ...' -iwj bz zero_bit_to_track bra one_bit_to_track @@ -306,10 +310,12 @@ one_bit_to_track advance_bit -; rotate tranmitbit to next position +; rotate transmitbit to next position rrncf TOTRACKBIT,1,0 ; rotate mask right - +;*** surely rrnc (`rotate right not through carry' I assume) +;*** will leave a copy of the top bit in the N flag ? Then you +;*** can use branch if negative. -iwj btfss TOTRACKBIT,7,0 call advance_pointer @@ -327,6 +333,7 @@ advance_pointer movwf FSR1H,0 movff INDF1,W,0 andlw 0x80 +;*** bit test bit 7 of INDF1 ? That's simpler than load into W and AND. -iwj bnz advance_read_buffer bra advance_read_byte @@ -334,6 +341,8 @@ advance_pointer advance_read_buffer +;*** I suggest swapping this and advance_read_byte round, since +;*** advance_read_byte is a more `inner' bit of the `loop' -iwj ; move pointer to next buffer @@ -359,6 +368,7 @@ totrack_overflow ; write na=cb=1 and return bra one_bit_to_track +;*** oh, we always introduce a one bit in between ? *confused* -iwj @@ -376,9 +386,13 @@ advance_read_byte movwf FSR1H,0 ; set high byte of IND1 pointer movff INDF1,W,0 andwf TOTRACKBIT,0,0 ; mask out bit to be transmitted +;*** This is the same code as the 2nd stanza in read_from_buffer, +;*** above. Surely it should be made common (eg a subroutine) ? -iwj bz zero_bit_to_track bra one_bit_to_track +;*** zero_bit_to_track and one_bit_to_track end up calling advance_bit - +;*** is that right ? *confused* -iwj diff --git a/detpic/nmra-stream.asm b/detpic/nmra-stream.asm index 00e2005..97716aa 100644 --- a/detpic/nmra-stream.asm +++ b/detpic/nmra-stream.asm @@ -289,6 +289,10 @@ read_from_buffer movwf FSR1H,0 ; set high byte of IND1 pointer movff INDF1,W,0 andwf TOTRACKBIT,0,0 ; mask out bit to be transmitted +;*** to `mask out' means to clear, eg +;*** if you were to `mask out bit 0 of 0xff' you'd get 0xfe +;*** I suggest `test bit to be transmitted' or `check bit ...' or maybe +;*** `select bit ...' -iwj bz zero_bit_to_track bra one_bit_to_track @@ -306,10 +310,12 @@ one_bit_to_track advance_bit -; rotate tranmitbit to next position +; rotate transmitbit to next position rrncf TOTRACKBIT,1,0 ; rotate mask right - +;*** surely rrnc (`rotate right not through carry' I assume) +;*** will leave a copy of the top bit in the N flag ? Then you +;*** can use branch if negative. -iwj btfss TOTRACKBIT,7,0 call advance_pointer @@ -327,6 +333,7 @@ advance_pointer movwf FSR1H,0 movff INDF1,W,0 andlw 0x80 +;*** bit test bit 7 of INDF1 ? That's simpler than load into W and AND. -iwj bnz advance_read_buffer bra advance_read_byte @@ -334,6 +341,8 @@ advance_pointer advance_read_buffer +;*** I suggest swapping this and advance_read_byte round, since +;*** advance_read_byte is a more `inner' bit of the `loop' -iwj ; move pointer to next buffer @@ -359,6 +368,7 @@ totrack_overflow ; write na=cb=1 and return bra one_bit_to_track +;*** oh, we always introduce a one bit in between ? *confused* -iwj @@ -376,9 +386,13 @@ advance_read_byte movwf FSR1H,0 ; set high byte of IND1 pointer movff INDF1,W,0 andwf TOTRACKBIT,0,0 ; mask out bit to be transmitted +;*** This is the same code as the 2nd stanza in read_from_buffer, +;*** above. Surely it should be made common (eg a subroutine) ? -iwj bz zero_bit_to_track bra one_bit_to_track +;*** zero_bit_to_track and one_bit_to_track end up calling advance_bit - +;*** is that right ? *confused* -iwj -- 2.30.2