search

basnijholt / miflora

☘️🌡🌼🥀🏡 Mi Flora Plant sensor Python package
MIT License
366 stars 98 forks source link

Support XIAOMI MIJIA Temp and Humidity Sensor v2 #90

Closed ratcashdev closed 6 years ago

ratcashdev commented 6 years ago

Support for the recently announced Temp and Humidity Sensor of Xiaomi with Bluetooth and LCD display. I did not want to duplicate all the back-end code, thus added support for an extra sensor in this package.

To be fully used, it needs a file called site-packages/homeassistant/components/sensor/mitemp.py that is just a slightly amended version of the miflora.py, available here: https://gist.github.com/ratcashdev/2060fd95158dacae93a80a92ae7083a4

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.7%) to 92.031% when pulling 4d7bccdf09ff0fefdf004b86fff099638145ff0c on ratcashdev:master into 4cca1a25e7e9dadd5727fe936c260278935eb92b on open-homeautomation:master.

ratcashdev commented 6 years ago

please advice how to make this code mergeable.

ChristianKuehnel commented 6 years ago

@ThomDietrich , @open-homeautomation

Does it make sense to add support for the "XIAOMI MIJIA Temp and Humidity Sensor v2" into this library? From looking at the code there are quite a few similarities, but it would certainly extend the scope of this library from the plant sensors.

ratcashdev commented 6 years ago

IMHO, the best solution would be to extract all the back-end code into a separate package and then make both miflora and mitemp require those. I assume a lot of other sensors with bluetooth and their implementation would benefit from a common back-end providing 3 different implementation options (bluepy, pygatt, gatttool).

ChristianKuehnel commented 6 years ago

@ratcashdev I started extracting the backend funktionality: https://github.com/ChristianKuehnel/btlewrap

This is from the current master branch, without your notification extensions...

ratcashdev commented 6 years ago

Lovely! Let me know when that is merged and I will submit a separate PR for the notification enhancements.

flavio20002 commented 6 years ago

I had the same idea, but 16 days later...:-) Anyway, I would like to help. My Home assistant installation has already my version of the mitemp_bt Sensor (so I called it) and I've putted all in a single python file using only gatttool.

ChristianKuehnel commented 6 years ago

@ratcashdev The bluetooth backend library is ready to get your notifications merged: https://github.com/ChristianKuehnel/btlewrap

It's also on pypi: https://pypi.python.org/pypi/btlewrap

And you can use it to build a new library to the Xiaomi Mijia sensors

ratcashdev commented 6 years ago

PR submitted: https://github.com/ChristianKuehnel/btlewrap/pull/1

ratcashdev commented 6 years ago

I got the mitemp implementation ready with btlewrap==0.0.2 (https://github.com/ratcashdev/mitemp) A couple of notes though:

  1. This is my very first python package, I have no account on travis, pypi, coverage, etc. I was wondering, if anyone is willing to do the publishing?
  2. I am still undecided on the naming. @flavio20002 's mitemp_bt sounds better, honestly as is mitemp_ble What do you suggest?
flavio20002 commented 6 years ago

Hi, thank you very much. I like both names, but I prefer "bt" beacuse it's more known. Once published, I will test in my Home-asssistant. Did you update the Hass component with the right dependence (the gist version has still miflora)? Thanks

ratcashdev commented 6 years ago

repo and gist updated. See the readme for the gist's link. The new name is mitemp_bt

ratcashdev commented 6 years ago

so back to the publishing question: Is anyone volunteering to publish?

ratcashdev commented 6 years ago

All right. mitemp_bt==0.0.1 is available and can be integrated into hass using https://gist.github.com/ratcashdev/28253bb2c220788e4961f213fe87ff33

flavio20002 commented 6 years ago

Thank you very much. Are you planning to create a PR to Home Assistant? Some notes:

ratcashdev commented 6 years ago

@flavio20002

  1. I'd rather leave it. while I haven't encountered cases when it would have spikes, batter safe than sorry
  2. I am using it for more than a month. However, I can confirm, I have the same log entries like you (>10s). It seems to me that sometimes the device is too slow to respond to connection requests.
  3. I am afraid i am not the one to make it official, but I will for sure create a PR.
ratcashdev commented 6 years ago

The PR is here https://github.com/home-assistant/home-assistant/pull/13955