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

Bugfix/handle hidapi exceptions in sendReport #55

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dalethomas81
Copy link

Handle an exception generated by hidapi in the sendReport thread. This exception is thrown when no controller is connected. Since the exception is not handled, the thread ends, and the service needs to be restarted to recover. By handling these exceptions, the service becomes fault tolerant and is able to recover automatically.

  • Added a Try/Exception to handle an IOError and an AttributeError.
  • Added a Connected property to indicate when the device is not connected.

-added try except around thread code to handle the IOError raised by hidapi.
-added 'connected' property to indicate when controller is disconnected.

(cherry picked from commit a5ed419)
(cherry picked from commit 6aeeee1)
@flok
Copy link
Owner

flok commented May 27, 2024

Thanks for the pull request, i added some remarks :)

@dalethomas81
Copy link
Author

Hey @flok - sorry, I am not seeing the remarks. Where can I find them?

@dalethomas81
Copy link
Author

Btw, this fixes #48.

@flok
Copy link
Owner

flok commented May 31, 2024

Hey @flok - sorry, I am not seeing the remarks. Where can I find them?

You should see them in this thread at the top as a review from me

@dalethomas81
Copy link
Author

I believe you may not have submitted the review request as there are none. Do you mind trying again or pasting your review here? Thank you for your patience. I am enjoying using this in my project. image

@@ -114,6 +114,7 @@ def init(self) -> None:
self.battery = DSBattery()
self.conType = self.determineConnectionType() # determine USB or BT connection
self.ds_thread = True
self.connected = True
Copy link
Owner

Choose a reason for hiding this comment

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

Is this variable really needed? you only set it 3 times but never compare any value to it?

Copy link
Author

Choose a reason for hiding this comment

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

Hey @flok - this is intended to be used externally to indicated the controller is connected.

As an example, here is how I am using it in my project. https://github.com/dalethomas81/ArfBotOS/blob/main/Controller/DualSenseServer.py#L380

Is there another way to know if it is connected? I am open to alternative approaches.

Comment on lines +244 to +250
except IOError:
self.connected = False
break

except AttributeError:
self.connected = False
break
Copy link
Owner

Choose a reason for hiding this comment

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

In my understanding you could also just break when a exception is thrown and the class variable connected is not needed

@flok
Copy link
Owner

flok commented Jun 1, 2024

I believe you may not have submitted the review request as there are none. Do you mind trying again or pasting your review here? Thank you for your patience. I am enjoying using this in my project.

You are right, my bad. Mostly working with gitlab now a days and forgot to press the button. You should be able to see it now

@dalethomas81
Copy link
Author

No worries! I did respond to your review. Can you let me know if that is satisfactory? Thank you.

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.

None yet

2 participants