Add update pla
8000
tform to MQTT integration
#80659
Conversation
|
Hey there @emontnemery, mind taking a look at this pull request as it has been labeled with an integration ( |
There was a problem hiding this comment.
Looks good overall, you will need a doc PR as well.
@emontnemery A review from your side is highly appreciated
thecode
added
the
noteworthy
Same, great work and nice addition to the MQTT support |
|
Thank you guys! |
There was a problem hiding this comment.
The PR looks good as such, but it omits a lot of features which are present in UpdateEntity.
It makes sense to start small and add additional features as needed, so that's not a problem as such.
However, this PR starts out with a dedicated topic + template for the latest_version property, which means every new property we want to add in the future will also need its own topic + template, if we follow the same pattern.
Would it be better to do something like:
- A general topic for the read only properties (
installed_version,latest_version,title, etc. etc.) - The general property topic accepts a JSON payload, with an optional template which returns key/value pairs for the properties
It's perfectly fine to only accept updates of thelatest_versionproperty on a general topic in this PR.
Or maybe it's wanted to separate the properties on their own topics for improved flexibility?
Is there some existing external software which supports updates over MQTT using a schema which is compatible with or at least similar to what's proposed in this PR? Zigbee2mqtt supports controlling OTA over MQTT, is its schema compatible with this PR?
|
|
||
|
|
||
| class MqttUpdate(MqttEntity, UpdateEntity, RestoreEntity): | ||
| """representation of an MQTT update.""" |
There was a problem hiding this comment.
| """representation of an MQTT update.""" | |
| """Representation of an MQTT update entity.""" |
There was a problem hiding this comment.
@emontnemery yes, Shelly should work with this PR for users which are using it via MQTT.
There was a problem hiding this comment.
@emontnemery My goal was to be able to create update entities for Shelly devices using MQTT. Initially, I also thought that one topic and template would be the best solution. However, It turned out that for Shelly Gen2 devices it will not work because these devices send installed_version and latest_version in separate topics (progress is also sent in a separate topic).
In my opinion, separate topics and templates are definitely a more flexible solution. If we decide on one topic and a static payload scheme, we will close access to this platform to devices/services that are not DIY and whose firmware is not easy to change.
There was a problem hiding this comment.
Perhaps it is an idea to have a optional state topic that can also accept a json dict (like the siren platform has), so the state can be overrided and has the capability of providing installed and latest version.
But for this PR, may be it is a good idea to have seperate topic's as it appears this is also a pattern in real live.
There was a problem hiding this comment.
I thought of one more possibility. state_topic can be used for title, entity_picture, release_url, release_summary properties, and for installed_version and latest_version will be separate topics.
There was a problem hiding this comment.
I think optional the value_template could resolve all of them if they are supplied, May be for some other cases they are all supplies in one tipic. The state_topic accepted payload could be JSON only. The separate topics are should then be optional probably.
There was a problem hiding this comment.
@jbouwh I'm not sure I understand you. You mean state_topic with JSON data, e.g.:
{"installed_version": "2.0", "latest_version": "2.1", "title" "Test", "release_url": "https://test.com/", "release_summary": "summary"}
and optionally installed_version_topic and/or latest_version_topic if required by the device? If installed_version_topic/latest_version_topic are present in the config we use them, otherwise we use values from state_topic JSON.
There was a problem hiding this comment.
@jbouwh I'm not sure I understand you. You mean
state_topicwith JSON data, e.g.:{"installed_version": "2.0", "latest_version": "2.1", "title" "Test", "release_url": "https://test.com/", "release_summary": "summary"}and optionallyinstalled_version_topicand/orlatest_version_topicif required by the device? Ifinstalled_version_topic/latest_version_topicare present in the config we use them, otherwise we use values fromstate_topicJSON.
That is indeed what I mean, state_topic should be optional in that case (I would advice base the config schema on MQTT_BASE_SCHEMA). My idea is that state_topic could be used to receive all config options.
There was a problem hiding this comment.
Thanks @bieniu for explaining 👍
Proposed change
This PR introduces
updateplatform for MQTT integration.Example of configuration:
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: