Improve the canonical "node-wifi-mqtt" firmware

Dear Andreas,
Of course I will. Tomorrow morning I will perform the tests.

Thanks!

See you,
Giuseppe

Dear Andreas,
I did not perform a very comprehensive test but at least for my setup the firmware works. Readability of the serial output would improve by removing the line breaks after “NOTIFICATION:”. The IP address is shown incorrectly as you can see in the output below. Changing line 915 to Serial.println(WiFi.localIP()); prints the IP properly (but bypasses the SerialDebugger?).

194 - NOTIFICATION:
                                wifi_connect | Connecting to WiFi 
316 - NOTIFICATION:
                                wifi_connect | Retrying WiFi connection in a few seconds:  0.40

716 - NOTIFICATION:
                                wifi_connect | Retrying WiFi connection in a few seconds:  0.40

1116 - NOTIFICATION:
                                wifi_connect | Retrying WiFi connection in a few seconds:  0.40

1517 - NOTIFICATION:
                                wifi_connect | Retrying WiFi connection in a few seconds:  0.40

1917 - NOTIFICATION:
                                wifi_connect | Retrying WiFi connection in a few seconds:  0.40

2317 - NOTIFICATION:
                                wifi_connect | Retrying WiFi connection in a few seconds:  0.40

7259 - NOTIFICATION:
                                wifi_connect | WiFi connected! IP address:  381462720

7761 - NOTIFICATION:
                                mqtt_connect | Connecting to MQTT broker 
7870 - NOTIFICATION:
                                mqtt_connect | Successfully connected to MQTT broker 
7870 - NOTIFICATION:
                                read_weight | Read weight (HX711) 
8206 - NOTIFICATION:
                                read_weight | Read weight (HX711) Weight: -12.18

8206 - NOTIFICATION:
                                read_temperature_array | Read temperature array (DS18B20) 
9253 - NOTIFICATION:
                                read_humidity_temperature | Read humidity and temperature (DHTxx) 
9258 - NOTIFICATION:
                                read_battery_level | Battery ADC:  621

9284 - NOTIFICATION:
                                read_battery_level | Battery level:  38%
{"weight":-12.18,"airtemperature":23.90,"airhumidity":43.60,"broodtemperature":23.44,"battery_level":38}
9462 - NOTIFICATION:
                                transmit_readings | MQTT publish succeeded

Dear Matthias,

thanks for testing! We just pushed some fixes to the branch “node-wifi-mqtt-silent”, which might mitigate the issues you brought to the table. Hopefully they work actually. Would you check again?

Thanks in advance!

Cheers,
Andreas.

SerialDebugger.debug() always starts with a new line. So no need to use Serial.println() after or before SerialDebugger.debug() . Serial.print() should fit better

With:

Serial.print("Weight: " + String(weight));
SerialDebugger.print(adc_level);
SerialDebugger.print(WiFi.localIP().toString().c_str());
SerialDebugger.print(WIFI_RETRY_DELAY);

output then changes to

194 - INFO:     wifi_connect | Connecting to WiFi 
348 - INFO:     wifi_connect | Retrying WiFi connection in a few seconds:  0.40
748 - INFO:     wifi_connect | Retrying WiFi connection in a few seconds:  0.40
1148 - INFO:    wifi_connect | Retrying WiFi connection in a few seconds:  0.40
1548 - INFO:    wifi_connect | Retrying WiFi connection in a few seconds:  0.40
1949 - INFO:    wifi_connect | Retrying WiFi connection in a few seconds:  0.40
2349 - INFO:    wifi_connect | Retrying WiFi connection in a few seconds:  0.40
7101 - INFO:    wifi_connect | WiFi connected! IP address:  192.168.188.22
7604 - INFO:    mqtt_connect | Connecting to MQTT broker 
7632 - INFO:    mqtt_connect | Successfully connected to MQTT broker 
7632 - INFO:    read_weight | Read weight (HX711) 
7966 - INFO:    read_weight | Read weight (HX711) Weight: -10.72
7966 - INFO:    read_temperature_array | Read temperature array (DS18B20) 
8990 - INFO:    read_humidity_temperature | Read humidity and temperature (DHTxx) 
8995 - INFO:    read_battery_level | Battery ADC:  618
8998 - INFO:    read_battery_level | Battery level:  37%
{"weight":-10.72,"airtemperature":21.40,"airhumidity":44.00,"entrytemperature":20.69,"battery_level":37}
9166 - INFO:    transmit_readings | MQTT publish succeeded

Many thanks @Thias!
I just tested the firmware; it seems to work flawlessly. However, I still have one remark: does it make any sense to perform a fixed number of Wi-Fi connection trials (e.g. 15), then proceeding? Wouldn’t be better to indefinitely try to estabilish the connection, maybe with a more reasonable delay (in order not to rapidly discharge the battery; if sensor_battery is enabled, we can also avoid to fully discharge the battery by breaking the loop and printing a error string if battery level is lower than a fixed threshold) ? This is also useful as nodes does not need to be rebooted if they are powered on before activating the Wi-Fi access.

Cheers,
Giuseppe

Thanks, @Thias. We just pushed an update reflecting your recommendations.

Dear Giuseppe,

thanks for your suggestions!

We just pushed some updates regarding your recommendations to the branch “node-wifi-mqtt-connectivity” (see node-wifi-mqtt.ino). We didn’t add the complexity to monitor the battery level but rather added simple enhancements to not proceed with measurements. This prevents the battery from draining too much by sending the node into deep sleep mode when connectivity couldn’t be established.

The new connectivity flow should account for that, as “wifi_connect()” is now called inside “measure()”.

Are you fine with that? We are looking forward for tests on the iron by one of you.

Cheers,
Andreas.

Hello Andreas,
I’m absolutely fine with that. I just tested the firmware and it works!

See you,
Giuseppe

Thanks, @gtuveri! We just merged the changes into the master branch. Have fun!

Cheers,
Andreas.

1 Like

I have tried to run node-wifi-mqtt with a up to date Arduino IDE (2018-11). Some comments and improvements:

@Andreas you link in the sketch to the SerialDebug lib in the Arduino playground section:

// http://playground.arduino.cc/Code/SerialDebugger
// https://github.com/hiveeyes/arduino/tree/node-wifi-mqtt-silent/libraries/SerialDebugger
#include <SerialDebug.h>

I first tried the lib on arduino.cc but it is outdated and not runnig with the current Arduino IDE. The forked hiveeyes lib is working, so please remove the reference to the origial location or add a warning to use only the documentation but not the lib itself on this location!

The current and by default installed ArduinoJson (version 6.5.0-beta) lib is not working with our sketch! Details see 🔥 JsonBuffer was not declared in this scope / does not name a type · Issue #756 · bblanchon/ArduinoJson · GitHub

However, version 6 replaces JsonBuffer with JsonDocument .

So you have to install a 5.x version

1 Like

Dear Clemens,

thanks a bunch for sharing your findings! We will try to address them soon by improving the documentation and maybe considering an upgrade to more recent libraries.

SerialDebugger

Silly me referencing the original library there. Yes, we forked it and because we usually work on the command line pulling the whole Git repository, this isn’t usually an issue to us. By the way, you should do the same as it will always deliver a coherent state, at least we try to keep it that way.

ArduinoJson 6

According to the comments on this issue on GitHub, ArduinoJson 6 might still be in beta? As far as we can tell, it is common understanding that the Arduino IDE misbehaves by offering the upgrade without honoring its beta state. As the firmware repository also nails the ArduinoJson version it was built for, this issue will not come up when just cloning the complete firmware repository from GitHub, we really recommend it! ;]

Outlook

Saying that, there’s definitively room for great improvement on the documentation side of things in order to minimize misunderstandings and mistakes. Further, we will also have a look into upgrading to ArduinoJson 6 if it’s already robust enough. Maybe the outcome of your work will already improve this? According to this comment, it is possible to support both versions of ArduinoJson in the same codebase, which we would take care of when adding the support for ArduinoJson 6. Do you agree?

Edit:

Ah, you also downgraded to ArduinoJson 5. Good decision, thanks for listening ;].

Cheers,
Andreas.

I know, I know, but our code has to run in the “normal” Arduino IDE also and without testing this a new user would run in difficults. So it’s good to test, I don’t like to miss this step!

The sketch node-wifi-mqtt.ino is only prepared for 2 DS18B20 sensors as maximum.

I have tried to expand it with this code but while running the sketch is stopping

#if SENSOR_DS18B20
//    json_data["broodtemperature"]            = ds18b20_temperature[0];
for (int i = 0; i < ds18b20_device_count; i++) {
//      String json_label = "broodtemp-";
//      json_label = json_label + i; 
//      json_data[json_label] = ds18b20_temperature[i];

  if (i == 0) {
    json_data["broodtemp-1"] = ds18b20_temperature[0];
  }
  if (i == 1) {
    json_data["broodtemp-2"] = ds18b20_temperature[1];
  }
  if (i == 2) {
    json_data["broodtemp-3"] = ds18b20_temperature[2];
  }
  if (i == 3) {
    json_data["broodtemp-4"] = ds18b20_temperature[3];
  }
  if (i == 4) {
    json_data["broodtemp-5"] = ds18b20_temperature[4];
  }

}
#endif

I have tried both variants because I’m not sure if a string is possible in the json_data object. But both variants lead to the same stopping sketch.

That’s correct, nobody wanted to drive a Open Hive Temperature Array with it, yet. I already was expecting you would be the first one to come up with this ;], welcome!

I will try to sublimate myself into this a bit but will have to take care about other obligations first. Would you mind to do this after Bits & Bäume Konferenz 2018 an der TU Berlin or do you have any plans to show this very feature at the conference already? If so, let’s see what we can do about it - maybe this evening? :P

1 Like

Hi Clemens,

regarding

Let’s find where memory corruption happens then!

Q&A

Two things come to my mind.

a) Possible, most obvious source of memory corruption when misconfigured

Did you properly configure

// For more DS18B20 devices, configure them like...
//const int ds18b20_device_order[ds18b20_device_count] = {0,1};

for multiple devices?

Otherwise, the sensor reading code


    // Iterate all DS18B20 devices and read temperature values
    for (int i=0; i < ds18b20_device_count; i++) {

        // Read single device
        float temperatureC = ds18b20_sensor.getTempC(ds18b20_addresses[i]);
        ds18b20_temperature[i] = temperatureC;

        // Give operating system / watchdog timer some breath
        yield();

}

might crash due to reading data from an invalid device address coming from uninitialized memory through

ds18b20_addresses[i]

Don’t know how

ds18b20_sensor.getTempC(...)

would handle that kind of guy.

b) Double check

Does your code work with a single device, i.e. by setting "ds18b20_device_count = 1" again?

c) Buffer size too low

Would you try again after increasing the buffer size at "StaticJsonBuffer<512> jsonBuffer;" please?

Es liegt immer am Kabel.™

-) Add more debugging statements

You might want to insert more print-style debug statements at sensor reading time as well as payload serialization time. E.g. when knowing about the value of the

int json_length = json_data.measureLength()

assignment, increasing buffer sizes c) might have become obvious already.

Cheers,
Andreas.

P.S.: off-by-one error again, apologies.


P.P.S.:

TLDR; This probably isn’t your problem, but for the records…

You might also want to double check with

json_data.measureLength()

down a few lines. If that would yield a wrong value (why should it? ;]),

char payload[json_length+1]

might reserve a char buffer too short to hold the final payload before putting it on the wire. However, this probably does not contribute to an eventual memory corruption, as

json_data.printTo(payload, sizeof(payload))

should be safe by limiting the copy operation to

sizeof(payload)

itself.

– Let’s get on the phone, finally?

Maybe read_temperature_array() also needs some tuning?

Asynchronous vs. synchronous reading

Using the asynchronous mode by

// Make it asynchronous
ds18b20_sensor.setWaitForConversion(false)

might sound cool, but getting it synchronous again by

// Wait at least 750 ms for conversion
delay(1000)

does not make sense to me and feels a bit flaky. Maybe let’s go with a synchronous

// Initiate temperature retrieval
ds18b20_sensor.requestTemperatures()

?

The initial configuration should be ok:

const int ds18b20_device_count = 5;
[...]
const int ds18b20_device_order[ds18b20_device_count] = {0,1,2,3,4};

Yes it is working with a single device and debug puts also right valutes

5822 - INFO:	read_temperature_array | Read temperature array (DS18B20) 
6888 - INFO:	read_humidity_temperature | Read humidity and temperature (DHTxx) 
6894 - INFO:	read_battery_level | Battery ADC:  914
6894 - INFO:	read_battery_level | Battery level:  169%{"weight":-3.575555,"airtemperature":22.2,"airhumidity":45.3,"broodtemp-1":21.4375,"broodtemp-2":21.875,"broodtemp-3":21.8125,"broodtemp-4":22.125,"broodtemp-5":21.8125,"battery_level":169}
Exception (9):
epc1=0x40207bb3 epc2=0x00000000 epc3=0x00000000 excvaddr=0x74616222 depc=0x00000000

ctx: cont 
sp: 3ffffa30 end: 3fffffd0 offset: 01a0

>>>stack>>>
3ffffbd0:  00000000 00000000 0000001f 3ffffd60

I have increased StaticJsonBuffer from 512 to 1024 – still the same problem.

But after that trial I have changed

json_data["broodtemperature-1"] = ds18b20_temperature[0];
[...]
json_data["broodtemperature-5"] = ds18b20_temperature[4];

to

json_data["bt-1"] = ds18b20_temperature[0];
[...]
json_data["bt-5"] = ds18b20_temperature[4];

Now it is working. I fear the problem is the printTo function, but just a guess.

json_data.printTo(payload, sizeof(payload));
1 Like

Good to hear you got it working. Maybe increase the size reserved to the JSON data structure to 2048 or even more?

No, this is not the problem.