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 on -> off freezes the device #1115

Closed
jsponz opened this issue Aug 7, 2018 · 15 comments
Closed

MQTT on -> off freezes the device #1115

jsponz opened this issue Aug 7, 2018 · 15 comments

Comments

@jsponz
Copy link

jsponz commented Aug 7, 2018

With the last version, when switching off the MQTT, device freezes. I have to plug it off and plug it on again.

When switching on the MQTT, there is no problem.

Any hint?

@xoseperez
Copy link
Owner

Cannot reproduce the issue... can you check the debug log to see if there is any useful info there?

@jsponz
Copy link
Author

jsponz commented Aug 13, 2018

The problem is the library async-mqtt-client.

ESP just restarts and the web loses connection. You need to refresh the explorer.

@xoseperez
Copy link
Owner

How did you get to that conclusion? What version/revision of the library are you using?

@pniaps
Copy link

pniaps commented Aug 17, 2018

Hi @jsponz , @xoseperez
When you hit Save button, if MQTT is enabled, it disconnects and connects again, and the problem is in the library async-mqtt-client. When it disconects, it crashes. The ESP reboots, but the web remains frozen until you reload it.

The problem is

https://github.com/marvinroger/async-mqtt-client/blob/33cdd5038b8b411db47322908ca4eb503f630909/src/AsyncMqttClient.cpp#L707

function _sendDisconnect closes connection on line 661, so _client.send() on line 707 crashes.

https://github.com/marvinroger/async-mqtt-client/blob/33cdd5038b8b411db47322908ca4eb503f630909/src/AsyncMqttClient.cpp#L661

I will send a pull request to https://github.com/marvinroger/async-mqtt-client

@xoseperez
Copy link
Owner

Please, let me know when you send the PR.

@Misiu
Copy link

Misiu commented Sep 6, 2018

@pniaps did You send PR for this?

@Misiu
Copy link

Misiu commented Oct 1, 2018

@pniaps any news?

@pniaps
Copy link

pniaps commented Oct 14, 2018

Hi.
pull request sent marvinroger/async-mqtt-client#117

Sorry for the delay

@mcspr
Copy link
Collaborator

mcspr commented Oct 14, 2018

@pniaps I'll wait for someone to answer the pr you proposed, but isn't that the case of having network request inside system callback? Inside WiFi event callback that is. If you change the callbacks to set boolean flags and do the connection inside loop() instead everything works ok. And, in espurna case, that is exactly what it does.

I did track this for some time but could not reproduce the original issue :/ 2.3.0 ... 2.4.2 did ok switching mqtt on / off / on again. Switching ESPAsyncTCP version did not change a thing too.

@pniaps
Copy link

pniaps commented Oct 14, 2018

Will try, but callback or not, _client.send(); is after _client.close(true);

@mcspr
Copy link
Collaborator

mcspr commented Oct 14, 2018

Yep, by "connection" i meant doing both .connect() and .disconnect() calls on mqttClient.

Maybe this is related? me-no-dev/ESPAsyncTCP#75
ESPAsyncTCP version used by platformio is older , so maybe changing this:

https://github.com/me-no-dev/ESPAsyncTCP#55cd520

to this
https://github.com/me-no-dev/ESPAsyncTCP#0450e61

will help.

Wiki too mentions older version in Arduino IDE instructions
https://github.com/xoseperez/espurna/wiki/ArduinoIDE#installing-libraries-manually (note 1)

@mcspr
Copy link
Collaborator

mcspr commented Oct 19, 2018

cc @jsponz @Misiu
if you can reliably reproduce this, please test PR mentioned above: https://github.com/pniaps/async-mqtt-client
just unpack over old one if using arduino ide or for pio replace lib_deps address here with that link (whole string, #hash too):

lib_deps =
ArduinoJson
https://github.com/marvinroger/async-mqtt-client#v0.8.1

@stale
Copy link

stale bot commented Dec 18, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 18, 2018
@stale
Copy link

stale bot commented Dec 25, 2018

This issue will be auto-closed because there hasn't been any activity for two months. Feel free to open a new one if you still experience this problem.

@mcspr
Copy link
Collaborator

mcspr commented Mar 4, 2019

Some notes, since PR was never merged and this is still a possible bug:

  • reproducible only with async-mqtt-client git ((marvinroger/async-mqtt-client@33cdd50), not point release version 0.8.1
  • lwip's tcp_output function, being called by the second _client->send(), crashes after _client->close() sets pcb to NULL.. But, only in system context. If called inside loop() it behaves just fine, because close() is not instant.
  • esp32 async tcp library actually does check if pcb is not NULL, working around this bug: https://github.com/me-no-dev/AsyncTCP/pull/30/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants