search

espressif / ESP8266_RTOS_SDK

Latest ESP8266 SDK based on FreeRTOS, esp-idf style.
http://bbs.espressif.com
Apache License 2.0
3.34k stars 1.57k forks source link

esp-mqtt contains bug that is fixed in esp-mqtt repo (GIT8266O-480) #907

Open looi opened 4 years ago

looi commented 4 years ago

esp-mqtt in ESP8266_RTOS_SDK is unable to handle multiple MQTT messages in a single TCP packet. This bug was already fixed in esp-mqtt repo (https://github.com/espressif/esp-mqtt/). I have locally fixed the problem by patching in the changes:

espressif/esp-mqtt$ git format-patch -10 d4b66556186141e09db89c57781c6709e0e71d8a 0001-Add-BEFORE_CONNECt-event-and-refresh-the-connection-.patch 0002-Fix-for-case-where-multiple-MQTT-messages-are-fitted.patch 0003-added-a-fix-for-incomplete-header-message-received-t.patch 0004-mqtt-support-for-sending-fragmented-messages-and-ful.patch 0005-mqtt-data-events-fixed-to-keep-consistent-msg_id-whe.patch 0006-update-log-levels-logging-complete-error-code-remove.patch 0007-ci-added-basic-support-for-building-static-analysis-.patch 0008-Avoid-further-wait-period-after-reconnect-timeout-oc.patch 0009-Add-client-force-reconnect-function.patch 0010-minor-fixes-for-issues-present-with-fragmented-packe.patch

espressif/ESP8266_RTOS_SDK$ patch -p1 < *.patch

The result can be seen here: https://github.com/looi/ESP8266_RTOS_SDK/commit/9168c0f1c1c6f51d808e055f38c5c073e5eec934

trombik commented 4 years ago

like other components, esp-mqtt has been always lagged behind mostly because the component is NOT submoduled. i do not bother to create PRs because there are many PRs left unmaintained, some PRs are a few years old.

trombik commented 4 years ago

for those of you who want to use esp-mqtt instead of mqtt in the SDK, here is how I did in a project. https://github.com/trombik/esp32-homie/commit/e8b67211020ead14fb5f4e9df0a804851591b8c1

nsfilho commented 4 years ago

like other components, esp-mqtt has been always lagged behind mostly because the component is NOT submoduled. i do not bother to create PRs because there are many PRs left unmaintained, some PRs are a few years old.

This is a sad reality. I don't understanding that. All of us knows about esp32, but many projects are based on ESP8266 and isn't simple to replace a entire ecosystem. Please espressif read this post and update this project.

At least, unify this project with esp-idf (in this case, it is more easy to maintain).

trombik commented 4 years ago

I'm also frustrated, but I do understand why the company doesn't invest as much resources as they do in ESP32. ESP8266 is a great product for sure. but it's a product in the past.

nsfilho commented 4 years ago

@trombik I understanding but don't agree heheheh. Let me explain: if the product life cycle is too short, no one could build products based on then. Because if you use a product for 3 or 5 years, what happens? Are you going to loose your LTS? Why I'm telling that?

Because eletronics products are a little more durable products, it's not a season products (much of the case)....

But anyway, a solution for us, in mqtt (as I'm doing) sometimes is put delays between messages (when transmit), and put then in a circular queue when received to post-process (to maintain the receive pipe most quickly as possible)....

:)

ghost commented 4 years ago

Will this be fixed? I'm seeing issues which might well be from this specific bug (not sure though). I think I'm getting a wdt trigger, because memory or some other resource runs out for the task, because not all publish ack's are received/handled if there's a lot of messages at once, so the queue of un'acked messages fills up (at QOS1, at QOS0 no problem)

nsfilho commented 4 years ago

@trombik congratulations by your implementation. I have solved my problem using your code as reference. Really thank you by share your efforts with our community.

@cranphin yes, this solution fixed the problem. 😄

trombik commented 4 years ago

this workaround applies to not only MQTT component, but also any other components as long as newer component is compatible.

nsfilho commented 4 years ago

@trombik great!! You are 100% sure!! Can I ask you something? Why you discontinued your project? What do you think about create a repository with some esp-idf stuffs, like esp-idf-lib?

If you believe this is possible, I will help in this endurance!

Because most of the project needs the same things. Like mqtt, dht11/dht22, i2c lcd display / i2c oled and some other things, from from a button debouncing software until PID control system. My personal opensource project, I will try to implement most of then: https://github.com/nsfilho/e12aio3

ghost commented 4 years ago

I wonder if @donghengqaz could have a look at this, it seems like a fairly easy win, and a standard MQTT lib which doesn't crash a lot would be nice :)

trombik commented 4 years ago

@trombik Why you discontinued your project?

see "About" of the repository.

I wonder if @donghengqaz could have a look at this, it seems like a fairly easy win,

if you think fairly easy, why don't you do it yourself? you must not have looked at ignored PRs.

ghost commented 4 years ago

if you think fairly easy, why don't you do it yourself? you must not have looked at ignored PRs.

I hope it's fairly easy for someone working at Espressif, you have already laid a foundation by showing how to use the newer version of the mqtt library. But I don't know which direction they want to take, and which version of the library they want to use in the future, and with which release they want to do that, especially considering it looks like the new version has API changes. That's why it's probably not a good idea for me to attempt it :) Of course I could be wrong in all of that, I found asking or saying something silly is the quickest way to learn things :)