Jon,

I promised to comment on your interrupt-driven serial program and I will also comment on mine later if time permits.

My comments in red.

Of course, I am nothing of perfect etc. so these are only my ideas. Arguable, of course, all of them.

Jan Waclawek

;******************************************************************** 
;******************************************************************** 
CR            EQU    0DH 
LF            EQU    0AH 
              ; 
TXBUFSZ      EQU    2             ;Define buffer sizes 
2 bytes long Tx buffer... Not really worth, is 
it? See also comment on TXBUFSZ in the interrupt routine itself.
RXBUFSZ      EQU    8 
              ;                    ;Indexes 
RX_IN         EQU    08H           ;RX next to go in 
RX_OUT       EQU    09H           ;RX next to go out 
TX_IN         EQU    0AH           ;TX next to go in 
TX_OUT       EQU    0BH           ;TX next to go out 
RXBUF         EQU    0CH           ;Allow RXBUFSZ to next variable 
TXBUF         EQU    14H           ;Allow TXBUFSZ to next variable 
the definition of variables using EQU is not 
the best idea; if you need to squeeze in some other variable or 
change the sizes of buffers, it would require a lot of work. It 
is better to use DB or similar pseudoinstructions intended to 
reserve space for variables; for buffers (with already defined 
size) use DB TXBUFSZ .
              ; 
CHR           EQU    16H 
End of variables in RAM -> the initial value of
 stack should come here (see MOV SP,#17h below; should be like 
MOV SP,#SPINIT and SPINIT definition should come here). Again, 
the idea is to have as much job done by the assembler itself 
automatically, when juggling with the variables. 

              ; 
NEEDTI       BIT    01H           ;Clear it if TI=1 is needed 
              ;                    ;Set if TI=1 is not needed 
;******************************************************************** 
       ORG    0h 
       LJMP   MAIN                 ;Reset vector 
       ; 
;****************************************************************** 
       ORG    23h  
       LJMP   SER_ISR             ;Serial port interrupt vector 
       ; 
;******************************************************************** 
       ORG    40h 
       ; 
MAIN:  MOV    SP,#17h             ;Initialize stack pointer 
see comment on initial value of stack ponter above
       LCALL  SERINIT             ;Initialize serial port 
       SETB   EA                   ;Enable all interrupts 
       MOV    DPTR,#SINIT         ;Serial Port Initialized! 
       LCALL  TEXT_OUT            ;Send the message out 
; 
LOOP:                              ;Repeating loop 
       LCALL  GETCHR 
       MOV    A,R1 this
       CPL    A    and this
       JZ     LOOP and this
can be written as one instruction: CJNE R1,#0FFh,LOOP
Anyway; IMHO it is not the best idea to indicate a valid 
received character in this way - see comment below in GETCHR routine
       LCALL  PUTCHR 
       AJMP   LOOP                 ;Loop back 
; 
It would be nice to have a comment here, like: "the program here waits
for any received character and echoes it back". But, is this a good example for 
interrupt-driven serial communication? It can be done in the same way
also using polling... Some other, more complicated, example would be fine.

;******************************************************************** 
A comment should come here, saying something like: 
"this is a string output routine, something like PRINT in BASIC or 
"printf" in C or "write" in Pascal. Sends out the string pointed
to by DPTR, terminated by zero.
TEXT_OUT: 
       CLR    A                    ;dptr has message 
       MOVC   A,@A+DPTR           ;get the next character 
       INC    DPTR                 ;move index to next position 
       JZ     WT2                  ;if zero then end of message 
       MOV    R1,A                 ;else put the chr in the tx buffer 
       LCALL  PUTCHR 
;############################### 
Now THIS (a delay loop) has nothing to do here! 
It indicates some FATAL problem with interrupt-driven communication. The point 
of using interrupts was not to delay the "main" program by the serial routines, 
but this is contradicting it.
Correctly, the  transmitting routine should check for empty space in the 
transmit buffer and if available, put the character otherwise wait. There is a 
buffer empty space check in PUTCHR, but it only fails to put the character and 
THIS routine is suposed to check whether it failed and retry.
In this way, if the string is shorter than the transmit buffer, it would be
placed into the buffer immediately without any delay, and the main program could
move on while the buffer would be transmitted "on the background" by the interrupt
routine - and that's the whole point of interrupt driven communication.
       mov    r3,#2                ;Temporary delay loop 
LP1:   mov    r2,#225             ;to prevent buffer overrun 
       djnz   r2,$ 
       djnz   r3,LP1 
;############################### 
       AJMP   TEXT_OUT 
A note on the preferencial use of particular form of JMP: 
if the jump is explicitly given (not derived by assembler from the genericv JMP),
in short self-contained routines such as this the SJMP is preferred rather than
AJMP (if the length of jump permits it of course). The reason is, if this routine
gets shifted somewhere on the 2k page boundary, AJMP would fail 
while SJMP would not.
WT2:   RET 
; 
;******************************************************************** 
;                                  ;R1 has character 
PUTCHR: 
       SETB   C                    ;using C to store state of EA 
       JBC    EA,PCSKP            ;if interrupts enabled, then disable 
       CLR    C                    ;EA was already disabled 
Ehm... what about... MOV C,EA ?
       ; 
PCSKP:       PUSH   PSW           ;store current status, including EA 
... but not MOV C,EA is needed; why not PUSH IE ?
       MOV    CHR,R1              ;store the character 
there is NO NEED to store the character in CHR - R1 does not
get corrupted until the character is really stored in the buffer!
       CLR    C 
       MOV    A,TX_IN 
       SUBB   A,TX_OUT 
INCORRECT! This code attempts to get the occupied space in buffer
and compare it against the buffer size (below) to get the empty space.
However, it completely misses the fact that in circular buffer the "head" pointer
(TX_IN) may get BELOW the "tail" pointer (TX_OUT)! Although this implementation 
of circular buffer is - ehm - nonstandard (the pointers are NOT the actual
pointers to the buffer - see below) and the described problem will occur less
frequently than in the "normal" implementation (it takes 256 bytes to transmit for 
the "wraparound" of the TX_IN/TX_OUT to occur), it is still not a proper
way to go.
       CLR    C 
       SUBB   A,#TXBUFSZ          ;TXBUFSZ = 2 
       JC     TBUFOK              ; 
       MOV    R1,#0FFh            ;else error RETURN VALUE -1 
       AJMP   PCXIT                ;buffer full  return 
        ; 
TBUFOK: 
       MOV    A,TX_IN 
       ANL    A,#TXBUFSZ-1       ;TXBUFSZ =2-1=1 0 through 1 = 2 
Now this is a method which I object.
The pointers don't properly wrap around at the end of buffer; rather, 
their topmost bits are masked out when calculating the real address.
It assumes that the buffer sizes are in power of two (should be added as a note
at the buffer length declaration).
It also prevents to use the real addresses as pointers, so they the address
has to be calculated each time.
Maybe it works, but I don't like it. I could imagine working with something 
similar, but not in this form - I would keep the PROPER address as pointer,
and I would mask it after incrementing - it requires exact positioning
of the buffer but I think this is more appropriate than this approach.
       ADD    A,#TXBUF            ;# of characters in buffer+buffer address 
       MOV    R0,A                 ;location in buffer 
       MOV    @R0,CHR             ;put character in buffer 
       INC    TX_IN                ;move index 
       JNB    NEEDTI,SKPTI       ;exit if no more characters in buffer 
       CLR    NEEDTI              ;else more to send 
       SETB   TI                   ;interrupt and send them 
SKPTI: 
       MOV    R1,#00h             ;done - return value 0 
PCXIT: 
       POP    PSW                  ;restore and return 
       MOV    EA,C 
       RET 
; 
;******************************************************************** 
GETCHR: 
       SETB   C                    ;use C to store the state of EA 
       JBC    EA,GCSKP            ;if all enabled then disable 
       CLR    C                    ;all were disabled already 
GCSKP: 
       PUSH   PSW                  ;store the current state 
The same applies for storing EA as above...
       ; 
       CLR    C 
       MOV    A,RX_IN             ;how many characters in buffer? 
       SUBB   A,RX_OUT 
       JNZ    RBUFNZ              ;jump if buffer not empty 
the four above lines merge into 
MOV A,RX_OUT
CJNE A,RX_IN,RXBUFNZ
       MOV    R1,#0FFh            ;else buffer is empty return value -1 
       AJMP   GCXIT                ;exit with buffer empty code 
       ; 
RBUFNZ: 
       MOV    R1,RX_OUT           ;next to get out 
       INC    RX_OUT              ;move index 
       MOV    A,R1 
There is no need to move RX_OUT into R1 first, 
then move it from R1 into A
       ANL    A,#RXBUFSZ-1       ;RXBUFSZ = 8-1 =7 0 through 7 = 8 
       ADD    A,#RXBUF            ;# of characters in buffer+buffer address 
       MOV    R0,A                 ;R0 has buffer location 
       MOV    A,@R0                ;get chr from buffer 
       MOV    R1,A                 ;put chr in R1 
GCXIT: 
       POP    PSW                  ;restore previous state 
       MOV    EA,C                 ;restore previous interrupt state 
       RET                         ;return 
; 
;******************************************************************** 
SERINIT:                          ;Initialize serial port 
       CLR    A 
       MOV    TX_IN,A             ;initialize indexes 
       MOV    TX_OUT,A 
       MOV    RX_IN,A 
       MOV    RX_OUT,A 
       CLR    TR1 
       CLR    TI 
       CLR    RI 
       MOV    SCON,#01011010B    ;TI SET INDICATES TRANSMITTER READY. 
                                   ;MODE 1 REN 
       MOV    TMOD,#00100001B    ;TIMER 1 MODE 8 BIT AUTO RELOAD 
        MOV    TH1,#0FDh           ;TIMER RELOAD VALUE 
       SETB   TR1                  ;START TIMER 
       SETB   NEEDTI 
       SETB   ES 
       RET 
; 
SINIT: 
       DB     CR,LF 
       DB     'Serial Port Initialized! ' 
CRLF:  DB     CR,LF,0 
;******************************************************************** 
SER_ISR: 
       PUSH   ACC                  ;store current state 
       PUSH   PSW 
       MOV    PSW,#00h 
       PUSH   00h 
I think this deserves a comment - this is storing R0 from 
register bank 0, and MOV PSW,#0 ensures, that the isr will really work with
register bank 0...

       JNB    RI,TXISR            ;if not receive then check xmit 
       CLR    RI                   ;else clear RI and process 
       CLR    C 
       MOV    A,RX_IN 
       SUBB   A,RX_OUT            ;how many characters in buffer? 
       ANL    A,#0-RXBUFSZ       ; 
Argh... Why this mask??? No need even for that twisted pointer
scheme...
       JNZ    TXISR                ;jump if no characters in buffer 
FATAL! The receiver refuses to receive further characters if 
there is already one character in buffer - they got lost!!!
       MOV    A,RX_IN             ;else  
       ANL    A,#RXBUFSZ-1       ;number of characters in buffer 
       ADD    A,#RXBUF            ;plus buffer address = 
       MOV    R0,A                 ;buffer position 
       MOV    @R0,SBUF            ;put received character in buffer 
       INC    RX_IN                ;increment buffer position 
; 
TXISR: 
       JNB    TI,ISRXIT           ;if TI=0 then exit 
       CLR    TI                   ;else clear TI and process 
       MOV    A,TX_IN             ;how many characters in buffer? 
       XRL    A,TX_OUT 
       JZ     TXBUFZ              ;jump if buffer empty 
MOV A,TX_OUT; CJNE A,TX_IN,TXBUFZ and the following line 
can be omitted
       MOV    A,TX_OUT            ;else 
       ANL    A,#TXBUFSZ-1       ;number of characters in buffer 
       ADD    A,#TXBUF            ;plus buffer address = 
       MOV    R0,A                 ;buffer position 
       MOV    A,@R0                ;get character from buffer 
       MOV    SBUF,A              ;send character out 
       INC    TX_OUT              ;increment buffer position 
       CLR    NEEDTI              ;need to set TI 
       AJMP   ISRXIT              ;exit ISR 
TXBUFZ: 
       SETB   NEEDTI              ;buffer was empty dont need to set TI 
ISRXIT: 
       POP    00h                  ;restore and return 
       POP    PSW 
       POP    ACC 
       RETI 
;******************************************************************** 
;********************************************************************