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

feat: add support for cancellable hotplug #110

Closed
wants to merge 2 commits into from

Conversation

erkki-silvola
Copy link
Contributor

@erkki-silvola erkki-silvola commented Jan 29, 2025

Note did not add much docs as I was not sure how much conversation this will raise, also unsure about the naming. This is intended for 'library' use case where hotplug needs to be started and stopped...

@kevinmehall
Copy link
Owner

kevinmehall commented Jan 29, 2025

I don't think you need special support in nusb for this -- if you drop the HotplugWatch, it will clean itself up. You can do the rest externally with select or or from several different async libraries, and a oneshot channel or signal directly. You've effectively re-implemented that. I don't think every library that provides a Stream needs to re-implement their own version of cancellation, because the power of the Future and Stream traits is that they are composable.

@erkki-silvola
Copy link
Contributor Author

I don't think you need special support in nusb for this -- if you drop the HotplugWatch, it will clean itself up. You can do the rest externally with select or or from several different async libraries, and a oneshot channel or signal directly. You've effectively re-implemented that. I don't think every library that provides a Stream needs to re-implement their own version of cancellation, because the power of the Future and Stream traits is that they are composable.

Yes, sorry for this, still a bit new on this, used stream abortable which effectively is the same

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.

2 participants