256dpi / arduino-mqtt

MQTT library for Arduino
MIT License
1.01k stars 230 forks source link

Publish method blocking on ESP32. #272

Closed raphael-bmec-co closed 2 years ago

raphael-bmec-co commented 2 years ago

I am not 100% sure yet but it seems as though the publish method may contain several blocking methods.

From lwmqtt_publish in client.c, lwmqtt_send_packet_in_buffer calls lwmqtt_write_to_network which contains a while loop with each loop containing a network call. Should this not include a yield/vtaskdelay?

Similarly, from lwmqtt_publish in client.c, lwmqtt_cycle_until contains a while loop with each loop. Should this not include a yield/vtaskdelay?

Looking forward to some feedback from someone in the know.

raphael-bmec-co commented 2 years ago

@256dpi if you have a minute to give your thoughts on this I would really appreciate it. I now understand this is actually in the underlying implementation. We have currently modified the library but I am wondering if you have any thoughts on the correct way to go about resolving this? I can't see a justification for the lwmqtt project adding ESP32 specific support.

256dpi commented 2 years ago

I assume that on multi-tasking systems, network calls automatically yield. For example on ESP32, a read/send on a socket should result in a push/pop to/from a queue that will yield to scheduler if blocked. The loops are only there to ensure we send/read all the data we need. Maybe I'm wrong, and maybe this is not the case on other platforms. But, in order to make a change we would need to investigate this further and do proper testing. However, I lack the time do to this at the moment.

Since you're using ESP32, I heavily recommend to use the esp-idf directly instead of the Arduino API. If you do, you cannot only write more performant applications with the native APIs, but you can also use my https://github.com/256dpi/esp-mqtt component that is a fully asynchronous MQTT client.

raphael-bmec-co commented 2 years ago

@256dpi Thanks for this feedback. There are some other issues that may have been causing this. We will keep an eye on this and provide feedback.

The esp-idf is a great option and I have spent some time trying to implement the esp-mqtt library in Arduino. Unfortunately, some of the flags in the Arduino config preclude this for secure connections.

We use a lot of Arduino libraries so using Arduino as a component is our long term goal but at the moment platformIO does not offer good support for this. However, they seem to have firmed up there relationship with Espressif recently so our fingers are crossed for better support soon.

Thanks again.

raphael-bmec-co commented 2 years ago

Seems like this may have been related to other issues in the core. Closing.

Decezaris commented 6 months ago

Same problem here! The publish is blocking the core. :/