knolleary / pubsubclient

A client library for the Arduino Ethernet Shield that provides support for MQTT.
http://pubsubclient.knolleary.net/
MIT License
3.82k stars 1.47k forks source link

client.publish() crash on ESP32 related to string length #940

Closed fflori4n closed 2 years ago

fflori4n commented 2 years ago

Hello,

When sending a JSON string larger than 128 chars, client.publish() causes heep overflow on the ESP32. I have changed the buffer size with client.setBufferSize(512); but maybe it's not working or I'm not using it correctly.

here's the min reproducable code:

#include <WiFi.h>
#include <PubSubClient.h>
#include <LinkedList.h>
const char* ssid = "*****";
const char* password = "******";
class MqttPublisher{
  public:
  /// using <PubSubClient.h>
    #define _OWNER "fflori4n"
    #define _CLUSTER "ESP0"
    #define path "******"
    #define retryNo 5
    //const char* mqttToppic = (owner path cluster);

    const char* mqttToppic = "*****"; 
    const char* mqttUser = "*****";
    const char* mqttPassword = "******";
    const char* mqttServer = "192.168.1.105";
    const int mqttPort = 1883;
    String mqttMsg = "";
    PubSubClient client;

    MqttPublisher(PubSubClient client){
        this->client = client;
        client.setBufferSize(512);      /// buffersize is set here.
    }
    void init(){
      client.setServer(mqttServer, mqttPort);
      }
    int connect(){
      if(client.connected()){ /// TODO: check if network is available
        return 0;
      }
      Serial.print("Connecting to MQTT...\t\t");
      for(int i=0; i < retryNo; i++){
        if (client.connect("ESP32Client", mqttUser, mqttPassword )) {
          delay(1000);
          Serial.println("[ OK ]");
          return 0;
        }
        else{
          delay(15000);
        } 
      }
      Serial.print("[ ER ] Can't connect to MQTT server: ");
      Serial.print(client.state());
      return -1;
    }
    int publishMqtt(){

      if(!client.connected()){
        if(this->connect() == -1){
          return -1;
        }
      }
      Serial.print("publishing... \t\t\t");

      //client.setBufferSize(512);   /// issue No 2 : if this is uncommented, ESP will also crash. But might be unrelated or I'm not using setBufferSize() corretly.

      String testMsg = "{ \"Temp0_ESP0_fflori4n\": 22.0, \"Temp1_ESP0_fflori4n\": 22.0, \"Humidity0_ESP0_fflori4n\": 35.0, \"Dust0_ESP0_fflori4n\": 0.6, \"Motion0_ESP0_fflori4n\": 1.0 }"; /// bad message, probably to long? - over default 128 chars
      //String testMsg = "{\"Temp0_ESP0_fflori4n\": 22.0, \"Temp1_ESP0_fflori4n\": 22.0, \"Humidity0_ESP0_fflori4n\": 35.0, \"Dust0_ESP0_fflori4n\": 0.6}"; /// good message, less than 128 chars. Sent without problem.

      Serial.println(testMsg.length());

      if (!client.publish(mqttToppic,  testMsg.c_str())) { ///crash, heep overflow somehow...
        Serial.println("[ ER ] publish failed");
        client.loop();
        return -1;
      }

      Serial.println("[ OK ]");
      client.loop();
      return 0;
    }
};
WiFiClient espClient;
PubSubClient client(espClient);
MqttPublisher publisher(client);

void setup() {
  Serial.begin(115200);
  Serial.println(F("Serial OK"));

  WiFi.begin(ssid, password);
  Serial.print("Connecting to WiFi..");
  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }
  Serial.println("Connected to the WiFi network");
  publisher.init();
}
int tick = 2000;
void loop() {
 if(tick > 15000/50){
  tick = 0;
  publisher.publishMqtt();
 }
 delay(50);
 tick++;
}

and the serial output:

rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:1
load:0x3fff0018,len:4
load:0x3fff001c,len:1216
ho 0 tail 12 room 4
load:0x40078000,len:10944
load:0x40080400,len:6388
entry 0x400806b4
Serial OK
Connecting to WiFi.......Connected to the WiFi network
Connecting to MQTT...       [ OK ]
publishing...           151
Guru Meditation Error: Core  0 panic'ed (LoadProhibited). Exception was unhandled.
Core 0 register dump:
PC      : 0x4008c0bf  PS      : 0x00060e33  A0      : 0x8008b471  A1      : 0x3ffb55e0  
A2      : 0x0000811c  A3      : 0x3ffb5828  A4      : 0x00000001  A5      : 0x00000001  
A6      : 0x00060c23  A7      : 0x00000000  A8      : 0x00000000  A9      : 0x3ffb5720  
A10     : 0x00000008  A11     : 0x00000000  A12     : 0x00001388  A13     : 0xffffffff  
A14     : 0x3ffc3ec0  A15     : 0x00000008  SAR     : 0x0000001d  EXCCAUSE: 0x0000001c  
EXCVADDR: 0x0000812c  LBEG    : 0x4000c2e0  LEND    : 0x4000c2f6  LCOUNT  : 0xffffffff  

ELF file SHA256: 0000000000000000

Backtrace: 0x4008c0bf:0x3ffb55e0 0x4008b46e:0x3ffb5600 0x40089545:0x3ffb5620 0x4012e7dd:0x3ffb5660 0x4008593e:0x3ffb5680 0x40085cb0:0x3ffb56c0 0x4013178a:0x3ffb5700 0x4012e6cd:0x3ffb5720 0x401018dd:0x3ffb5740 0x40101f3a:0x3ffb5760 0x40089792:0x3ffb5790

Rebooting...
ets Jun  8 2016 00:22:57
IronBamBam1990 commented 2 years ago

you cant do delay(). its better to do with task manager. You must also create a FS and file like config.json .With buffer you can do something like this below (i attached two example scripts you should figured out ) and than should be all right:

I think this is what you need

char buffer[250];

sprintf(buffer,
"{\"host\": \"%s\", " "\"short_message\": \"%s\", " "\"Lattitude\": \"%f\", " "\"Longitude\": \"%f\", " "\"Message Title\": \"%s\", " "\"Message\": \"%s\" " "}", mqttMessage.host, mqttMessage.shortMessage.c_str(), mqttMessage.lattitude, mqttMessage.longitude, mqttMessage.messageTitle.c_str(), mqttMessage.message.c_str() );

bool PublishToTopic(mqttMessageType &mqttMessage, const char* topic, MQTT::QoS qos)
{
    MQTT::Message message;
    message.qos = qos;
    message.retained = false;
    message.dup = false;

    mqttMessage.host = "XYZ";
    mqttMessage.shortMessage = "This is a short_message";
    char buffer[250];
    sprintf(buffer,                                
            "{\"host\": \"%s\", "
            "\"short_message\": \"%s\", "
            "\"Lattitude\": \"%f\", "
            "\"Longitude\": \"%f\", "
            "\"Message Title\": \"%s\", "
            "\"Message\": \"%s\" "
            "}",
            mqttMessage.host, mqttMessage.shortMessage.c_str(),
            mqttMessage.lattitude, mqttMessage.longitude,
            mqttMessage.messageTitle.c_str(), mqttMessage.message.c_str()
            );

    cout << buffer;
    message.payload = (void*)buffer;
    message.payloadlen = strlen(buffer) + 1;

    rc = client.publish(topic, message);
    if (rc != 0) {
        printf("Error %d from sending QoS 0 message\n", rc);
    }
    else
    {
        cout << "Message published successfully to " << topic << endl;
    }

    return true;
}

OR


void loop() {

  StaticJsonBuffer<300> JSONbuffer; /// <<===== Here i preffer to use DynamicJsonDocument json(ESP.getMaxFreeBlockSize() - 4096);  or  DynamicJsonDocument json(1024);

  JsonObject& JSONencoder = JSONbuffer.createObject();  ///// YOU CAN USE DYNAMICJSON is better

  JSONencoder["device"] = "ESP32";
  JSONencoder["sensorType"] = "Temperature";
  JsonArray& values = JSONencoder.createNestedArray("values");

  values.add(20);
  values.add(21);
  values.add(23);

  char JSONmessageBuffer[100];  /// Exchange for dynamic buffer
  JSONencoder.printTo(JSONmessageBuffer, sizeof(JSONmessageBuffer));  
  Serial.println("Sending message to MQTT topic..");
  Serial.println(JSONmessageBuffer);

  if (client.publish("esp/test", JSONmessageBuffer) == true) {
    Serial.println("Success sending message");
  } else {
    Serial.println("Error sending message");
  }

  client.loop();
  Serial.println("-------------");

}
IronBamBam1990 commented 2 years ago

ok thats should help you

void Serialize(){ Serial.println("[WSc] SENT: Broadcast message!!"); const size_t capacity = 1024; DynamicJsonDocument doc(capacity);

doc["temp"] = temp; doc["humi"] = humi; doc["humi1"] = humi1; doc["humi2"] = humi2;
doc["humi3"] = humi3; doc["humi4"] = humi4; String buf; serializeJson(doc, buf);

Serial.println("Json " + buf ); }

fflori4n commented 2 years ago

The problem was with the publishing function, managed to fix it with a workaround splitting up the string, suggested in the large msg example.

client.publish() was replaced with client.beginPublish(), client.print(), and client.endPublish(). As in: https://github.com/knolleary/pubsubclient/blob/master/examples/mqtt_large_message/mqtt_large_message.ino

Thanks for the suggestions, especially the JSON buffer stuff is much appreciated and will be part of the code in the future.