search

Jinjinov / Usb.Events

Subscribe to the Inserted and Removed events to be notified when a USB drive is plugged in or unplugged, or when a USB device is connected or disconnected. Usb.Events is a .NET Standard 2.0 library and uses WMI on Windows, libudev on Linux and IOKit on macOS.
MIT License
95 stars 23 forks source link

Separate UsbEventWatcher construction from start of enumerating devices #20

Closed d79ima closed 2 years ago

d79ima commented 2 years ago

Library version

10.0.1

OS & OS version

Ubuntu 18.04 LTS

Describe the bug

This is kind of a bug or more like a race condition. Since we have to new up UsbEventWatcher before we can subscribe to UsbDeviceAdded event. It is possible that some device will be missed in the event subscriber because UsbEventWatcher constructor already starts enumerating devices in a background thread.

Suggestion

Could you please separate construction of UsbEventWatcher from start of enumerating devices. Constructor could just not call the Start method and you could simply make the Start method public and let user call it after they finished subscribing to all the desired events.

An even better option would be to separate EnumerateDevices from StartMonitoring into separate public methods. And make EnumerateDevices populate the UsbDevicesList instead of firing UsbDeviceAdded event. This way it is more natural to use the library. You can new up UsbEventWatcher, then EnumerateDevices to get a list of all currently known devices, then StartMonitor to monitor for changes to devices current and new.

Jinjinov commented 2 years ago

Thank you for noticing this.

I agree that constructor shouldn't call Start() before you have the chance to subscribe to events.

Unfortunately removing Start() from constructor is a breaking change for all who use my library - if they don't call Start() it will just stop working.

I can put another Action callback parameter in constructor, to let you subscribe to events in constructor. Or perhaps I will add bool startWatcher = true to constructor and it will work for all other users, and you can set it to false.

About separating EnumerateDevices from StartMonitoring - I agree that your suggestions would make the library better :) I would also have to do that for Windows and for MacOS - the library has to be consistent - but unfortunately I don't have the time to do that now.

I pushed the changes to repo - do you have any other suggestions? If not, I will make a new NuGet.

d79ima commented 2 years ago

Thank you, i understand. Let's go withbool startWatcher = true for now as a constructor arg. This will be a pretty simple change. No other suggestions at this time :)

Jinjinov commented 2 years ago

Ok, thanks :) I published a new NuGet .