-
Notifications
You must be signed in to change notification settings - Fork 4
add interface index to BOOTMOUSE class #15
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
base: main
Are you sure you want to change the base?
Conversation
|
It seems the interface usage of HID devices that have multiple interfaces (combo keyboards) is somewhat random. My initial testing indicating that just detaching/reattaching the interface returned would work. It didn't take too much use before I found instances that I needed to check all the interface up to the returned interface. After a fair amount of use/testing I found that sometimes the third interface needed to be checked as well. The bottom line is that the current version of this checks the first 3 interfaces for report mice. The code is still using the returned interface for boot mice which I think is always going to be 0 so it's possible the BootMouse class doesn't need to have the interface added as an attribute after all (which was the original purpose of this PR), but this seems to be working now and I don't think using the class attribute for boot mice is actually wrong. If on review someone believes that boot mice should simply check interface 0, I can put that back in place and remove the interface from the BootMouse class again. |
|
The BootMouse class currently maintains a single boolean to indicate that a kernel driver was attached. The atexit callbacks that I've been using check the flag and if it's True then check the first three interfaces on the device and reattach any that are not active. This procedure has been working in all the combination of tests I've been running however I don't know if there is a type of HID device that has interfaces that would not be happy having the attach_kernel_driver method being called. A more precise approach would be to use a list for the was_attached attribute so that only interfaces that were attached and are not currently active would have the attach_kernel_driver method called. I'm not sure the change is necessary though? |
|
I decided to ask ChatGPT about the two issues that were worrying me. Using the interface index rather than 0 for boot mice and using a single was_attached boolean. Understanding that I haven't confirmed the ChatGPT conclusions with direct documentation research, I think it's recommendations lines up with the way I was leaning. ChatGPT indicated that the order of kebyoard/mouse on the USB interfaces for a boot mouse is not guaranteed so using zero is probably not a good idea. It also suggested that reattaching all three interfaces would likely work but it would be a better solution (cleaner and more efficient) to utilize a list for was_attached. It also indicated that in some rare situations calling attach_kernel_driver() unnecessarily could cause errors or hang the device. I'm going to go ahead and update this PR to include a list for was_attached. Until that's complete and tested I'll switch this to draft status. |
|
The was_attached attribute is now set to an empty list if no interfaces are detached and if one or more interfaces are detached the attribute is set to a list of the interfaces that were detached. Since an empty list tests as a boolean False this should work with any existing code that expected was_attached to be a boolean. I also updated the release method to take advantage of the lists in was_attached and have used that method in the atexit callbacks of the memory and pypaint game PRs. |
|
It occurs to me that I could probably make some minor backward compatible changes to the find_and_init_boot_mouse and then replace most of the find_and_init_report_mouse with a call to find_and_init_boot_mouse eliminating a lot of repeated code. The trade-off is that I think it would be slightly harder to read/follow. Let me know if you want me to make that update. |
|
I went ahead and combined the find_ functions and I'm thinking it may actually be easier to follow so I'm going to go ahead update the PR but I probably won't get a chance to test it until tomorrow so I'm switching this back to Draft.... |
|
I ran through my tests and this seems to work fine. |
|
I'm sorry for the churn on this, while I was updating the DOC strings it occurred to me that there was a more organized way to setup the functions. This latest version has the common code in a generic find_and_init_mouse function and then the two backwards compatible functions are simple shells that mirror each other. |
I believe to properly work with non BOOT mice, the kernel driver detach/attach commands need to be called using the USB interface index of the mouse.
This PR adds the mouse interface index to the attributes of the BootMouse class and uses the index to detach the kernel driver before configuring the mouse.