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

Add paho-mqtt as a requirement in manifest.json #94

Merged
merged 1 commit into from
Nov 26, 2023
Merged

Add paho-mqtt as a requirement in manifest.json #94

merged 1 commit into from
Nov 26, 2023

Conversation

chishm
Copy link
Contributor

@chishm chishm commented Nov 25, 2023

Add a requirement on the external package paho-mqtt so that Home Assistant will install it when trying to setup ha-dyson for the first time.

I've not put version restrictions in because I don't know what is/isn't compatible, but I found it worked with paho-mqtt==1.6.1.

Copy link
Contributor

@codyc1515 codyc1515 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you see an error because of not having this?

@chishm
Copy link
Contributor Author

chishm commented Nov 25, 2023 via email

@dotvezz
Copy link
Member

dotvezz commented Nov 26, 2023

@chishm huh, this is super interesting and I'm honestly surprised this hasn't popped up before. Needless to say, it makes perfect sense to add paho-mqtt as an explicit requirement so I'll definitely do it.

I wonder if something changed with new versions of Home Assistant; I think it used to be in the dep tree of a basic installation, and it's still in the docker image for example: (edit: But I noticed it's not included when I do a normal installation on Debian. Not sure if that's really new or it's always been that way, but I kinda suspect it's new.)

> docker run --rm -it homeassistant/home-assistant pip show paho-mqtt
Name: paho-mqtt
Version: 1.6.1
Summary: MQTT version 5.0/3.1.1 client class
Home-page: http://eclipse.org/paho
Author: Roger Light
Author-email: roger@atchoo.org
License: Eclipse Public License v2.0 / Eclipse Distribution License v1.0
Location: /usr/local/lib/python3.11/site-packages
Requires: 
Required-by: aiomqtt, ibmiotf, pyeconet, pyezviz, pysmappee, python-roborock, roombapy, tuya-iot-py-sdk

@dotvezz dotvezz merged commit 45fa996 into libdyson-wg:main Nov 26, 2023
2 checks passed
@dotvezz
Copy link
Member

dotvezz commented Nov 26, 2023

Thanks for the PR! Merged and I'll do a release tonight.

@chishm
Copy link
Contributor Author

chishm commented Nov 26, 2023

I wonder if something changed with new versions of Home Assistant; I think it used to be in the dep tree of a basic installation, and it's still in the docker image for example: (edit: But I noticed it's not included when I do a normal installation on Debian. Not sure if that's really new or it's always been that way, but I kinda suspect it's new.)

I'm not sure if something has changed (this is my first time installing ha-dyson), but it sounds like you've managed to replicate my setup. To be exact, I run HA in its own Python venv on Ubuntu, and I don't use anything else that uses mqtt.

Thanks for the PR! Merged and I'll do a release tonight

You're welcome, and thanks for the careful consideration and merge.

@chishm chishm deleted the manifest-requirements branch November 26, 2023 12:38
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.

3 participants