espressif / esp-mqtt

ESP32 mqtt component
Apache License 2.0
603 stars 255 forks source link

Allow to configure network timeout (IDFGH-3905) #166

Closed dshil closed 4 years ago

dshil commented 4 years ago

Hi @david-cermak, the origins of this PR is the following.

I have a secure connection to the MQTT broker. Sometimes WiFi stack is stacked for ~10seconds and network timeout is also 10 seconds, thus in most of the cases the connection is aborted and the reconnect process is issued. Since I use mbedTLS the reconnect process is very expensive (as you might know). My device has too few heap size and I want to avoid scenarios when I run out of heap.

For me, the simplest solution is to increase this timeout and to avoid the connection to be aborted. That's why I want this option to be configurable.

P.S. the initial error I had is:

13:00:41.881 [err] esp-idf: E (288902) MQTT_CLIENT: Error write data or timeout, written len = 0, errno=11

Then I tried to manually set it to 20s and then to 30s - work perfectly for me.

david-cermak commented 4 years ago

Hi @dshil

Thanks for posting this PR! LGTM, cherry-picked as https://github.com/espressif/esp-mqtt/commit/a03228ac4639eeb324af99dcded44a1f4113d3c3 I agree that having the network timeout configurable is a good thing. I've just added notes about default values and moved the option to the end of the struct.

dshil commented 4 years ago

Hi @david-cermak and thanks!

The last question, I need this change in IDF-v4.2, is it possible to update the corresponding submodule to the latest version in esp-idf repo? If yes I will prepare a PR.

david-cermak commented 4 years ago

@dshil Yes, no problem to reference this commit in v4.2 of IDF. We typically merge such IDF commit to master first and then backport it to all major releases. No need to post a PR to IDF repo for that, we usually do that automatically, but please be patient with us, as it might take some time until it gets pushed to the release branches.

dshil commented 4 years ago

@david-cermak got it. Will wait for the backport. Thanks!

dshil commented 4 years ago

Hi @david-cermak, apologies for the reminder but it seems that the option still not ported to master branch, correct? I understand you have a lot of another work to do, but probably I can elaborate somehow to speedup the process?

dshil commented 3 years ago

Hi @david-cermak, almost 1.5 months have passed and I just checked the esp-mqtt component in master branch, no updates. Probably it is updated internally. Anyway, I am very interested in this configuration parameter, any chance to see it in the upcoming v4.2 release?