Skip to content
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

Prevent MQTT Client exception #822

Closed
wants to merge 3 commits into from
Closed

Conversation

borpin
Copy link
Contributor

@borpin borpin commented Apr 9, 2018

Issue - subscribing to a topic when the MQTT client was not connected caused an exception; not fatal but not nice. This has been noticed on a number of topics recently on the Forum.

Documentation

The onConnect callback is only called if there is a CONNACK from the MQTT server.

The onConnect callback assumed than any call to it meant a successful connection. This is not correct, only a response of '0' means success, so $connected now set as such.

The reconnect try, did not check the connected flag before attempting to subscribe to the topic. By doing so the exception can be avoided.

Issue - subscribing to a topic when the MQTT client was not connected caused an exception; not fatal but not nice.

The onConnected callback is only called if there is a CONNACK from the server.

The onConnected callback assumed than any call to it meant a successful connection.  This is not correct, only a response of '0' means success so $connected now set as such.

The reconnect try, did not check the connected flag before attempting to subscribe to the topic.  By doing so the exception can be avoided.
@TrystanLea
Copy link
Member

Hello @borpin just tested this here and it doesnt seem to work for me. If I restart mqtt_input it does not detect any inputs?

@borpin
Copy link
Contributor Author

borpin commented Apr 29, 2018

Odd. What do you see in the logs?

…tion

Modified the logic around the connection to and subscription to MQTT server to prevent exceptions if there is no connection to be made.  The subscription ID is returned in the log.

A persistent connection is made as per @pb66 code.  The ID returned now, is always the same number. Without it, the number returned increased.
@borpin
Copy link
Contributor Author

borpin commented May 9, 2018

Yes, it was because the callback takes a little while so the code never got back into the subscription code. Split the connection and subscription up and added a variable that will take the topic ID on successful subscription and 0 if not subscribed.

I have also added in #823 code to create a persistent connection.

Without this, it can be seen that the ID returned on subsequent subscriptions (forced by restarting the mqtt_input daemon) increases. With it, the ID is the same. Using node-red, and using a QOS of 2, retained = true, data that was in the MQTT buffer, comes across (at least one value).

A loss of connection to the MQTT broker has been tested by stopping the MQTT broker (on a remote machine).

@TrystanLea
Copy link
Member

Hello @borpin I've uploaded a version of this that fixes the conflict to test here:
#822

I will close this PR

@TrystanLea TrystanLea closed this Jul 15, 2018
TrystanLea added a commit that referenced this pull request Jul 26, 2018
@borpin borpin deleted the bpo-MQTTClient branch January 15, 2019 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants