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

MQTT disconnect message not implemented (IDFGH-835) #3215

Closed
gregjesl opened this issue Mar 21, 2019 · 5 comments
Closed

MQTT disconnect message not implemented (IDFGH-835) #3215

gregjesl opened this issue Mar 21, 2019 · 5 comments

Comments

@gregjesl
Copy link
Contributor

Duplicate issue of espressif/esp-mqtt#97 (which does not appear to be tracked in your issue tracking database).

Issue

The MQTT client never sends the DISCONNECT packet to the broker. This results in the last will and testament always being sent, which is not the intended behavior.

Solution

I submitted the fix in espressif/esp-mqtt#106

@gregjesl
Copy link
Contributor Author

@david-cermak I targeted the idf branch in my pull request but I branched from the master branch. Should I branch from the idf branch for my pull request?

@github-actions github-actions bot changed the title MQTT disconnect message not implemented MQTT disconnect message not implemented (IDFGH-835) Mar 21, 2019
@david-cermak
Copy link
Collaborator

@gregjesl Thank you for your help with this issue!

Yes please keep the idf as target branch (will soon merge it to master), but you seem to have to rebase onto idf.

@gregjesl
Copy link
Contributor Author

Closed per espressif/esp-mqtt@caf5007

@gregjesl
Copy link
Contributor Author

Reopened - submodule needs to be updated.

@gregjesl gregjesl reopened this Mar 29, 2019
@mastertheknife
Copy link

mastertheknife commented Jul 5, 2019

Hi,
This is also the case with ESP8266, on the latest SDK (RTOS v3.2 - released 22 days ago)
As a temporary workaround for my project, i copied these changes into esp_mqtt_client_stop() and they work well.
I can confirm that the change (and the subsequent one to only disconnect if connected: espressif/esp-mqtt#118) work on the ESP8266:

--- mqtt_client.c.orig	2019-07-05 22:37:13.604686922 +0300
+++ mqtt_client.c	2019-07-05 22:53:15.787319054 +0300
@@ -788,6 +788,12 @@
 esp_err_t esp_mqtt_client_stop(esp_mqtt_client_handle_t client)
 {
     if (client->run) {
+        if(client->state == MQTT_STATE_CONNECTED) {
+            client->mqtt_state.outbound_message = mqtt_msg_disconnect(&client->mqtt_state.mqtt_connection);
+            if (mqtt_write_data(client) != ESP_OK) {
+                ESP_LOGE(TAG, "Error sending disconnect message");
+            }
+        }
         client->run = false;
         xEventGroupWaitBits(client->status_bits, STOPPED_BIT, false, true, portMAX_DELAY);
         client->state = MQTT_STATE_UNKNOWN;

@igrr igrr closed this as completed in 6289a26 Jul 25, 2019
trombik pushed a commit to trombik/esp-idf that referenced this issue Aug 9, 2019
0xFEEDC0DE64 pushed a commit to 0xFEEDC0DE64/esp-idf that referenced this issue May 5, 2021
espressif#3364)

Especially if the user wants to use the library as component in IDF,
there are some pitfalls while doing make menuconfig. One is this missing
dependency which will now fail with a better error message with a hint to
the user how to fix it.

refs espressif#2154 espressif#3215
david-cermak added a commit to espressif/esp-mqtt that referenced this issue Dec 16, 2022
…x static analysis warnings

closes espressif/esp-idf#3619 including mqtt commit 7223302
closes espressif/esp-idf#3215 including mqtt commit caf5007
egnor pushed a commit to egnor/esp-mqtt that referenced this issue Dec 23, 2022
egnor pushed a commit to egnor/esp-mqtt that referenced this issue Dec 23, 2022
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

No branches or pull requests

3 participants