Enhance MQTT notifications #108
Conversation
|
Support for OSPi, OS2.3 and OS3.0 (only tested on esp dev board). This also adds keep-alive messages with the broker and an availability topic. Requires specific libraries to be installed. For OSPi you need the latest mosquitto libraries via |
mqtt.cpp
Outdated
| #if defined (ARDUINO) | ||
| static int last_state = 999; | ||
| if (!mqtt_client->connected() && (now - last_reconnect_attempt >= (unsigned long)MQTT_RECONNECT_DELAY*1000)) { | ||
| if (mqtt_client->connect(_id, NULL, NULL, "opensprinkler/availability", 0, true, "offline")) |
There was a problem hiding this comment.
Note here the issue with PubSubClient that has a timeout of 20 seconds. So each MQTT_RECONNECT_DELAY, you could lock the program on this line for 20 seconds, meaning that it won't react to anything.
I strongly recommend a non blocking client like AsyncMqttClient
There was a problem hiding this comment.
That remark is of course important when using the wifi on esp8266 with weak signal. That's a real pain. For wired connection, you shouldn't loose your connection and PubSubClient is an easy acceptable multi arch/board solution.
There was a problem hiding this comment.
Yeah, I agree. I have the async libraries working but the problem child is OS2.3.
Both PubSubClient and UIPEthernet introduce timeouts (DNS Lookup, TCP Connection and MQTT Sync). The timeouts can be de-tuned but UIPEthernet does a lot more than just support MQTT.
The issue is really down to support for keep-alive and LWT. I'm reluctant to give up on these as "availability" is an important feature for "set and forget" devices. I'm going to play around a bit more to see if I can find a happy medium. We may need different solutions for each platform (i.e async mosquitto for OSPi, async-mqtt for OS3.0 and something cut down for OS2,3) but I would prefer to keep the variations to a minimum if possible.
@jbaudoux , Let me know if you have thoughts/insight into OS2.3 solutions.
There was a problem hiding this comment.
Looking to close out this comment and appreciate any feedback.
There are currently three "timeouts" that impact MQTT network traffic and can block the main loop whilst establishing an MQTT connection as follows:
- DNS Lookup - A 15 second timeout if the network is down or the DNS server is unavailable
- TCP Connect - A 25 second timeout if the MQTT server port is closed
- MQTT Sync - A 15 second timeout if the MQTT Broker service is not responding
Note that these are not additive but rather the first timeout will exit the connection attempt so the maximum block would be 25 seconds (numbers based on OS2.3 which is worst case). This is similar for other outbound traffic such as HTTP Stations and IFTTT Notifications. However, MQTT is more susceptible to blocking as the "Keep Alive" communication happens on a very regular basis e.g. typically acks are sent every 15-60 second. Meaning that with default settings the main loop could be blocked for 25 seconds every minute if say the MQTT server was having issues.
I would like to maintain keep-alives as they provide a mechanism to detect OS going offline and allow for automations that alert the user. Very useful in "set and forget" equipment. Unfortunately, there aren't async versions of the libraries available across all platforms and for those platforms that do have libraries available (OSPi and OS3.0 WiFi) then all of the networking code base would need to be re-factored to transition to async communication. So I'm proposing is to remain with the existing synchronous architecture but adjust the KeepAlive and BackOff periods to maintain a balance between responsiveness and ensuring minimal disruption to the main event loop.
A very conservation approach would be to extend the KeepAlive period to 60 minutes (i.e. the Broker will recognise that OS is offline after an hour). To ensure the firmware doesn't block the event loop every hour for around 25 seconds then we can set a back-off period of 24 hours i.e. following an MQTT connection failure then Firmware would wait for 24 hours before attempting to reconnect. This is very conservative but would have limited impact on the main sprinkler functionality. The downside is that if there is a patchy network signal then the user may end up with significant number of "blackout" periods with no MQTT notifications.
An more aggressive approach would be to re-code the various timeouts in the libraries, say to 6 seconds. This would impact more than just MQTT functionality but would mean the KeepAlive period could be say 5 minutes and the back-off period down around an hour. It would mean modifying the libraries as they don't have apis for these settings.
@rayshobby , I would appreciate thoughts on this to get the balance right.
There was a problem hiding this comment.
I think this issue is mainly in case of wifi connection with esp8266. Using asyncTCP is for sure the best way to solve this. And once esp8266 core 3.0.0 will be released, you could normally drop UIPEthernet. But that means splitting code in arduino for esp8266 and non esp8266.
|
I think indeed that it can be a further enhancement. FYI. Esp supports
async tcp. Core provides lwip driver for embedded wifi chip and soon there
will be support for wired Ethernet with chip w5100/w5500. Full async tcp
stack wifi and Ethernet is nearly there. PR on core is under review.
I have ospi only, I'll test your PR with it. I don't know the differences
with the OS versions.
Le sam. 16 nov. 2019 à 21:14, PeteBa <[email protected]> a écrit :
… ***@***.**** commented on this pull request.
------------------------------
In mqtt.cpp
<#108 (comment)>
:
> + DEBUG_LOGF("MQTT Publish: Failed (%s)\n", mosquitto_strerror(rc));
+ }
+#endif
+}
+
+// Need to regularly call the loop function to ensure "keep alive" messages are sent to the broker and to reconnect if needed.
+void OSMqtt::loop(void) {
+ static unsigned long last_reconnect_attempt = 0;
+ unsigned long now = millis();
+
+ if (mqtt_client == NULL || !_enabled || os.status.network_fails > 0) return;
+
+#if defined (ARDUINO)
+ static int last_state = 999;
+ if (!mqtt_client->connected() && (now - last_reconnect_attempt >= (unsigned long)MQTT_RECONNECT_DELAY*1000)) {
+ if (mqtt_client->connect(_id, NULL, NULL, "opensprinkler/availability", 0, true, "offline"))
Yeah, I agree. I have the async libraries working but the problem child is
OS2.3.
Both PubSubClient and UIPEthernet introduce timeouts (DNS Lookup, TCP
Connection and MQTT Sync). The timeouts can be de-tuned but UIPEthernet
does a lot more than just support MQTT.
The issue is really down to support for keep-alive and LWT. I'm reluctant
to give up on these as "availability" is an important feature for "set and
forget" devices. I'm going to play around a bit more to see if I can find a
happy medium. But suspect we will need different solutions for each
platform (i.e async mosquitto for OSPi, async-mqtt for OS3.0 and something
cut down for OS2,3).
@jbaudoux <https://github.com/jbaudoux> , Let me know if you have
thoughts/insight into OS2.3 solutions.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#108?email_source=notifications&email_token=AB3Q6ZMPINSNKX7AU7W7CLTQUBIBRA5CNFSM4JADSIOKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCL2BTAQ#discussion_r347105682>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3Q6ZNQQXAQ5BAZXCX4AFLQUBIBRANCNFSM4JADSIOA>
.
|
|
@PeteBa I tried your PR with OSPI but I cannot connect to Firmware with App 2.1.4. What should I do to make it work without loosing my config? |
|
@jbaudoux , you need to either host your own version of the App with the associated MQTT PR included or you need to "hardcode" Firmware with your local settings. I suggest the later if you just want to test thing out. To hardcode your settings then you can change Line 142 of I would strongly recommend against using you main OSPi system to test this PR as there is no guarantee that your config will not be corrupted and that the system will operate properly. |
|
@PeteBa That's what I did for testing and connection to MQTT is successful. But I could not connect with the App to test launching a station. I'm currently running my PR and I don't have that connection issue. I don't know what could be wrong :( |
|
Do you have firmware debug enabled ? You can edit Also, what error do you get in your browser when navigating to |
|
Yes I enabled debug. First thing I did ;-) Can't use the local server as it's not connected to Internet and I don't host the UI locally. Maybe due to the added options in the previous release that I don't run yet. I'll leave it like that for now. I don't want to break my production system as I'm not going back to the site for some months. |
|
@jbaudoux , just looking a little closer at the log you included. It shouldn't be possible for those two different Any chance you can double-check that part of the code (i.e. where you hardcoded the config settings from above) ? |
|
That's because I harcoded mqtt config after the setup and copied the log
line. Mqtt connection is successful, so issue must be elsewhere.
Looking at latest release note, some config data is now moved to file.
Maybe that's why it didn't worked.
Le dim. 24 nov. 2019 à 23:07, PeteBa <[email protected]> a écrit :
… @jbaudoux <https://github.com/jbaudoux> , just looking a little closer at
the log you included. It shouldn't be possible for those two different MQTT
Setup lines to occur together as they are in the different blocks of the
if/else conditional:
19-11-24 15:42:24 - MQTT Setup: Config (None) Disabled
19-11-24 15:42:24 - MQTT Setup: Config (localhost:1883) Enabled
Any chance you can double-check that part of the code (i.e. where you
hardcoded the config settings from above) ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#108?email_source=notifications&email_token=AB3Q6ZO3ULP24BS5U5QUSZ3QVMCK3A5CNFSM4JADSIOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFAXMWI#issuecomment-557938265>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3Q6ZO77MQD6RFP727KBH3QVMCK3ANCNFSM4JADSIOA>
.
|
|
@jbaudoux , just wondering if you have had a chance to take another look at this ? To help narrow down any issue, could you try the current master branch to see if that works. If it does, then that would suggest a problem with the PR. If you have a spare Raspberry PI then would be good to try with a clean install to ensure there is no configuration files corrupt. |
|
I have spare rpi but no spare ospi shield to test. So without the hardware,
I don't know if that can be of any help
Le lun. 16 déc. 2019 à 02:02, PeteBa <[email protected]> a écrit :
… @jbaudoux <https://github.com/jbaudoux> , just wondering if you have had
a chance to take another look at this ? To help narrow down any issue,
could you try the current master branch to see if that works. If it does,
then that would suggest a problem with the PR. If you have a spare
Raspberry PI then would be good to try with a clean install to ensure there
is no configuration files corrupt.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#108?email_source=notifications&email_token=AB3Q6ZLIMK7PDN4FTJNAB63QY3HSRA5CNFSM4JADSIOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG5HI3I#issuecomment-565867629>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3Q6ZKKH5JO4ZYGTPYV3K3QY3HSRANCNFSM4JADSIOA>
.
|
|
You shouldnt need the OSPi shield to do basic testing as the RPI can run Firmware on its own (you just wont have physical relays/sprinklers to drive) but the web interface, mqtt functionality and basic operation (station on/off should work fine). It makes for a good stand-alone test environment if you dont want to disturb your main OSPI unit. From your note above, it seems that the App (2.1.4) wasnt connecting when using this PR. It would be good to see what is going on here so that we can fix if anything has been introduced incorrectly. Also, you might want to update to the latest App version 2.1.6 but I dont think anything new was introduced in the recent releases that would impact the App connecting with Firmware but worth ruling out. |
|
@rayshobby , Hey Ray, the last commit completes the majority of the changes to the basic MQTT functionality. There are updates to the message syntax that I would like to make and also to add in things like multi-site support but these will come in separate PRs. I am conscious that this PR is already getting fairly large so will pause here to get some feedback. If there are others monitoring this thread that want to try out the functionality - I suggest in a test environment 8) - then let me know and I'll be happy to provide guidance on set-up. I would appreciate the feedback opportunity. |
|
I don't tested yet but its working for OSPi? Just asking before update to this beta firmware. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I have hidden a few comments above as off-topic to de-clutter this PR. The comment discuss the REST api to OpenSprinkler and how it can be used to integrate with Home Assistant. They are still available to read for anyone interested. |
|
@PipeDeveloper , sorry for the delay responding as been on holiday. Yes, this works on OSPi but I suspect you may have found an alternative integration solution for HomeAssistant. Note that this PR does not allow for controlling an OpenSprinkler unit but rather generating notifications when its activity changes. I have included instructions below if anyone wants to try out the PR. I suggest using a new SD card with the latest Raspbian Buster so that you can go back to your original setup when finished trying. Firstly, ensure you have the right dependencies: Next, clone the OpenSprinkler repository and download the PR branch: The PR is meant to be used with an equivalent PR on the App so that you can configure your MQTT server IP and Port address via the UI but if you just want to try this out then you can change line 567 in main.cpp from Then build and run: You should see MQTT messages arrive in your broker when you turn stations on/off. let me know if you have any issues. |
|
This is interesting, and looks reasonable. I'm thinking of other ideas to build on top of, like reporting the wifi signal strength that I've seen in some other ESP8266 firmware by Anavi. |
|
Is there anything outstanding before this can be merged into master? Does it need more testing? |
|
@Nick-Adams-AU , status is in the comments above. Closing as I've moved on but code available for anyone to pick up and move forward. |
|
@rayshobby this is so close and something we’ve been wanting for years! Any chance you can take from here? |
|
MQTT always is better than an API. Also for control switches like a pump, read sensors, etc. |
|
Thanks PeteBa for the enhancements. I have reopened a new PR #113 to continue the work. |
This PR builds on the work from #102 and credit for the first commit of the PR has got to go to jbaudoux for starting the ball rolling on MQTT. The content has only been replicated here to facilitate rebasing onto the latest version 2.1.9 of Firmware and moving the content to a new feature branch. The second commit extends the functionality to allow configuring the MQTT Broker (server:port) via the App.
@jbaudoux, I asked Ray to create a new feature branch for this work so others can collaborate. I intend to work next on extending support for Last Will & Testament and Keep Alive, as well as, Arduino support. Appreciate your thoughts and involvement.