-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
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
ruckus_unleashed: Detected blocking call to sleep inside the event loop #84550
Comments
Hey there @gabe565, mind taking a look at this issue as it has been labeled with an integration ( Code owner commandsCode owners of
(message by CodeOwnersMention) ruckus_unleashed documentation |
This is an annoying problem in upstream code: pexpect/ptyprocess#2 It needs some way to close down async. |
I dont remember if we check the sleep argument in ha. A sleep of zero is no issue. |
@elupus is correct. I've tried to eliminate as many sleep calls as possible, but unfortunately can't avoid all of them when using pexpect. |
The Unleashed SSH client isn't ideal for this sort of integration. Apart from having to insert sleeps etc, this integration will make it very difficult to use the CLI for anything else, since 'enable' is single-user so any CLI user will be forcibly de-privileged every few minutes. @gabe565 - If you're interested, I forked your pyruckus and migrated it to use HTTPS AJAX, which should be more reliable. It's a major bit of brain surgery, and I'm not much of a Python programmer, so I didn't PR the changes back to your repo. |
I added pyruckus methods to block & unblock devices, in case you wanted to add this functionality in Home Assistant. Edit: I added methods to turn an AP's status LEDs on and off, in case you want to e.g. turn them off at night. |
@ms264556 Nice work! I haven't gotten a chance to take a look at your fork yet, but I love the idea of getting that merged upstream. An HTTP API would be more standard and would fix the blocking call to sleep error. |
@gabe565 I'm obviously a bit worried, since this integration seems to be used by ~100 users. |
Hey! Sorry I've been a bit slow to respond. If you create a PR, I think it will automatically add me for approval! Although, I think using the same package for this would make sense instead of creating a new one. The package wasn't meant for only SSH so I'd definitely like to merge your PR and add support for both. Otherwise, it would be fine if this was a breaking change and users have to swap from SSH to HTTP. It would be worth it for the benefits of using a JSON API instead of string parsing |
I also forgot to mention this but I don't have access to a physical Ruckus device anymore, so I can't test your change unfortunately. I made a package that mocks Ruckus SSH, but not HTTP |
I added a bunch of pyruckus compatibility shims to my library and poked it into a pyruckus fork, because I thought this was the quickest way to get it into HA. But the pyruckus PR was pending for quite a while, so I ended up just removing the shims and modifying HA to directly use my library. Feel free to just copy-paste whatever you want from aioruckus into pyruckus (my code is 0BSD licensed, so requires no attribution) then retarget the HA PR. (I just noticed your comment mentions not having a ruckus device anymore. I'd find it really hard to do bugfixes/enhancements just using a mock). |
@ms264556 Yeah that's been the main reason I've been slow to review your PR. It's hard to accept big changes like this without a real device, even though I'm really excited about them. You're library is looking great, though! I just don't want this to cause fragmentation if we can avoid it. Honestly, at this point I don't really want to keep SSH. Your changes retain all of the same functionality and HTTP is pretty much always preferable imo. I'd like to reopen that PR and get it merged in! Also, do you want me to add your as a maintainer on gabe565/pyruckus? Edit: I can't reopen the PR in pyruckus anymore since the history has diverged. Let me know what you want to do, I don't mind working with you either way! |
There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. |
The problem
Sometimes (actually too often) my Ruckus Unleashed crashes and looks like Home Assistant doesn't handle this error well:
What version of Home Assistant Core has the issue?
core-2022.12.8
What was the last working version of Home Assistant Core?
No response
What type of installation are you running?
Home Assistant OS
Integration causing the issue
Ruckus Unleashed
Link to integration documentation on our website
https://www.home-assistant.io/integrations/ruckus_unleashed
Diagnostics information
No response
Example YAML snippet
No response
Anything in the logs that might be useful for us?
No response
Additional information
No response
The text was updated successfully, but these errors were encountered: