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

Implement a polling fallback for USB monitor #130918

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

puddly
Copy link
Contributor

@puddly puddly commented Nov 18, 2024

Proposed change

Background: I'm preparing to add a usb.async_register_port_event_callback callback that will allow for integrations to register themselves for USB events, namely USB device discovery and removal. This will allow the SkyConnect integration to unload itself when the SkyConnect becomes unplugged, without having to actually use the serial port.

The current USB monitor does not work on macOS because it uses udev, which only runs on Linux. We instead can periodically call _async_scan_serial as a generic fallback solution for platforms that do not support udev properly.

This is intended for development purposes so the 5s polling interval realistically could be reduced to 1s. The only real platform I can see affected by this is WSL, which I don't believe we support for running HA Core.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration (usb) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of usb can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign usb Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@balloob
Copy link
Member

balloob commented Nov 19, 2024

Because of our supported environments, this would only be needed during development. Should we make that explicit to avoid a custom image without udev relying on this logic, while it's only intended for development.

@puddly puddly marked this pull request as ready for review November 20, 2024 17:00
@puddly puddly requested a review from bdraco as a code owner November 20, 2024 17:00
@puddly puddly mentioned this pull request Nov 21, 2024
19 tasks
@puddly
Copy link
Contributor Author

puddly commented Nov 22, 2024

I've just realized that HA OS does not actually use udev itself! Supervisor calls usb/scan on hardware changes, currently only addition.

In fact, I think the udev monitor is only used when you're running Core in a virtualenv, since it will not work on HA OS, Supervised, or Container due to the check for Docker!

@bdraco You seem to be contributing most to the usb component. Is my analysis here correct? It seems like USB hardware monitoring for Container installs is broken right now: they will be flagged as docker and thus pyudev will not be used, but there also will be no Supervisor to run usb/scan.

Should we whitelist non-Supervised container for pyudev monitoring? Or switch all non-Supervised platforms over to a 1s polling period?

@puddly puddly marked this pull request as draft November 22, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants