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

Adding MQTT and Python 3 support. #8

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

stenjo
Copy link

@stenjo stenjo commented Jan 19, 2019

Closes #7

@stenjo
Copy link
Author

stenjo commented Jan 19, 2019

Seems like the check for "None" in the source for serial no does not work properly. Have added another check in the connect() method on the WavePlus() Class

@aleksanderaleksic
Copy link

I like the idea of using the MQTT broker to publish current values from Wave Plus, but shouldn't it rather be in a separate module? I think that if someone else wants to use the reader with another protocol they then depend on MQTT. If you get what I mean?

@stenjo
Copy link
Author

stenjo commented Jan 21, 2019

I think that is a good idea. I can split the mqtt part into a different module. So the non-mqtt users are not neccessarily depandant on the paho-mqtt library.

@stenjo
Copy link
Author

stenjo commented Jan 22, 2019

Added some changes yesterday. Now all classes are in their own files and the mqtt functionality (and include) is in a separate file. Works on my pi :-)
closes #8

@aleksanderaleksic
Copy link

Hi @stenjo
I talked to @orjangj who is the main maintainer of the repo, he doesn't have time to review your PR at the moment. Personally, I don't know python 😅 But after talking to @orjangj he had some comments:

  1. You have forked out from an old version of the repo, so you will need to merge in the latest changes.
  2. He would rather like to have the MQTT support outside of the repo.
  3. He is interested in the phyton 2.7 and 3 support

So if you could merge in the latest version and still have the support for phyton 3 that would be great. But you will have to move the MQTT support to another repo.

@frostmo84
Copy link

mqtt is great for home assistant and other home automation platforms. i will try to set this up when I have som free time. would be very interesting to adapt this for use with an ESP32 if possible.

@alexchandel
Copy link

Python 2 is officially end-of-lifed in 1 month: https://www.python.org/doc/sunset-python-2/
Python 3 support is needed.

@stenjo You need to rebase your PR. Lots changed in #6.

@stenjo
Copy link
Author

stenjo commented Nov 26, 2019

Python 2 is officially end-of-lifed in 1 month: https://www.python.org/doc/sunset-python-2/
Python 3 support is needed.

@stenjo You need to rebase your PR. Lots changed in #6.

Seems to be updated already. What changes are missing?

@RomanGar
Copy link

Hi @stenjo ,

Can you please share your *.things file for the openHAB?
It would help newbies like myself to get this configured.

I have the Airthings Wave 2nd generation, that is not supported by either script (neither read_wave.py, nor read_waveplus.py) but with help from orjangj I managed to get the read_waveplus.py working (as well as yours WavePlus.py) , now I just need the mqtt setup/configure on the openHAB.

@steoj
Copy link

steoj commented Dec 11, 2019

I'd be happy to. Are you able to make a PR with the changes needed for these scripts to work for you? I need to get home to pull the .things file from my OpenHAB setup.

@RomanGar
Copy link

I will try ;-), never used GitHub before

@stenjo
Copy link
Author

stenjo commented Dec 14, 2019

This PR is now updated with an openhab waveplus.items file for usage in an OpenHab installation.

@stenjo
Copy link
Author

stenjo commented Jan 1, 2020

I will try ;-), never used GitHub before

I'd also be happy to know what changes you had to do to make it work. Also bought a Airthings Wave (Radon, Temp and humidity only) and are not able to make that work with either of the programs (not this PR or the original).
Just add your comments in this thread and I'll see what I can do,

@alexchandel
Copy link

You both changed the same files (namely read_waveplus.py), yours clearing the changes that fixed issue 4.

@stenjo
Copy link
Author

stenjo commented Jan 2, 2020

You both changed the same files (namely read_waveplus.py), yours clearing the changes that fixed issue 4.

We are, indeed, changing the same file, but the code is basically moved into separate class files and kept the same (for the most part). I am not able to see how my changes invalidates or reverses the fixes in issue 4.
That might be beside the point though, as we have two branches with same changes, and this creates the need for a rebase, maybe? How do I do that? Have not been able to figure that out yet....

@engineerjoe440
Copy link

I just want to jump in here... for nerds like myself, having Python 3 support is super important. I'm looking to deploy these scripts on a Debian 11 server that's hosting a Home Assistant Docker container. Python 3 really is the way to go. So I'd love to see this make it back "upstream" to the primary Airthings project. Not too much trouble for me to have cloned @stenjo's fork, but for newer folks, it would be much better to get direct access to the Python 3 content. Not to mention, having up-to-date code makes a big difference when considering the viability of Airthings products in a self-hosted or tech-savvy world.

So... I'd love to see this get merged. I see there's merge conflicts, but are there other reasons this hasn't been merged?

@lundstrj
Copy link

lundstrj commented Jun 3, 2024

bump

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.

Python 3 and MQTT support.
9 participants