Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

publish() needs an idle period to succeed? #435

Open
ohjohnsen opened this issue Apr 22, 2018 · 6 comments
Open

publish() needs an idle period to succeed? #435

ohjohnsen opened this issue Apr 22, 2018 · 6 comments

Comments

@ohjohnsen
Copy link

On my ESP8266, I've made an application both for publishing temperature/humidity, and for publishing power usage. Norwegian fusebox power monitors blink 1000/2000 times per 1kWh consumed, and by measuring time between each blink, I calculate the current power usage. The blink is measured using a GA1A1S202WP light sensor on pin A0. The measured temperature and humidity and calculated power usage is sent to a Mosquitto MQTT broker on a fixed interval using the SimpleTimer library.

However, I see that if I don't have a delay([some hundreds of milliseconds]); statement right after the publish() call, the messages are not received successfully by Mosquitto, even though publish() returns "1".

Here's my main loop:

void loop() {
	if (!client.connected()) {
		Serial.print("WiFi state: ");
		Serial.println(WiFi.status());
		Serial.print("MQTT connection lost: ");
		Serial.println(client.state());
		reconnect();
	}
	client.loop();
	timer.run();
	raw_lux_sum_value += analogRead(A0);
        // Code for measuring averaged light value, determining fuse box power monitor LED state, and calculating power usage.
}

And here's the publish function being called in the timer callback:

void publishData(float p_temperature, float p_humidity, unsigned int p_power_consumption) {
	// create a JSON object
	// doc : https://github.com/bblanchon/ArduinoJson/wiki/API%20Reference
	StaticJsonBuffer<200> jsonBuffer;
	JsonObject& root = jsonBuffer.createObject();
	// INFO: the data must be converted into a string; a problem occurs when using floats...
	root["temperature"] = (String)p_temperature;
	root["humidity"] = (String)p_humidity;
	root["powerconsumption"] = (String)p_power_consumption;
	root.prettyPrintTo(Serial);
	Serial.println("");
	/*
	{
	"temperature": "23.20",
	"humidity": "43.70",
	"powerconsumption": "500"
	}
	*/
	char data[200];
	root.printTo(data, root.measureLength() + 1);
	int success = client.publish(MQTT_SENSOR_TOPIC, data);
	Serial.print("MQTT publish result: ");
	Serial.println(success);
	delay(500);
}

If I remove the delay(500); at the end of the publishData() function, the messages are not received successfully 99.9% of the time by Mosquitto.

@giddyhup
Copy link

Can confirm. I ran into the same issue.

@ebivan
Copy link

ebivan commented Aug 10, 2018

I can confrim that too. But in my case 100ms are enough, maybe even less. I will tray to go further down as mine is running on batteries and goes to deepsleep after publishing the sensor data.

@jonasvdd
Copy link

jonasvdd commented Jan 2, 2019

I can confirm this too.

Within my code I just Establish a MQTT connection and publish multiple consecutive messages to various topics. At first i just established a connection to the topic prefix and published them without delay in a loop. I saw that only the first published one arrived at my broker.

As stated above, I introduced a delay of 100ms. By doing so, ~ 90% of the packages arrived correctly. But occasionally, the last packages wouldn't be sent. The succeeding method makes sure all packages will be sent.

The succeeding snippet makes sure that all packages will be sent, without introducing a hard coded delay. This is achieved by dis- and reconnecting each iteration to the broker. It took for sending 10 messages of 31 bytes ~ 4-5 seconds. Which is quite much. But all packages will arrive. So if you want to go low power, I would propose to take the delay instead of this method.

while (processed_size < total_recv_size) {
    ...
    // try to connect to the topic
    while (!mqtt.connect(mqtthelper.topic_str)) {
        SerialMon.println(" === MQTT NOT CONNECTED === ");
        delay(100);
    }
    mqtt.publish(mqtthelper.topic_str, payload, payload_size + TIMESTAMP_BYTES);
    mqtt.disconnect();     // disconnect explicitly
    ...
}

(sorry for bad English, not a native speaker)

@ghmartin77
Copy link

Seeing the same here, especially if going to deep sleep without a delay. This is probably due to the fact that the TCP stack implementation lwIP decides on its own, when it's time to send out data in its buffers. Fixed the problem for me by introducing some code into WiFiClient.cpp/.h and ClientContext.h

WiFiClient.h (in the "public:" section):

void waitUntilAllDataSent();  

WiFiClient.cpp:

void WiFiClient::waitUntilAllDataSent() {
	return _client->waitUntilAllDataSent();
}

ClientContext.h ("public:" section):

inline void waitUntilAllDataSent() {
    while ((_datasource && _datasource->available()) || tcp_sndbuf(_pcb) < _sndbufsize) {
    		yield();
    	}
    }

ClientContext.h ("private:" section):

size_t _sndbufsize;

ClientContext.h (last line of c'tor):

_sndbufsize = tcp_sndbuf(pcb);

In my code I do the following after publish():

		mqttWiFiClient.flush();
		mqtt.loop();
		mqttWiFiClient.waitUntilAllDataSent();

@jonasvdd
Copy link

Since I work with the TinyGSM library, which uses pubsubclient for MQTT messages, I just had to add the client.flush() line after publishing the message.

This is a cleaner way to solve this than my initial proposal. . It also solves the delay overhead from dis and reconnecting each time. My setup ran with this code for 2 weeks. None of the packages were lost (I published to 2-3 different topics each hour).

Thanks for your input @ghmartin77 😄

while (processed_size < total_recv_size) {
    ...
    // try to connect to the topic, just left it in to be sure
    while (!mqtt.connect(mqtthelper.topic_str));
    mqtt.publish(mqtthelper.topic_str, payload, payload_size + TIMESTAMP_BYTES);
    // Flushes TCP buffers, client is an instance of TinyGSMClient class
    client.flush(); 
    ...

@ghmartin77
Copy link

ghmartin77 commented Jan 21, 2019

Additional note: ESP8266 core for Arduino introduced WiFiClient.setSync() with this commit: esp8266/Arduino#5089. It's available in 2.5.0 (currently in beta) and allows for synchronous sending without tampering around with the code.

And one more thing: Just saw this one made it into 2.4.0: esp8266/Arduino#3967
Explains why @jonasvdd 's approach works. I was poking around with 2.3.x still where I saw this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants