plapointe6 / EspMQTTClient

Wifi and MQTT handling for ESP8266 and ESP32
GNU General Public License v3.0
444 stars 132 forks source link

Library fails with ESP32 Ethernet boards that use ethernet instead of WiFi #97

Open zzpaf opened 2 years ago

zzpaf commented 2 years ago

Description of the problem

When using ESP32 with ethernet, MQTT library fails to send as it is relying on WiFi being connected, is there any failsafe for this situation already built in that I did not see?

Versions

This happens on all versions

Hardware

C++ code

The following code stops MQTT from executing on "Ethernet-only" connections

bool EspMQTTClient::handleMQTT()
{
  // PubSubClient main loop() call
  _mqttClient.loop();

  // Get the current connextion status
  bool isMqttConnected = (isWifiConnected() && _mqttClient.connected());

I believe that we should remove the isWifiConnected or at least have a possibility to ignore wifi connectivity otherwise the MQTT library will never be able to work over Ethernet.

Since we have the _handleWiFi parameter, maybe we can use that in all if statements where we can use

if( ( _handleWiFi == false || isWifiConnected() ) && *****)

This way we can eliminate the checking for WiFi connectivity altogether in case we are not having MQTT Library manage WiFi connections.

Thanks, Pablo

plapointe6 commented 2 years ago

I have pushed the suggested fix into de dev branch. Thanks for pointing this out. I have to do more testing before merging the dev branch into master, but you can try it if you want !

zzpaf commented 2 years ago

Will test today and come back to you. Thanks!

zzpaf commented 2 years ago

I tried unsuccessfully to make it work from the dev branch, I think there might be some issues and I did not want to modify too many things:

I am getting errors on WiFiEvent from the EspSimpleWifiHandler library:: SYSTEM_EVENT_STA_DISCONNECTED has been renamed to?: ARDUINO_EVENT_WIFI_STA_DISCONNECTED

Same for: SYSTEM_EVENT_STA_GOT_IP has been renamed to?: ARDUINO_EVENT_WIFI_STA_GOT_IP

After fixing these I am still getting errors all related to WiFiEvent:

Users/XXX/Documents/XXX/Arduino/libraries/EspSimpleWifiHandler/src/EspSimpleWifiHandler.cpp: In constructor 'EspSimpleWifiHandler::EspSimpleWifiHandler(const char*, const char*, const char*, bool)':
/Users/XXX/Documents/XXX/Arduino/libraries/EspSimpleWifiHandler/src/EspSimpleWifiHandler.cpp:56:5: error: no matching function for call to 'WiFiClass::onEvent(EspSimpleWifiHandler::EspSimpleWifiHandler(const char*, const char*, const char*, bool)::<lambda(arduino_event_id_t, system_event_info_t)>, arduino_event_id_t)'
     );
     ^
In file included from /Users/XXX/Library/Arduino15/packages/esp32/hardware/esp32/2.0.1/libraries/WiFi/src/WiFiSTA.h:28,
                 from /Users/XXX/Library/Arduino15/packages/esp32/hardware/esp32/2.0.1/libraries/WiFi/src/WiFi.h:32,
                 from /Users/XXX/Documents/XXX/Arduino/libraries/EspSimpleWifiHandler/src/EspSimpleWifiHandler.h:14,
                 from /Users/XXX/Documents/XXX/Arduino/libraries/EspSimpleWifiHandler/src/EspSimpleWifiHandler.cpp:1:
/Users/XXX/Library/Arduino15/packages/esp32/hardware/esp32/2.0.1/libraries/WiFi/src/WiFiGeneric.h:147:21: note: candidate: 'wifi_event_id_t WiFiGenericClass::onEvent(WiFiEventCb, arduino_event_id_t)'
     wifi_event_id_t onEvent(WiFiEventCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
                     ^~~~~~~

And so on...

Maybe this is a breaking change from 2.0.1 ESP32 library?

I will keep investigating tomorrow and see if I can make any sense of it or find an easier solution.

Thanks, -pablo

plapointe6 commented 2 years ago

Thank you, these logs are valuable.

It works well on my side (with both ESP32 and ESP8266), but I am not sure wich ESP32 core version I had when I tested this. I will do more testing in the next days.

zzpaf commented 2 years ago

Sorry I was not able to get much further, I am thinking of a different approach for you to check if this would work.

Maybe you can use the: ARDUINO_EVENT_ETH_GOT_IP and the ARDUINO_EVENT_WIFI_STA_GOT_IP and set 2 variables _wificonnected = true and _ethconnected

And unset them on: ARDUINO_EVENT_ETH_DISCONNECTED ARDUINO_EVENT_WIFI_STA_LOST_IP

And check for connectivity using if (_wificonnected || _ethconnected)

This way you will have a quite accurate and reliable way of knowing if there is a connection you can use?

And replace code:

 // Get the current connextion status
  // bool isWifiConnected = (WiFi.status() == WL_CONNECTED); ############# Modified
  bool isConnected = _isWificonnected || _isEthconnected

  /***** Detect ans handle the current WiFi handling state *****/

  // Connection established
  //if (isWifiConnected && !_wifiConnected) ############# Modified
  if (isConnected && !_wifiConnected)
  {
    onWiFiConnectionEstablished();
    _connectingToWifi = false;

    // At least 500 miliseconds of waiting before an mqtt connection attempt.
    // Some people have reported instabilities when trying to connect to 
    // the mqtt broker right after being connected to wifi.
    // This delay prevent these instabilities.
    _nextMqttConnectionAttemptMillis = millis() + 500;
  }

I will do some additional testing and see if this method would work.

Thanks