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

HID Keyboard: handle delayed and terminated connections #1929

Closed
wants to merge 2 commits into from

Conversation

DietmarSchwertberger
Copy link

@DietmarSchwertberger DietmarSchwertberger commented Nov 25, 2021

The example code is used on many projects, but actually it fails on many occasions, e.g. when shutting down and re-starting a PC.

The modifications handle connection problems. They will try until a successful connection is made and when it is dropped, they will re-try.

Unfortunately I don't know which exception types are thrown exactly. So at the moment I have just added except:
I could not test exactly this code, as my hardware is different.

Also, it might be a good idea to add example code for boot.py:

maintenance_mode = ...  # set when e.g. some key is pressed
usb_hid.enable((usb_hid.Device.KEYBOARD,), boot_device=1 if not maintenance_mode else 0)

@DietmarSchwertberger
Copy link
Author

DietmarSchwertberger commented Nov 25, 2021

@dhalbert : Would you mind looking at this?
You probably can immediately tell which exceptions are thrown when Keyboard(usb_hid.devices) or press/release/send fail. I would then update the pull request.
With the exception handlers and your boot_device modifications, my new ergonomic keyboard is really working well!

20211116_213305_20

@dhalbert dhalbert self-requested a review November 26, 2021 01:45
@dhalbert
Copy link
Contributor

Thanks for the PR. It is a long holiday weekend in the US; I will take a look at this next week, I hope.

keyboard = Keyboard(usb_hid.devices)
keyboard_layout = KeyboardLayoutUS(keyboard) # We're in the US :)
break # successfully connected, leave the connection loop
except:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will also cach KeyboardInterrupt and many other errors you don't want to catch and hide from the user

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I asked Dan to review. He probably knows the execption type(s) to catch.


time.sleep(0.01)
time.sleep(0.01)
except:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

# The keyboard object!
keyboard = Keyboard(usb_hid.devices)
keyboard_layout = KeyboardLayoutUS(keyboard) # We're in the US :)
break # successfully connected, leave the connection loop
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you probably want this break in an else clause of the try

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I prefer the break at this position as it's clearer. I have moved the KeyboardLaoyutUS code out of try/except, though. I think it can't fail.

@dhalbert
Copy link
Contributor

This very old PR has lain fallow, sorry. I recently revisited the same issue. Nowadays another, probably simpler, way to check would be to use supervisor.usb_connected. We could revise the code here, or close the PR and update sample code in several places, in conjunction with updating Learn Guide text in various places.

@DietmarSchwertberger
Copy link
Author

Sure, feel free to close this one, probably after posting a link to the new method.

Would be great to have also a method to fix adafruit/circuitpython#5380 .
This is the remaining issue for building reliable keyboards.

@dhalbert
Copy link
Contributor

(Finally) the adafruit_hid library now checks for .usb_connected and will wait indefinitely for a host to come up. See adafruit/Adafruit_CircuitPython_HID#118, which is released as https://github.com/adafruit/Adafruit_CircuitPython_HID/releases/tag/6.0.0. So I think this PR is now moot.

@dhalbert dhalbert closed this Sep 21, 2023
@DietmarSchwertberger
Copy link
Author

Well, with the other, merged PR the initial connection will not timeout. So, a delayed connection does not need to be handled any more.

But what happens when the PC shuts down with the USB ports remaining powered or when it goes to standby. Probably the connection will then fail at some point. So the try/except around the main loop is still required, right?

See also adafruit/circuitpython#5380 . This probably needs some controlled disconnection as well. A solution for this would probably remove the need for try/except for terminated connections.

@dglaude
Copy link
Contributor

dglaude commented Sep 23, 2023

(Finally) the adafruit_hid library now checks for .usb_connected and will wait indefinitely for a host to come up. See adafruit/Adafruit_CircuitPython_HID#118, which is released as https://github.com/adafruit/Adafruit_CircuitPython_HID/releases/tag/6.0.0. So I think this PR is now moot.

I am worried about that change that will likely break my ultimate mouse jiggle.
https://github.com/dglaude/Mouse_Jiggler_Trinket_M0/blob/main/code.py

In my code I actively wait for the USB to be there while blinking an LED to warn about a problem. That is the behavior if you plug your mouse jiggler in a USB charger... it is also what is happening when your computer boot.

To answer @DietmarSchwertberger I am not sure what is happening at the end, when you turn off your operating system, but the USB port is still powered because it is a charging port. In my code I force a reload on error, so when that occurs, I go back to the initial loop of blink warning.

I'll stop complaining here and try to complain on the usb_hid library about that breaking change. :-)

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.

4 participants