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

(ESP32) Support for larger MQTT payloads #6

Closed
proddy opened this issue Feb 26, 2021 · 18 comments
Closed

(ESP32) Support for larger MQTT payloads #6

proddy opened this issue Feb 26, 2021 · 18 comments
Assignees
Labels
enhancement New feature or request

Comments

@proddy
Copy link
Contributor

proddy commented Feb 26, 2021

As noticed with emsesp/EMS-ESP#715 with setups containing multiple heating circuits on a single thermostat, the MQTT payload may exceed the EMS-ESP buffer limit of 1024 bytes. The threshold for the TCP lwip stack is 1072 for (or 2090 if using lwip2 and compiling with -DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH). See emsesp/EMS-ESP#556 (comment).

In theory we can up the EMS-ESP buffer and possibly switch from static to dynamic heap, if there is room.

But I'd also like to explore MQTT chunking again so will experiment with the asyncMqttclient libs

@proddy proddy self-assigned this Feb 26, 2021
@MichaelDvP
Copy link
Contributor

I'm a bit confused, Is this an enhancement for esp32 or esp8266 or both?

(or 2090 if using lwip2 and compiling with -DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH)

I find this option only for esp8266: https://docs.platformio.org/en/latest/platforms/espressif8266.html#lwip-variant for esp32 there seems not to be such a option? https://docs.platformio.org/en/latest/projectconf/section_env_platform.html Is this also a limit for esp32?

In theory we can up the EMS-ESP buffer and possibly switch from static to dynamic heap, if there is room.

The v3 uses dynamic 4k buffer, static 1k is only in esp8266.

@proddy
Copy link
Contributor Author

proddy commented Feb 26, 2021

it is confusing! I wanted to create an issue so we have all the details in one place. It's also a place holder for an updated version of asyncmqttclient that supports chunking, to go over the lwip buffer limit for large payloads. That's what I'm playing around with now

@proddy
Copy link
Contributor Author

proddy commented Feb 27, 2021

updated the mqtt libs for v3 only.

@proddy proddy closed this as completed Feb 27, 2021
@MichaelDvP
Copy link
Contributor

@proddy : I see a issue with this new lib, after a mqtt-message is sent to ems-esp (on_message), the lib stops sending out messages and the queue is filled (mqtt stays connected). Sometimes i get a disconnect/reconnect a minute later and mqtt sends out messages again, sometimes not.

20210301_125418_EMS.log

After some tests/retries and reconnects the message is received 9 times and mqtt sending is also stopped.
EMS2.log

I installed the old mqtt lib and it works again without these issues.

Can you reproduce this?

@proddy
Copy link
Contributor Author

proddy commented Mar 1, 2021

haven't seen that on my side and have been running fine for the last days. I'll do some deep diving and see what I can find. We can always roll back to the previous lib.

@proddy proddy reopened this Mar 1, 2021
@MichaelDvP
Copy link
Contributor

I've tested now with the prebuilt bin to make sure it is not on my side. I can reproduce the issue, but it may be only with my mqtt-broker. Btw: the broker shows last-will message "offline".

@MichaelDvP
Copy link
Contributor

With qos 0 it's working ok, But qos 1/2 hang after on_message. Switching back to qos 0 reconnects, but i tstill hang, i have to reboot the esp.

@proddy
Copy link
Contributor Author

proddy commented Mar 1, 2021

ok, I think I may know where its failing. I'll reproduce here too with qos1

@MichaelDvP
Copy link
Contributor

It's failing in process_queue, line 694 (debug message inserted to check).

    // if this has already been published and we're waiting for an ACK, don't publish again
    // it will have a real packet ID
    if (mqtt_message.packet_id_ > 0) {
        LOG_DEBUG(F("Waiting for QOS-ACK"));
        return;
    }

The messages after the incoming message are not sent and we do not receive the ack in on_publish

@proddy
Copy link
Contributor Author

proddy commented Mar 1, 2021

yes I expected it was in that piece of code. In the old library there was no good support for qos1+2 so I implemented in code. I think all this can be removed now. I'm working on some big changes so hold of on any PRs until I've done this push

@proddy
Copy link
Contributor Author

proddy commented Mar 2, 2021

I did reproduce this as well - I'll look into what needs fixing tonight

@MichaelDvP
Copy link
Contributor

I've removed the qos-code and the on_publish callback, but it does not help. With qos 1: After the on_message (works) the next messages get a pid back but are not sent out. Mqtt-queue is always empts then.

ems-esp2:/$ 000+00:08:08.058 D 32: [mqtt] Received boiler => {"cmd":"wwtemp","data":"47"} (length 28)
ems-esp2:/$ 000+00:08:08.058 I 33: [boiler] Setting boiler warm water temperature to 47 C
ems-esp2:/$ 000+00:08:08.482 D 39: [mqtt] Publishing topic ems/boiler_data (#120, retain=0, try#1, size 604, pid 16)
ems-esp2:/$ 000+00:08:08.533 D 40: [mqtt] Publishing topic ems/boiler_data_ww (#121, retain=0, try#1, size 411, pid 17)
ems-esp2:/$ 000+00:08:08.584 D 42: [mqtt] Publishing topic ems/boiler_data_info (#122, retain=0, try#1, size 79, pid 18)

After some time mqtt reconnects

ems-esp2:/$ 000+00:09:30.245 D 110: [mqtt] Publishing topic ems/mixer_data (#141, retain=0, try#1, size 85, pid 37)
ems-esp2:/$ 000+00:09:38.171 I 111: [mqtt] MQTT disconnected: TCP
ems-esp2:/$ 000+00:09:39.207 I 112: [mqtt] MQTT connected
ems-esp2:/$ 000+00:09:39.207 D 113: [mqtt] Publishing topic ems/info (#142, retain=0, try#1, size 62, pid 38)
ems-esp2:/$ 000+00:09:39.258 D 114: [mqtt] Publishing topic ems/heartbeat (#143, retain=0, try#1, size 252, pid 39)

and next messages works ok until next incoming message.

@proddy
Copy link
Contributor Author

proddy commented Mar 2, 2021

interesting, does this happen only after an incoming message is received by EMS-ESP? I don't have time to write a sketch to reproduce it but can post a report to mqttasyncclient outlining the steps

  • set QOS to 1
  • publish something (works)
  • receive an incoming MQTT (works)
  • publish something again (fails, get's a Packet ID but is not transmitted)

is that correct?

@MichaelDvP
Copy link
Contributor

Yes, exactly.
As addition: in step 2 after publishing on_publish is triggert, in step 4 there are no on_publish called.

@bertmelis
Copy link

Does the client disconnect after a while? That is, after the keepalive times out. Or is it just stuck?

@proddy
Copy link
Contributor Author

proddy commented Mar 2, 2021

thanks for the quick fix @bertmelis . @MichaelDvP I've made the change, so we can test

@MichaelDvP
Copy link
Contributor

@bertmelis Yes it have disconnected after ~30sec. With your fix all is working well now. Thanks.

@proddy Tested qos 0, 1, 2 and all is ok now. I've gone back to your original mqtt.ccp with some extra debug messages. You can close the issues here and there. And we should add in mqttClient_->onDisconnect: mqtt_messages_.clear();, otherwise it hangs forever if the top message is waiting for ack on reconnect.

@proddy
Copy link
Contributor Author

proddy commented Mar 2, 2021

@MichaelDvP I don't mind if we go with your version of mqtt.cpp. We can never have enough debug messages, just wrap them in #if defined(EMSESP_DEBUG). I was actually thinking of adding debug levels, like -DEMSESP_DEBUG=2 so when we're debugging MQTT or the Dallas stuff we don't get all the EMS crap

@proddy proddy closed this as completed Mar 4, 2021
@proddy proddy transferred this issue from emsesp/EMS-ESP Mar 14, 2021
@proddy proddy added the enhancement New feature or request label Mar 15, 2021
proddy added a commit that referenced this issue Dec 4, 2021
proddy referenced this issue in proddy/EMS-ESP32 Jun 11, 2023
proddy referenced this issue in proddy/EMS-ESP32 Jun 13, 2023
MichaelDvP pushed a commit to MichaelDvP/EMS-ESP32 that referenced this issue Oct 12, 2024
small changes, use commands to change dashboard values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants