Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CR/LF might not work on terminals with 1 Mbit/s baudrate #195

Open
sy2002 opened this issue Dec 6, 2020 · 3 comments
Open

CR/LF might not work on terminals with 1 Mbit/s baudrate #195

sy2002 opened this issue Dec 6, 2020 · 3 comments
Assignees
Labels

Comments

@sy2002
Copy link
Owner

sy2002 commented Dec 6, 2020

IO$GETS_CORE in monitor/io_library.asm implements the various gets variants that are used throughout the whole system. For providing maximum flexibility regarding the end user's terminal settings, the Monitor accepts as line ends: CR or LF or CR/LF. For being able to do the latter one, a timed wait is being used, see this comment from the source code:

                ; For also accepting CR/LF, we need to do a non-blocking
                ; check on STDIN, if there is another character waiting.
                ; IO$GETCHAR is a blocking call, so we cannot use it here.
                ; STDIN = UART, if bit #0 of IO$SWITCH_REG = 0, otherwise
                ; STDIN = PS/2 ("USB") keyboard
                ;
                ; At a terminal speed of 115200 baud = 14.400 chars/sec
                ; (for being save, let us assume only 5.000 chars/sec)
                ; and a CPU frequency of 50 MHz we need to wait about
                ; 10.000 CPU cycles until we check, if the terminal program
                ; did send one other character. The loop at GETS_CR_WAIT
                ; costs about 7 cycles per iteration, so we loop (rounded up)
                ; 2.000 times.
                ; As a simplification, we assume the same waiting time
                ; for a PS/2 ("USB") keyboard

_IO$GETS_CR     MOVE    2000, R3            ; CPU speed vs. transmit speed
_IO$GETS_CRWAIT SUB     1, R3
                RBRA    _IO$GETS_CRWAIT, !Z

Challenge:

CR (stand alone) as a line end and LF (stand alone) as a line end should always work fine.

But CR/LF is probably not stable on terminals with 1 Mbit/s baudrate.

Back in the days, when there was no support for switchable baudrates, this hardcoded solution seemed to be good enough.

Solution:

  • Refactor this and take the baudrate divisor into consideration.
  • Use the to-be-done delay OS function as described in issue Delay function #189 for being accurate and flexibe.
  • Make sure not to use any magic numbers but only constants from our global constants file
@sy2002 sy2002 added the V1.7 label Dec 6, 2020
@sy2002 sy2002 self-assigned this Dec 6, 2020
@bernd-ulmann
Copy link
Collaborator

Without having thought about this issue in depth, couldn't we set a flag when we encounter a LF character and just return from the routine. Upon the next GETx we check if the first character read is CR. If true and if the flag is set, we just discard the CR character. In either case we reset the flag after reading the first character. Would this work?

@sy2002
Copy link
Owner Author

sy2002 commented Dec 7, 2020

@bernd-ulmann Great idea! And so much simpler and more stable than using a delay. 👍

In the meantime we do have a hardware FIFO in the FPGA UART implementation, so this would work even if the program at hand is not calling GETx in a loop but only from time to time: The surplus LF(*) would just sit in the FIFO and do no harm.

P.S. About (*): I guess you mixed things a bit and wanted to say: [For being able to support CR/LF additionally to a pure CR or a pure LF]: "we set a flag when we enocunter a CR and just return from the routine. Upon the next GETx we check if the first character read is LF. If true and if the flag is set, we just discard the LF character. In either case we reset the flag after reading the first character."

@bernd-ulmann
Copy link
Collaborator

You are, of course, right... The CR should set the flag and then suppress an optional LF coming afterwards. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants