search

raetha / wyzesense2mqtt

Configurable WyzeSense to MQTT Gateway intended for use with Home Assistant or other platforms that use MQTT discovery mechanisms.
MIT License
80 stars 23 forks source link

Check for /dev/wyzesense #16

Closed jhenkens closed 4 years ago

jhenkens commented 4 years ago

If someone follows steps to make an alias for the wyzesense dongle, which makes a lot of sense when running in docker to be able to reliably pass it to the container, then it'd be nice to be able to detect and use that.

See the instructions in this comment, which might be good to replicate in a README.md possibly.

https://github.com/kevinvincent/ha-wyzesense/issues/66#issuecomment-569470754

raetha commented 4 years ago

Hi @jhenkens, I'll try to take a look this week. Completely agree with being able to support the symlink. And sorry for the delay, I'm in healthcare IT and the last couple months have had me braindead by the time I get home. Things are starting to calm down a bit though, and I want to get back to some improvements here.

raetha commented 4 years ago

As another aside, I would actually expect the autodetect routine to find the symlink, so that's what I want to see why it wouldn't be working as expected.

jhenkens commented 4 years ago

https://github.com/raetha/wyzesense2mqtt/pull/16/commits/73966d08b5b9a408f2f73f5d1796255d8cedd908#diff-aa6ca537abfd053b09f34cda91329777L107

Could it be due to that line? if ("hidraw" in device_name):

I didn't play around with the other part of the code

raetha commented 4 years ago

Ok, finally got a chance to dig in and play here. By adding the device alias on the host, the root device isn't really passed through to the container, and thus the entire /sys/class logic of auto detection simply won't work as you can no longer see the device class. That means the auto value in the config file doesn't work.

Rather than hard coding something for /dev/wyzesense, which is a value the user could change to anything they want, what makes sense here is to update your config file and set the device path manually rather than using auto detection. That was the original reason for the config file override, it just defaults to automatic as that will work for most people.

I think I may try to add some documentation to cover this scenario, as custom dev names are nice for some folks who might have multiple hidraw devices. But I'd still recommend that you use the config file instead of changing the code.

Does that make sense?

jhenkens commented 4 years ago

Yep, makes sense!

On May 13, 2020, at 4:45 PM, Elias Hunt notifications@github.com wrote:

 Ok, finally got a chance to dig in and play here. By adding the device alias on the host, the root device isn't really passed through to the container, and thus the entire /sys/class logic of auto detection simply won't work as you can no longer see the device class. That means the auto value in the config file doesn't work.

Rather than hard coding something for /dev/wyzesense, which is a value the user could change to anything they want, what makes sense here is to update your config file and set the device path manually rather than using auto detection. That was the original reason for the config file override, it just defaults to automatic as that will work for most people.

I think I may try to add some documentation to cover this scenario, as custom dev names are nice for some folks who might have multiple hidraw devices. But I'd still recommend that you use the config file instead of changing the code.

Does that make sense?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

raetha commented 4 years ago

Great. I'll close this one, but definitely feel free to submit others, or open issues if you think of other things. I'm happy to take assistance with making this better in the long run.