1technophile / OpenMQTTGateway

MQTT gateway for ESP8266 or ESP32 with bidirectional 433mhz/315mhz/868mhz, Infrared communications, BLE, Bluetooth, beacons detection, mi flora, mi jia, LYWSD02, LYWSD03MMC, Mi Scale, TPMS, BBQ thermometer compatibility & LoRa.
https://docs.openmqttgateway.com
GNU General Public License v3.0
3.55k stars 784 forks source link

Publish HADiscovery message with retain flag #448

Closed pensionado closed 4 years ago

pensionado commented 5 years ago

Is your feature request related to a problem? Please describe. Not really, it is that just every couple of minutes it publishes the HADiscovery config topic.

Describe the solution you'd like Publish HADiscovery message once only at startup with the retain flag. It avoids unnecessary MQTT messages (and processing by clients that subscribe to it).

Describe alternatives you've considered None, they are nuisance duplicate messages

Additional context Retained config messages will be obtained when a client subscribes to the topic.

ma-ochoa commented 4 years ago

If the entities are added to home assistant with mqtt autodiscover, if home assistant is restarted, the entities do not appear until omg sends the autodiscover topic.

I do not know if the solution is to indicate the retain flag in the discovery or adjust a variable so that the delivery time of the topic autodiscovery is adjustable, it is currently sent approximately every 60 minutes, during this time the entities give homeassitant error.

pensionado commented 4 years ago

No if a topic is published with the retain flag, they will stay in the MQTT broker and then when homeassistant starts up and subscribes to the autodiscovery topic it will receive all the retained messages and the gateway does not need to republish them every so often. Only at start up time just to make sure they are retained in the MQTT broker.

ma-ochoa commented 4 years ago

So the problem I have is from Home assistant, mosquitto or OMG?

I run HA and mosquitto in the same machine

When I restart HA all devices created with autodiscover disappear and do not appear again unless I wait for OMG to update the autodiscover topic or manually restart OMG.

I do not know if OMG has the possibility to send you an mqtt command to force the autodiscover update, maybe it would be a solution, then on restarting HA could send this command and solve the problem.

pensionado commented 4 years ago

The problem is, now, caused by the way OMG publishes the config topic. If it uses the retain flag, as I proposed, Home assistant will get the config messages immediately at startup when it subscribes to the topic in the MQTT broker, because they would have been retained by the MQTT broker (if the retain flag has/had been set). This is on how other products, like miflora-mqtt-daemon, are being auto discovered in Home assistant at startup time.

pensionado commented 4 years ago

v0.9.3-beta, doesn't use retain flag (yet ?): smal mqtt trace of this: ''' homeassistant/binary_sensor/600194BFC9E7/config {"stat_t":"home/OpenMQTTGateway1/LWT","name":"OpenMQTTGateway1","uniq_id":"600194BFC9E7","dev_cla":"connectivity","pl_on":"online","pl_off":"offline","pl_avail":"online","pl_not_avail":"offline","device":{"name":"OpenMQTTGateway1","manufacturer":"OMG_community","sw_version":"0.9.3beta","identifiers":["600194BFC9E7"]}} homeassistant/sensor/600194BFC9E7gatewaySRFB/config {"stat_t":"home/OpenMQTTGateway1/SRFBtoMQTT","name":"gatewaySRFB","uniq_id":"600194BFC9E7gatewaySRFB","val_tpl":"{{ value_json.value }}","device":{"name":"OpenMQTTGateway1","manufacturer":"OMG_community","sw_version":"0.9.3beta","identifiers":["600194BFC9E7"]}} homeassistant/binary_sensor/840D8E658E66/config {"stat_t":"home/OpenMQTTGateway2/LWT","name":"OpenMQTTGateway2","uniq_id":"840D8E658E66","dev_cla":"connectivity","pl_on":"online","pl_off":"offline","pl_avail":"online","pl_not_avail":"offline","device":{"name":"OpenMQTTGateway2","manufacturer":"OMG_community","sw_version":"0.9.2","identifiers":["840D8E658E66"]}} homeassistant/sensor/840D8E658E66gatewaySRFB/config {"stat_t":"home/OpenMQTTGateway2/SRFBtoMQTT","name":"gatewaySRFB","uniq_id":"840D8E658E66gatewaySRFB","val_tpl":"{{ value_json.value }}","device":{"name":"OpenMQTTGateway2","manufacturer":"OMG_community","sw_version":"0.9.2","identifiers":["840D8E658E66"]}} homeassistant/binary_sensor/600194BFC9E7/config {"stat_t":"home/OpenMQTTGateway1/LWT","name":"OpenMQTTGateway1","uniq_id":"600194BFC9E7","dev_cla":"connectivity","pl_on":"online","pl_off":"offline","pl_avail":"online","pl_not_avail":"offline","device":{"name":"OpenMQTTGateway1","manufacturer":"OMG_community","sw_version":"0.9.3beta","identifiers":["600194BFC9E7"]}} homeassistant/sensor/600194BFC9E7gatewaySRFB/config {"stat_t":"home/OpenMQTTGateway1/SRFBtoMQTT","name":"gatewaySRFB","uniq_id":"600194BFC9E7gatewaySRFB","val_tpl":"{{ value_json.value }}","device":{"name":"OpenMQTTGateway1","manufacturer":"OMG_community","sw_version":"0.9.3beta","identifiers":["600194BFC9E7"]}} '''

1technophile commented 4 years ago

not yet indeed, I will add it soon

1technophile commented 4 years ago

With this branch : https://github.com/1technophile/OpenMQTTGateway/tree/discovery-publish-once

OMG publish only at first MQTT connection the discovery messages.

@pensionado do you think you could test it and say me if it is ok for you? You can comment on the PR directly

pensionado commented 4 years ago

Will do, will report back in a couple of days.

pensionado commented 4 years ago

Tested the branch discovery-publish-one, with following findings: 1) No discovery message is broadcast at all, not under homeassistant topic nor base topic, 2) SYStoMQTT topic is being published as expected, 3) No SRFBtoMQTT messages are send at all, not even when sensor next to gateway. 4) LWT and version topic published under base topic, with retain.

Will reflash with traces enabled and will send trace log as soon as available.

ma-ochoa commented 4 years ago

I have tried the "publish-once" branch

The flag retain works correctly, the mqtt server retains the topics with which you can restart home assistant and the entity remains configured.

However, I have not received any temperature value from my 6 mijia sensors or my lywsd2 sensors.

When the 0.9.3beta branch was reinstaled, the temperature values ​​reappeared.

Since the topics were already held on the mqtt server in some ways the home assistant restart continues to work.

1technophile commented 4 years ago

Hello,

Thanks for the feedback.

I think I have found the issue, the sensor report correctly to MQTT but the discovery message is missing some infos:

{"stat_t":"/BTtoMQTT/582D3433AAF3","name":"MiJia-tem","uniq_id":"582D3433AAF3-MiJia-tem","dev_cla":"temperature","val_tpl":"{{ value_json.tem | is_defined }}","unit_of_meas":"°C","device":{"name":"OpenMQTTGateway_ESP32_M5STICK_C_BLE_IR","manufacturer":"OMG_community","sw_version":"version_tag","identifiers":["D8A01D47277C"]}}

We can see that stat_t attribute content miss the base topic and the gateway name.

I have made a correction on the same branch, if you could test it, it would be great.

pensionado commented 4 years ago

Tried thus latest update. but even that also didnt work. I have done 2 tests, the first with 2 bridges, one on the old code the other on the new code, the second with only one bridge on the old code (0.9.2). I have attached

  1. the log from my MQTT broker (mqttlog.txt) covering both tests, new bridge is ip 192.168.88.100, old bridge is ip 192.168.88.104, 2 the mqtt subscription logs of both tests rfbrdige5.txt (first with new code bridge in it) and rfbridge6.txt for second test,
  2. ip traces on router both first (sonoff10) and second test (sonoff11) in captures.zip captures.zip

mqttlog.txt rfbridge5.txt rfbridge6.txt

Will start analyzing data tomorrow, but must say mqttlog looks worrying for the next code.

1technophile commented 4 years ago

Tried thus latest update. but even that also didnt work.

When you say it didn't work, may I understand that your gateway still repeat the discovery publishing at a fixed interval or that you don't see the sensor appearing into HASS?

mqttlog.txt

That's strange, are you sure you don't have 2 gateways with the same name?

pensionado commented 4 years ago

Have the last 2 days been investigating, and was able to modify the programs so it behaved as expected; discovery message published with retain and only once at first mqtt connect. However, then testing with sensors to see if the expected messages were published, I discovered they were not (compared it with other bridge running 0.9.2 code). To exclude problems created by me modifying the program, I went back to the original 0.9.3beta, made only the necessary changes in user_config and flashed that and tested again. Code 0.9.3beta also does not react to rf sensors, I see the discovery, the systomqtt and further nothing. So it appears 0.9.3beta is already broken for SRFB. Did comparison between 0.9.2 and 0.9.3 but there were to may changes in main.ino for me to identify what may have broken it. Please advice how I can help.

pensionado commented 4 years ago

Found reason for not seeing sensors, switch was not on. Have been able to implement this feature request in 0.9.2 code by making following mods: main.io:

  1. after line 424: // Publish discovery pubMqttDiscovery();
  2. copied pub_custom_topic from 0.9.3 beta and replaced: pub(topicori, JSONmessageBuffer, true); with: client.publish(topicori, JSONmessageBuffer, true);
  3. commented in function stateMeasures call to pubMqttDiscovery()
1technophile commented 4 years ago

Thanks for this analysis, I reproduce the issue of repetitive MQTT connection with v0.9.3beta, I will investigate and keep you updated.

pensionado commented 4 years ago

To help you with that, I investigated the IP traces I send you, and from that I saw that on connection the LWT message was send and acknowledged, followed by a composite (publish) message of version and subscription to commands. However, the data for the subscription like topic was missing, so the program didnt get a subscribe ack and reset the connection, after a while the complete subscription became part of the message and the program received an ack for the subscribe and proceeded. I was investigating why this happened and what fixed it, then I found there was an easier solution to my enhancement request and started playing with it. Hence the solution I mentioned before, I have not been able to find out what caused this repeated mqtt connect due to missing parts of the subscription message. After I went back to 0.9.2 I found out that the missing rf messages was a problem of the on/off switch, so my comment about 0.9.3beta is probably not correct that it doesnt function for rf messages.

1technophile commented 4 years ago

Your diagnosis isrelevant, the issue comes from a bad method implementation for string concatenation, I'm currently finding the proper way to do it.

1technophile commented 4 years ago

I have updated the branch you should not have issue now with topics and missing data.

jhmoon2000 commented 4 years ago

Wouldn't we have clean-up code for retained messages? For example, clean up the device config message when the device was disconnected for, say, 6 hrs. I'm having trouble getting rid of moved devices' config messages in zigbee2mqtt.

ma-ochoa commented 4 years ago

actual developed branch works ok in home assistant.

pensionado commented 4 years ago

Feedback on 0.9.3rc testing:

Further excellent work, really love this OMG

1technophile commented 4 years ago

Thanks for the feedback!

  • Discovery messages is still being issued multiple times, due to the fact that pubMQTTDiscovery is executed from stateMeasures which gets executed every 2 minutes by default, if you move this after the publish announcement and version (line 534 in main.ino) , just after broker connection and delete the other call in another place of the program (which didnt work by the way).

Indeed sometimes merge are not going as they should be... I will remove the one in stateMeasure and move the one in the client.connected() section (it should have been before connectedOnce = true;

  • Device name of restart and reset switch needs to be unique, else only the last gateway that came up will have these switches and the action will only apply to that last one. (made a mod to name device gatewayname_restart/reset, that worked having multiple OMGs).

This is strange, the switch should have a name constituted of mac adress of the ESP + restart or erase, on your side you see only erase or restart as the switch name?

pensionado commented 4 years ago

On initial test, hass created entities switch.erase_omg and switch.restart_omg, since I have multiple OMGs this wasn't handy, because hass would create erase_omg_2, erase_omg_3 which would leave me to figure out which one it was hence the mod I made for myself, to prefix the name with the gateway name. Then I discovered that I had to delete the switch.erase_omg from hass for that to take effect. So the name of the entity was definitely not macaddres + restart or erase. These were the 2 MQTT messages published on first run: homeassistant/switch/600194BFC9E7restart/config {"stat_t":"home/OpenMQTTGateway1/LWT","name":"restart OMG","uniq_id":"600194BFC9E7restart","pl_on":"{\"cmd\":\"restart\"}","pl_avail":"online","pl_not_avail":"offline","cmd_t":"home/OpenMQTTGateway1/commands/MQTTtoSYS/config","device":{"name":"OpenMQTTGateway1","manufacturer":"OMG_community","sw_version":"version_tag","identifiers":["600194BFC9E7"]}} and homeassistant/switch/600194BFC9E7erase/config {"stat_t":"home/OpenMQTTGateway1/LWT","name":"erase OMG","uniq_id":"600194BFC9E7erase","pl_on":"{\"cmd\":\"erase\"}","pl_avail":"online","pl_not_avail":"offline","cmd_t":"home/OpenMQTTGateway1/commands/MQTTtoSYS/config","device":{"name":"OpenMQTTGateway1","manufacturer":"OMG_community","sw_version":"version_tag","identifiers":["600194BFC9E7"]}}

pensionado commented 4 years ago

In looking back at my last comment, I see you gave them uniq_id with mac adress prefixed, but that is not what hass uses as the name of the entity, it used the name.

1technophile commented 4 years ago

In looking back at my last comment, I see you gave them uniq_id with mac adress prefixed, but that is not what hass uses as the name of the entity, it used the name.

Do you think we should use the same value into unique_id and name field = unique_id field?

pensionado commented 4 years ago

Do you think we should use the same value into unique_id and name field = unique_id field?

Yes. at least for the switch entities, they are most likely going to be used. Looking at it from a higher perspective, currently gatewayname is used in the MQTT messages and mac address is used for constituting uniq-ids (these are used by hass internally). Currently I use gatewayname as a unique name in my environment (and my local mods make them part of the name, so I can distinguish in hass from the entity name which OMG it is), so I know which rfbridge (in my case) processed the messages, this defeats the purpose of preventing duplicates.

I would like to suggest following modifications/suggestions:

I realize this makes things more complicated, but at the moment gatewayname is used for multiple purposes, creating sort of conflicts, between duplicate prevention and identification.

1technophile commented 4 years ago

I take your comments and open a separate issue. As this one has been taken into account. Thanks