8000 usb_cdc.data on Macropad RP2040 · Issue #7456 · adafruit/circuitpython · GitHub
[go: up one dir, main page]
Skip to content

usb_cdc.data on Macropad RP2040 #7456

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.

Sign up for GitHub

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

Jump to bottom
Closed
mscreations opened this issue Jan 15, 2023 · 12 comments
Closed

usb_cdc.data on Macropad RP2040 #7456

mscreations opened this issue Jan 15, 2023 · 12 comments
Assignees
@dhalbert
Labels
bug usb
Milestone
8.0.0

Comments

@mscreations
Copy link

CircuitPython version

8.0.0-beta.4
8.0.0-beta.5
8.0.0-beta.6

Code/REPL

In boot.py:
import usb_cdc
usb_cdc.enable(console=True, data=True)

code.py:
import usb_cdc
import time

usb_cdc.data.timeout = 5.0

while(1):
    if usb_cdc.data.in_waiting > 0:
        starttime = time.monotonic()
        print(str(usb_cdc.data.read(1)))
        print(str(time.monotonic() - starttime))

Behavior

usb_cdc.data.read(1) times out (time is printed out > 5.0 seconds every time) and no data is read.

Description

When I'm trying to utilize the usb_cdc.data serial, the data.read times out rather than reading the data that is in the buffer. My testing involved, installing the version of circuit python under test, erasing the filesystem, and then changing boot.py and code.py to my test case, so it should not be remnant of old code. I have also verified this issue on multiple macro pad devices.

Additional information

I've verified this issue can be reproduced on the versions listed above. The last version that this same code works on is 8.0.0-beta.3.
@Neradoc
Copy link
Neradoc commented Jan 15, 2023

I reproduced the issue on the pico, but there is a twist, the read(1) proceeds as expected if a terminal (like tio) is connected to the data port. If no terminal is connected to the port, nothing is read.

A burst of data like this adds the data to the buffer, and triggers a read or two.

echo "12345" > /dev/cu.usbmodem1234

@dhalbert dhalbert added this to the 8.0.0 milestone Jan 15, 2023
@dhalbert dhalbert added the usb label Jan 15, 2023
@dhalbert dhalbert self-assigned this Jan 15, 2023
@dhalbert
Copy link
Collaborator

@mscreations Does your experience match what @Neradoc reported? How are you sending characters on the host computer side?

I will do a bisect and track this down.

@mscreations
Copy link
Author

Here is the script I'm using to send data to the macro pad.

https://github.com/mscreations/CircuitPython_Macropad/blob/master/Macropad.ps1

@dhalbert
Copy link
Collaborator
dhalbert commented Jan 16, 2023 •
edited
Loading

This new behavior is caused by #7100, which fixed a worse problem: #6018: if an attempt was made to read from a CDC device when USB was not (yet) connected, TinyUSB could read from the wrong endpoint and everything would crash.

I noticed this because when I enabled usb_cdc.data and tried 8.0.0-beta.3 or earlier, bad things happen for me when the board tries to come up. It crashes right away, and appears dead. I thought at first there was something wrong with those builds. But I think whether beta.3 sort of works or not depends on the host computer and/or what is lying around in RAM.

@mscreations It appears your host computer script may write and then disconnect immediately. Can you confirm that? That's what @Neradoc is simulating by writing to the port from the command line. The write sends the characters down, and then immediately disconnects. CircuitPython reads one character, and then TinyUSB notices the disconnect, and then we won't return any more characters, because we notice we are disconnected.

Can you wait long enough before you disconnect for the write to complete and the RP2040 to finish reading what was sent? Perhaps the MacroPad should send some kind of acknowledgment back to your script.

When TinyUSB notices the disconnect, it resets the connection information. The fact that it used to be possible to read the rest of the buffered characters may have been an accident.

@mscreations
Copy link
Author

The script I use to send data to the macropad does not close the serial port immediately after writing. It only closes the port if the script exits abnormally or on Ctrl+C. I did find something else which is that the device is not seeing that the serial port has a connection when I connect using my script. I added in a line to skip getting serial input if the port is not connected (eg usb_cdc.data.connected). This at least allows the device to not throw an exception or sit waiting indefinitely for input. It also doesn't seem to see when I use the application from https://github.com/xhargh/MacropadApplicationDetector. HOWEVER, if I connect using putty, the device recognizes that it is connected and works the way I'm expecting. So that makes me wonder what Putty does differently when opening the serial port to allow the device to recognize it's connected.

@dhalbert
Copy link
Collaborator
dhalbert commented Jan 16, 2023 •
edited
Loading

@mscreations You can check whether TinyUSB thinks it is still connected by doing usb_cdc.data.connected, which will return True or False. So you could print this out in the your loop (instead of checking in_waiting). It would be interesting to see when/if it goes to False.

Note also this interesting note, which I had forgotten about, in our documentation:

connected:bool
True if this Serial is connected to a host. (read-only)

Note

The host is considered to be connected if it is asserting DTR (Data Terminal Ready). Most terminal programs and pyserial assert DTR when opening a serial connection. However, the C# SerialPort API does not. You must set SerialPort.DtrEnable.

Perhaps your ps1 script and the https://github.com/xhargh/MacropadApplicationDetector app are not setting DtrEnable?? But I'm not sure why it would work at all if they were not.

@mscreations
Copy link
Author

Bingo. I was just coming back to report that all the code I've tried and used isn't asserting DtrEnable by default. Once I did that my code runs perfectly (well mostly. still have some code bugs from new v8 changes unrelated to this serial issue.) I am going to close this as it is resolved as far as I'm concerned.

@dhalbert
Copy link
Collaborator

That's fantastic, thanks for the follow-up!

@dhalbert
Copy link
Collaborator

Original issue which caused us to add the documentation note: #4345

@Neradoc
Copy link
Neradoc commented Jan 16, 2023

I still think that the fact that read(1) times out when data.in_waiting > 0 (but disconnected) is a problem. Is it the expected behavior ? Should the buffer be reset on disconnect ? If you reconnect, the current content is not dropped, so what couldn't be read but was kept now can be read.
In fact the example I gave of sending data in bursts with echo things > port is a method that I've used, is it weird to want to read the remaining buffer after a disconnection ? It was a successful transmission.

@dhalbert
Copy link
Collaborator

@Neradoc -- these are good questions. I agree that we shouldn't have data.in_waiting > 0 if we are disconnected and the data can't be read. But the data is in the TinyUSB buffers, I think, and they may be considered invalid once the connection has gone away. In that case, dropping the connection should really clean things up and discard the data. Or, the buffer and channel state should be clean before and after a connection, so that attempts to read won't cause problems. It wasn't, which caused #6018.

I think @hathach might have an opinion here about what the semantics should be. But it may require TinyUSB changes. For now, we could not return data.in_waiting > 0 if there is no connection.

@dhalbert
Copy link
Collaborator

I tried remembering whether there ever was a connection using a boolean that was set and remained set even if the connection went away. Confusingly, this did not work, and I could not read characters, even though the code change was very simple. I spent a while trying to track down what was going on, but it wasn't clear. I'm going to give up on this for now.

@Neradoc Neradoc mentioned this issue Mar 28, 2023
Open
@Neradoc Neradoc mentioned this issue Mar 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dhalbert dhalbert

Labels
bug usb
Projects
None yet
Milestone
8.0.0
Development

No branches or pull requests
3 participants
@mscreations @dhalbert @Neradoc
0