pilebones / go-udev

Simple udev implementation in Golang
GNU General Public License v3.0
95 stars 28 forks source link

Matching on the SUBSTEM env value not possible for ExistingDevices #1

Closed stolowski closed 6 years ago

stolowski commented 6 years ago

Matching based on the SUBSYSTEM env key is not possible with ExistingDevices (I haven't checked if it applies to monitoring ).

The problem is that SUBSYTEM key is only inserted into env after matcher evaluates to true (see device.go). I'm happy to propose a fix for this (moving the Readlink(.. "/subsystem")) code above matcher check if you agree this is a bug and the fix is reasonable. Please let me know. In any case the matcher.sample included in the project is misleading I think as it's all based on SUBSYTEM check which won't work (for existing device at least).

pilebones commented 6 years ago

Thanks a lot for your feedback.

Indeed an uevent should contains the subsystem information inside the env map because the kernel sent it like that using an UEvent object. By this way, the monitoring mode is not impacted by this issue. Monitoring mode just subscribing then listenning the uevent channel provide by kernel.

I'm okay if you want to purpose a pull-request to fix it, thank you for the proposition. I will merge the PR after validation.

Then I will validate if matcher.sample or unit-test or other related project which used this repository should be update.

You are the first external contributor, so out of curiosity, what is your use case with pilebones/go-udev ?

stolowski commented 6 years ago

Ok, I've proposed a fix. One alternative would be to be insert SUBSYSTEM into env inside the getEventFromUEventFile(..), but that would change the meaning of the function as it would no longer really get you just the exact content of uevent file.

Regarding your last question, I'm one of the developers behind https://github.com/snapcore/snapd and I'm currenly looking for a go package that will allow us to handle hotplug events. I've not idea if we will use your package or not as I've just started my exploration and experiments.

Cheers!

pilebones commented 6 years ago

Indeed getEventFromUEventFile() has been implemented to provide basically the env inside uvent file. Subsystem is like a metadata provided using another way by kernel, that is why I had splitted these part of code.

I had implemented this project to autodetect (hotplug or if already plugged) a GPS devices to implement a GPS tracker for paragliding (because I'm a paraglider). My requirement: re-use nothing, be minimalistic (not use C-binding). My GPS daemon implementation is still in developpement but you can see a example of use/implementation to autodetect device : https://github.com/pilebones/go-gpsd/blob/master/autodetect.go#L27 Probably close to your needs, I hope so.

Anyway, feel free to do feedbacks, I will do my best to react quickly and effectively.

snapcore/snapd is an impresive project, and I would be happy if my project is integrated with yours if so your choice.

Best