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

Uart applet warnings #568

Open
ikarus23 opened this issue May 6, 2024 · 12 comments
Open

Uart applet warnings #568

ikarus23 opened this issue May 6, 2024 · 12 comments
Labels
applet Component: applet uart Applet: uart

Comments

@ikarus23
Copy link
Contributor

ikarus23 commented May 6, 2024

Hi. My Glasgow just arrived today and I'm very exited. I've just tested the UART applet with nothing connected and made some observations. Not sure of what to make of them. Maybe they will help with development.

  1. If I run the applet I'm always getting parity warnings, although the parity check is none by default.
> glasgow run uart -V 3.3 tty
I: g.device.hardware: generating bitstream ID d709aa1c79bf720fd874632c4cd73c2c
I: g.cli: running handler for applet 'uart'
I: g.applet.interface.uart: port(s) A, B voltage set to 3.3 V
I: g.applet.interface.uart: running on a TTY; enter `Ctrl+\ q` to quit
W: g.applet.interface.uart: 911 frame or parity errors detected
W: g.applet.interface.uart: 12226 frame or parity errors detected
W: g.applet.interface.uart: 12224 frame or parity errors detected
W: g.applet.interface.uart: 12213 frame or parity errors detected
[...]
  1. If I run the applet in pty mode, canceling with CTRL+\ q is terminating the program with a core dump
> glasgow run uart -V 3.3 pty
I: g.device.hardware: device already has bitstream ID d709aa1c79bf720fd874632c4cd73c2c
I: g.cli: running handler for applet 'uart'
I: g.applet.interface.uart: port(s) A, B voltage set to 3.3 V
/dev/pts/1
W: g.applet.interface.uart: 40 frame or parity errors detected
W: g.applet.interface.uart: 12211 frame or parity errors detected
W: g.applet.interface.uart: 12238 frame or parity errors detected
^\[1]    8247 quit (core dumped)  glasgow run uart -V 3.3 pty

  1. If I run the applet in pty mode, canceling with CTRL+C is terminating the program with errors
> glasgow run uart -V 3.3 pty
I: g.device.hardware: device already has bitstream ID d709aa1c79bf720fd874632c4cd73c2c
I: g.cli: running handler for applet 'uart'
I: g.applet.interface.uart: port(s) A, B voltage set to 3.3 V
/dev/pts/1
W: g.applet.interface.uart: 865 frame or parity errors detected
W: g.applet.interface.uart: 12221 frame or parity errors detected
W: g.applet.interface.uart: 12211 frame or parity errors detected
^C

[ Nothing happens after the first CTRL+C ]

^C
Traceback (most recent call last):
  File "/home/ikarus/.local/bin/glasgow", line 8, in <module>
    sys.exit(run_main())
             ^^^^^^^^^^
  File "/home/ikarus/Programs/glasgow/software/glasgow/cli.py", line 937, in run_main

[...]

KeyboardInterrupt

[ Nothing happens, lets press CTRL+C a third time ]

^CException ignored in: <module 'threading' from '/usr/lib/python3.12/threading.py'>
Traceback (most recent call last):

[...]

    if lock.acquire(block, timeout):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyboardInterrupt: 
@attie
Copy link
Member

attie commented May 6, 2024

The first is a non-issue - as there is no signal (and parity is disabled), it's just informing you that there were n framing errors... UARTs are typically idle-high, and if the line is low for an extended period then it is either a break (technically also a framing issue), or an invalid signal.

https://en.wikipedia.org/wiki/Universal_asynchronous_receiver-transmitter#Break_condition

@ikarus23
Copy link
Contributor Author

ikarus23 commented May 6, 2024

Yes, this I understand. Just wasn't expecting "warnings" when doing nothing. Also, the fill up the screen fast. But true, receiving nothing (idle-high) could technically be interpreted as framing errors. I guess the easy way is just to change the log level, right?

@whitequark
Copy link
Member

whitequark commented May 6, 2024

Just wasn't expecting "warnings" when doing nothing.

I consider this the desired behavior since it's an abnormal state indicating a connection error, a faulty transmitter, or some other condition you want to be aware of. I do not consider it a concern that when you are not actually using the applet for its only purpose you're getting what may seem to be spurious warnings.

2. If I run the applet in pty mode, canceling with CTRL+\ q is terminating the program with a core dump

If you run it in PTY mode it does not instruct you to use Ctrl+\ q because that kills the application with a core dump. Ctrl+\ is SIGQUIT; it's used in the TTY mode because it's difficult to find an escape character that's not otherwise used. This is expected behavior.

3. If I run the applet in pty mode, canceling with CTRL+C is terminating the program with errors

That seems like a bug.

@whitequark
Copy link
Member

This is expected behavior.

(That said, I'm wondering if we should switch to a different escape sequence and/or trap SIGQUIT and make it synonymous to SIGINT normally because training users to do a thing that most of the time coredumps is bad.)

@ikarus23
Copy link
Contributor Author

ikarus23 commented May 6, 2024

I consider this the desired behavior

It was just new to me because other USB to UART converters (e.g. FTDI, etc.) don's show that. But fair enough, I understand you point.

If you run it in PTY mode it does not instruct you to use Ctrl+\ q

Correct. I just tired it because Ctrl+cdid not work either. I did not know that it sends a SIGQUIT.

That seems like a bug.

Ok. :)

Thank you for the hard work on the project. Feel free to close this issue or to change its title to something more appropriate (e.g. "Bug when closing UART applet in PTY mode using Ctrl+c").

@whitequark
Copy link
Member

It was just new to me because other USB to UART converters (e.g. FTDI, etc.) don's show that. But fair enough, I understand you point.

One thing we can do is simply add a pullup on the RX line.

@whitequark
Copy link
Member

So reading through this issue with a fresh pair of eyes:

  1. For the framing error problem, I agree that it can be distracting to new users, and given that we use the UART applet as the first applet you run, it's worth putting a higher bar on the UX there. I think we should either:
    • Unconditionally enable the pullup.
    • Conditionally enable the pullup so that revAB users don't get a message about it. (Does anyone care? E.g. @smunaut?)
  2. I think it would be problematic to trap SIGQUIT and make it work the same as SIGINT because then we train users to do a different thing anyway. But we could make it do nothing by default, and something in some (small subset) of applets. Thoughts? This is less for the issue reporter and more for comaintainers.
  3. Ctrl+C in pty mode seems to be hitting a deadlock that should be fixed.

So I think there's actually three issues here, but it's a little more subtle than what it looked at first.

@theorbtwo
Copy link
Contributor

Is it worth the (small, I think) effort to change the message to not mention parity errors if parity is none? FWIW, I also like that it informs me when the DUT stops pulling it's transmit line high.

@whitequark
Copy link
Member

Is it worth the (small, I think) effort to change the message to not mention parity errors if parity is none?

Yes, happy to merge a PR right away.

@smunaut
Copy link
Contributor

smunaut commented May 7, 2024

@whitequark FWIW, no I don't care about a warning message :) And I also only use my revABs with custom applet anyway.

@whitequark
Copy link
Member

@smunaut Thanks. n=1 isn't a lot but it's better than n=0, so I'll go with an unconditional pullup and we can fix it later.

Anyone in this thread wants to add the pullups? You can glean how from the I2C applet.

@whitequark
Copy link
Member

We are now unconditionally enabling pulls on the UART applet: #583

This leaves SIGQUIT and the deadlock on Ctrl-C as the remaining issues.

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

No branches or pull requests

5 participants