-
Notifications
You must be signed in to change notification settings - Fork 401
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 12244417857Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - 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.
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)
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.
good point. I added an enable flag in the jukebox.yaml config of this component.
example config
modules:
named:
...
mqtt: mqtt
...
mqtt:
enable: true
# The client id used in communication with the MQTT broker and identification of the phoniebox
client_id: phoniebox_dev
# The username to authenticate against the broker
username: phoniebox-dev
# The password to authenticate against the broker
password: secret-password
# The host name or IP address of your mqtt broker
host: 127.0.0.1
# The port number of the mqtt broker. The default is 1883
port: 1883
@s-martin I added a short documentation about the MQTT feature. @pabera The implementation is working and can be used over MQTT already. Home Assistant is not natively supporting MQTT Media Players so I build in the past a little HACS add on to allow the control of phoniebox via mqtt. https://github.com/c0un7-z3r0/hass-phoniebox This extension is more or less a mapping of the MQTT events to the internal home assistant media player implementation. But the same can be achieved with the Universal Media Player from HA just needs manual mapping of events. Not sure if other Home Automation Systems support MQTT Media Players out of the box. |
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