-
Notifications
You must be signed in to change notification settings - Fork 397
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
feat: add basic mqtt support #2412
base: future3/develop
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 10231290157Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your contribution!
Please add a short doc file in documentation/builders/components
, which helps users how to use the new MQTT feature.
""" | ||
This file provides definitions for MQTT to RPC command aliases | ||
|
||
See [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line could be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it took me a while to get back to Phoniebox coding. This is a great addition. I have made only a few comments, the quality of code looks alright for now.
How do you use this code? What does your integration look like? Am I correct that this code does not work with Home Automations out of the box yet?
client_id = cfg.setndefault("mqtt", "client_id", value="phoniebox-future3") | ||
username = cfg.setndefault("mqtt", "username", value="phoniebox-dev") | ||
password = cfg.setndefault("mqtt", "password", value="phoniebox-dev") | ||
host = cfg.setndefault("mqtt", "host", value="127.0.0.1") | ||
port = cfg.setndefault("mqtt", "port", value=1883) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have some of those useful settings in jukebox.yaml?
Also, one addition I would prefer is to be able to enable/disable this functionality (e.g. through the jukebox.yaml)
This adds initial support and functionality for MQTT. Trying to be downward compatible as much as possible so potential mqtt implementations are not totally broken. Not all legacy features could be ported since the current future3 is not supporting them. Yet.
This PR referes to #1963