Assembly A86 - Get and Display Time

Question

I am working on an Assembly program to get system time and date, convert it to ASCII, and display it on the monitor. I am having trouble getting it to display properly and cannot find where I've gone wrong. This is for an assignment, and I'd rather have explanations than just solutions, if possible. Here is my code:

TITLE GETDTTM
PAGE 60, 132

;   This program retrieve the system date and time,
;   converts it to ASCII, and displays it to the screen

;   Define constants
;
CR      EQU 0DH ;define carriage return
LF      EQU 0AH ;define line feed
EOM     EQU '$' ;define end of message marker
NULL    EQU 00H ;define NULL byte
;
;   Define variables
;
JMP START
PROMPT  DB  CR, LF, "The current time is: ",EOM
PROMPT2 DB  CR, LF, "The date is: ",EOM
TIME    DB  "00:00:00", CR, LF, EOM
DATE    DB  "00/00/0000", CR, LF, EOM
;
;   Program code
;
START:  
CALL GET_TIME   ;call function to get system time
CALL GET_DATE   ;call function to get system date

LEA DX, PROMPT  ;print time prompt to screen
MOV AH, 09H
INT 21H

LEA DX, TIME    ;print time
MOV AH, 09H
INT 21H

LEA DX, PROMPT2 ;print date prompt to screen
MOV AH, 09H
INT 21H

LEA DX, DATE    ;print date
MOV AH, 09H
INT 21H


CVT_TIME:   ;converts the time to ASCII
CALL CVT_HR
CALL CVT_MIN
CALL CVT_SEC
RET

CVT_HR:
MOV BH, CH  ;copy contents of hours to BH
SHR CH,4    ;convert high char to low order bits
ADD CH, 30H ;add 30H to convert to ASCII
MOV [TIME], CH  ;save it
AND BH, 0FH ;isolate lower 4 bits
ADD BH, 30H ;convert to ASCII
MOV [TIME+1], BH    ;save it
RET

CVT_MIN:
MOV BH, CL  ;copy contents of minutes to BH
SHR CL, 4   ;convert high char to low order bits
ADD CL, 30H ;add 30H to convert to ASCII
MOV [TIME+3], CL    ;save it
AND BH, 0FH ;isolate lower 4 bits
ADD BH, 30H ; convert to ASCII
MOV[TIME+4], BH ;save it

CVT_SEC:
MOV BH, DH  ;copy contents of seconds to BH
SHR DH, 4   ;convert high char to low order bits
ADD DH, 30H ;add 30H to convert to ASCII
MOV [TIME+6], DH    ;save it
AND BH, 0FH ;isolate lower 4 bits
ADD BH, 30H ;convert to ASCII
MOV[TIME+7], BH ;save it

GET_DATE:   ;get date from the system
    MOV AH, 04H    ;BIOS function to read date
    INT 1AH        ;call to BIOS, run 04H
    CALL CVT_DATE
    RET
;CH = Century
;CL = Year
;DH = Month
;DL = Day
;CF = 0 if clock is running, otherwise 1

CVT_DATE:
    CALL CVT_MO
    CALL CVT_DAY
    CALL CVT_YR
    CALL CVT_CT
    RET

CVT_MO:     ;convert the month to ASCII
MOV BH, DH  ;copy month to BH
SHR BH, 4   ;convert high char to low order bits
ADD BH, 30H ;add 30H to convert to ASCII
MOV [DATE], BH  ;save in DATE string
MOV BH, DH  ;copy month to BH
AND BH, 0FH ;isolate lower 4 bits
ADD BH, 30H ;convert lower bits to ASCII
MOV [DATE+1], BH;save in DATE string
RET

CVT_DAY:    ;convert the day to ASCII
MOV BH, DL  ;copy days to BH
SHR BH, 4   ;convert high char to low order bits
ADD BH, 30H ;add 30H to convert to ASCII
MOV [DATE+3], BH    ;save in DATE string
MOV BH, DL  ;copy days to BH
AND BH, 0FH ;isolate lower 4 bits
ADD BH, 30H ;convert lower bits to ASCII
MOV [DATE+4], BH;save in DATE string
RET

CVT_YR:     ;convert the year to ASCII
MOV BH, CL      ;copy year to BH
SHR BH, 4       ;convert high char to low order bits
ADD BH, 30H     ;convert to ASCII
MOV [DATE+8], BH    ;save it
MOV BH, CL      ;copy year to BH
AND BH, 0FH     ;isolate low order bits
ADD BH, 30H     ;convert to ASCII
MOV [DATE+9], BH    ;save in DATE string
RET

CVT_CT:     ;convert the century to ASCII
MOV BH, CH      ;copy century to BH
SHR BH, 4       ;convert high char to low order bits
ADD BH, 30H     ;convert to ASCII
MOV [DATE+6], BH    ;save it
MOV BH, CH      ;copy century to BH
AND BH, 0FH     ;isolate low order bits
ADD BH, 30H     ;convert to ASCII
MOV [DATE+7], BH    ;save it
RET
;
;Program End
;

End

And here's what I get when I run it at 9:11AM on 2/19/2015:

The current time is: 09:00:00
The date is: 02/09/0005

I've tried to add lots of comments of my intentions, so that you can get an idea of what I'm trying to do and easier see if there's some kind of logic error. I think it's pretty clear from the output that I'm missing getting my minutes and seconds into TIME and have some ideas on how to fix that, but after noon, I get some weird times, and I'm confused as to what's happening to my date. Any help is much appreciated.

Edit: Got time to work by splitting it up and actually dealing with minutes and seconds... whoops. Now my output is as follows:

Run at 9:23AM on 02/19/2015

The current time is: 09:23:02
The date is: 02/09/0005

EDIT2: Getting closer! Thanks for the [DATE] catch - I fixed that and am getting correct month and day values, and closer on year values. Figured out I wasn't shifting far enough since year is 4 characters long - 16 bits, not 8! - so I couldn't get the whole thing by only SHR 4 bits! My output now looks like:

The current time is: 09:43:02
The date is: 02/19/0015

EDIT 3: Added CVT_CT to convert the century to ASCII and add it to the [DATE] string, but am still getting the same output...

The current time is: 10:06:02
The date is: 02/19/0015

EDIT 4: I forgot to add a call to my new function... Wow. Working now!!! Thank you all for your help!

The current time is: 10:09:02
The date is: 02/19/2015

Side question: Any idea why the seconds would always be 02?

Solution

All those individual (but very similar) functions for converting BCD into characters are somewhat messy and near guarantee you'll mess up some minor thing, such as forgetting to preserve registers when you may not the values in them later on.

If you're interested in avoiding this, look into the DRY (don't repeat yourself) principle (as opposed to WET (write everything twice). The Wikipedia page for DRY is a good start.

---

If you spend some time thinking about what can be moved to common code (i.e., refactoring), you'll end up with far less code to worry about, and therefore far less opportunity for bugs to sneak in.

The prime example in your case is the code that takes each BCD value and creates two characters from it. This consumed about forty lines of actual code (and that's just for the date bit, I assume there would have been another thirty-odd lines for the time, had you shown that).

If you look at the code below, you'll see I've refactored this out into put_bcd2 for a total of thirteen lines of code - even if you bump that up to twenty-seven because of the extra lines needed to call it, that's still a massive reduction. This greatly simplifyies both the code that does the conversion and the code that uses it.

; Main program.

    call    get_date            ; get date/time into string.
    call    get_time

    lea     dx, output          ; then output the string.
    mov     ah, 09h
    int     21h

    mov     ax, 4c00h           ; exit program.
    int     21h

; Variables.

output:
    db      "The current date is: "
date:
    db      "00/00/0000", 0dh, 0ah
    db      "The current time is: "
time:
    db      "00:00:00", 0dh, 0ah, '$'

; Subroutines.

; Gets the date and inlines it into the output.
get_date:
    mov     ah, 04h             ; get date from bios.
    int     1ah

    mov     bx, offset date     ; do day.
    mov     al, dl
    call    put_bcd2

    inc     bx                  ; do month.
    mov     al, dh
    call    put_bcd2

    inc     bx                  ; do year.
    mov     al, ch
    call    put_bcd2
    mov     al, cl
    call    put_bcd2

    ret

; Gets the time and inlines it into the output.
get_time:
    mov     ah, 02h             ; get time from bios.
    int     1ah

    mov     bx, offset time     ; do hour.
    mov     al, ch
    call    put_bcd2

    inc     bx                  ; do minute.
    mov     al, cl
    call    put_bcd2

    inc     bx                  ; do second.
    mov     al, dh
    call    put_bcd2

    ret

; Places two-digit BCD value (in al) as two characters to [bx].
;   bx is advanced by two, ax is destroyed.
put_bcd2:
    push    ax                  ; temporary save for low nybble.
    shr     ax, 4               ; get high nybble as digit.
    and     ax, 0fh
    add     ax, '0'
    mov     [bx], al            ; store that to string.
    inc     bx
    pop     ax                  ; recover low nybble.

    and     ax, 0fh             ; make it digit and store.
    add     ax, '0'
    mov     [bx], al

    inc     bx                  ; leave bx pointing at next char.

    ret

Comments

[Jester]

Kudos for asking a question properly. If you want to learn, you should single step it in a debugger and see where it doesn't do what you want. In the current form, you of course don't do anything with CL and DH so obviously you don't get any output for minutes or seconds. The same logic you used for the hours should do the trick. You might want to pass in the location where you want the output to the CVT_TIME function so you can reuse it for all 3 parts.

[Peter Cordes]

If you mov ah, al / shr ah, 4 you can unpack both nibbles in parallel. and ax, 0f0fh / add ax, '00' / mov [bx], ax / add bx, 2. I guess you were writing this for human-readability, not for efficiency, though? Or were you optimizing for code-size by using inc bx twice, instead of 3-byte add bx, 2 and a [bx+1] addressing mode (+1 byte)? Probably not, or you would have used al instead of ax for the 2-byte add/and al, imm8 encodings.

[Peter Cordes]

Anyway, yes, DRY is good, but in assembler sometimes macros instead of functions are the right way to avoid it, if the reason for using assembly is performance.