256dpi / arduino-mqtt

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

Strange issue with .publish() + question regarding role of .loop() #222

Closed ArnieO closed 3 years ago

ArnieO commented 3 years ago

I am using the library on an ESP32 that serves as a gateway for a unit that transmits data using ESP-NOW, that cannot easily be used while Wifi is active. To overcome this, I do the following after each reception of data from ESP-NOW:

I found two issues that I do not understand:

1) .publish() does not publish if I try to exploit its boolean return value Used like this I get the error message if publish does not "succeed", but nothing is published:

if (!mqtt_client.publish(mqtt_publish_topic, msg.c_str()))  // For some reason this does not send to MQTT,
           DPRINTLN("ERROR sending to MQTT *******************************************");

I verified the correct boolean return value by reducing the MQTT buffer length.

This behaves in the same way: bool mqttOk = mqtt_client.publish(mqtt_publish_topic, msg.c_str());

So in order to get the message published AND catch the error message, my code now looks like this:

mqtt_client.publish(mqtt_publish_topic, msg.c_str()); 
if (!mqtt_client.publish(mqtt_publish_topic, msg.c_str())) 
        DPRINTLN("ERROR sending to MQTT *******************************************");

This only gives one MQTT message, and triggers the error message if publish fails. Is this consistent with expected behaviour?

2) The role of .loop() I discovered that some messages were not published, I came to think of the .loop() function - and discovered to my astonishment that I had forgotten to insert it in the code. Nevertheless, the code published to MQTT - but not consistently. Inserting mqtt_client.loop() in the main loop() of my code makes no difference, probably since Wifi is not active there (only ESP-NOW). So I inserted mqtt_client.loop() immediately after the .publish(), before turning off Wifi and reinitialization of ESP-NOW: image

And this works fine! The unit now consistently publishes each MQTT message (still only one time, despite the double call to .publish()).

The role of the .loop() function is not described in the API. Hence my question: What is the role of the .loop() function, and how should it best be used?

256dpi commented 3 years ago

1) Surely the double call to publish should not be necessary. However, without having more context it's had to figure out whats' going on here. Maybe you can upload a stripped down sketch that still shows this behaviour?

2) The loop function maintains general connection state and receives incoming messages from the broker.

ArnieO commented 3 years ago
  1. Surely the double call to publish should not be necessary. However, without having more context it's had to figure out whats' going on here. Maybe you can upload a stripped down sketch that still shows this behaviour?

As indicated, I carefully tested step by step to confirm that the double publish() call was indeed necessary - and indeed only generates one MQTT message. It will be quite a bit of work to strip down code while preserving the switching between ESPNOW and Wifi (MQTT), so I'm afraid I cannot promise to do that as long as the code works (it still does).

  1. The loop function maintains general connection state and receives incoming messages from the broker.

In this case the client never receives messages from the broker, it only publishes to broker. And at that point I do not need to maintain connection, as I will switch back from Wifi/MQTT to ESPNOW. Does it mean that in this case the call to loop() can be omitted?

256dpi commented 3 years ago

Yes, if the client is only used briefly and does not receive messages, you can omit calling loop as it does not need to manage the keep alive interval.

ArnieO commented 3 years ago

Thank you for the clarification of loop()!

256dpi commented 3 years ago

I'm closing this for now. Please reopen if you think there is still an issue in the library here.