alexCajas / EmbeddedMqttBroker

This is a Mqtt broker for embedded devices, developed in C++, FreeRTOS to use advanced multitasking capabilities, and arduino core. Tested in an Esp32 and esp8266.
https://github.com/alexCajas/EmbeddedMqttBroker
MIT License
71 stars 14 forks source link

Cannot connect from paho client library #4

Closed SteeveGL closed 1 year ago

SteeveGL commented 1 year ago

I have an issue with this snippet.:

https://github.com/alexCajas/EmbeddedMqttBroker/blob/1917a4480da34aa8a2454c3d3d09c3243a291d03/src/ConcurrentTasks/NewClientListenerTask.cpp#L27-L31

I'm working on an app for android to connect to a MQTT broker, I'm using "org.eclipse.paho:org.eclipse.paho.client.mqttv3:1.2.5". This library has no issue to connect to other esp32 MQTT broker library excepted this one.

I found out others libraries do not validate client.available(). When I comment out those lines, the connection succeed.

Is a paho's library flaw?

alexCajas commented 1 year ago

HI! @SteeveGL, this issue is interesting, I don't now what to think, I was looking for any similar issue in [https://github.com/eclipse/paho.mqtt.java] and [https://github.com/espressif/arduino-esp32/], but I didn't find anything. My main question here is, if the issue happen in other mqtt broker libraries which uses WiFiClient::available() or not. Let me do some research about that and I will comment what I find. Probably I will write a version of this library compatible with eclipse/paho.client.mqtt, (when I have that, let me now if the issue is fixed using your app! thanks! ) and I will open an issue on [https://github.com/espressif/arduino-esp32/].

SteeveGL commented 1 year ago

So, I investigate a little more and I found a fix.

I find out the client.available() return false because for some reasons the Paho's library take time to fill up the buffer. So, we need to wait.

My previous workaround probably works because the moment between the client got from the tcpServer->available() and the ConnectMqttMessage connectMessage = messagesFactory.getConnectMqttMessage(client) read the buffer, is long enough to get the buffer filled. But client.available() is called too soon.

I added a very simple timeout loop to wait to get the buffer filled up:

...
int count = 1000; // 1 sec
for (size_t i = 0; i < count; i += 10)
{
  if (client.available())
    break;
  vTaskDelay(100);
}
...

The function in my code now looks like that:

void NewClientListenerTask::run(void *data)
{
  tcpServer->begin();
  while (true)
  {
    // Check for a new mqtt client connected
    WiFiClient client = tcpServer->available();
    if (!client)
    {
      vTaskDelay(10); // yield cpu, This line is necessary for meet freeRTOS time constrains
      continue;       // next iteration
    }

    // Waiting client to send mqttpacket
    int count = 1000; // 1 sec
    for (size_t i = 0; i < count; i += 10)
    {
      if (client.available())
        break;
      vTaskDelay(100);
    }

    if (!client.available())
    {
      continue; // next iteration.
    }

    /** reading bytes from client, in this point Broker only recive and
     acept connect mqtt packets **/
    ConnectMqttMessage connectMessage = messagesFactory.getConnectMqttMessage(client);

    if (!connectMessage.malFormedPacket() && !broker->isBrokerFullOfClients())
    {
      sendAckConnection(client);
      broker->addNewMqttClient(client, connectMessage);
    }
  }
}
alexCajas commented 1 year ago

Yes! That is, I have come to the same conclusion, but depending of the time to wait in the loop, others mqtt libraries fails to XD, testing with org.eclipse.paho.mqttv5.client, it fails some times but org.eclipse.paho.client.mqttv3:1.2.5 had success always, for now I using vTaskDelay(10) and waiting at most 1500, and it looks good. I will do some test with others mqtt tecnologies and I will update in a few days.