Skip to content

Conversation

@dlech
Copy link
Member

@dlech dlech commented Sep 13, 2025

I've been experimenting with Prime hub USB on Windows and the transmit time is just timing out all of the time causing the hub to unsubscribe from USB notifications, effectively losing communication with the hub.

I don't know what we could do in it's place yet to handle the case when a user program is blocked on printing and and application disappears without sending the unsubscribe message. But the timer doesn't seem like a viable solution. Maybe we just leave it like this for a while and see how often this really happens? If it does happen, the user just has to unplug the USB cable (and optionally plug it back in) to get unstuck.

On EV3, removing the timer wasn't so trivial, so I haven't done that yet. Maybe something @ArcaneNibble could help with?

Remove the transmit_timer that was monitoring whether the host was
reading data or not. In practice, this was causing false positives
when the host was just slow to read data, which caused the connection
to effectively drop.
We are getting connection drops because host applications don't always
read data in a timely manner. Ideally, we would remove this timeout
completely, but some other work needs to be done to make that feasible.
@coveralls
Copy link

coveralls commented Sep 13, 2025

Coverage Status

coverage: 58.549% (+0.01%) from 58.535%
when pulling 7f3a471 on dlech:usb-temporary-fix
into 3e8cb1e on pybricks:master.

@dlech dlech marked this pull request as draft September 14, 2025 18:58
@ArcaneNibble
Copy link
Collaborator

I've been hacking on proper NXT support and discovered an issue with its USB controller -- I don't see a way to abort packets that have already been queued, without triggering a STALL condition on the endpoint. This gives me an idea for how we can handle this:

  • Default the IN (device -> PC) endpoint to have the HALT flag set.
  • When a PC application is connected and wants to subscribe, it uses the standard USB CLEAR_FEATURE request to clear the endpoint halt condition. This tells the device that it's okay to send status and stdout.
  • If the PC application is exiting, it can use SET_FEATURE to set the endpoint halt condition.
  • If the device tries to send something, but the endpoint is halted --> skip sending
  • If the device tries to send something, and the endpoint is not halted, but the buffer is full --> set the endpoint halt condition

This feels like the proper USB-Approved™️ approach, not involving timeouts

@ArcaneNibble
Copy link
Collaborator

Thinking about this further, we might still want a timeout (block the user program for some interval before giving up)

The critical difference between using an endpoint halt vs another mechanism is that it signals to the host that the device thinks the "is subscribe" state changed

@dlech
Copy link
Member Author

dlech commented Sep 16, 2025

  • If the PC application is exiting, it can use SET_FEATURE to set the endpoint halt condition.

Applications can disappear without warning (especially web apps), so how this behave if this step is skipped? I guess we could have a timeout on the peripheral and set a halt after some time, then if the app is really still there it sees the halt, clears it and we are back on step 2?

If this works, it sounds like a better way to handle the subscribe/unsubscribe since there is a way to actually get notification from the peripheral that it was unsubscribed due to timeout without actually having to poll it.

@ArcaneNibble
Copy link
Collaborator

Yes, I realized you will still need a timeout and some amount of blocking. The advantage of using the endpoint halt is that the host gets notified when a timeout occurs.

@dlech
Copy link
Member Author

dlech commented Sep 16, 2025

OK, so we don't want this change then.

@dlech dlech closed this Sep 16, 2025
@dlech dlech deleted the usb-temporary-fix branch September 16, 2025 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants