Improve the canonical "node-wifi-mqtt" firmware


#41

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

See you,
Giuseppe


#42

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

Cheers,
Andreas.


#43

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


Pflege des Firmware Repository
#44

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.


#45

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!


#48

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.


#49

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 am 17./18. November 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


#50

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?


#51

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()

?


#55

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

#56

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));

#57

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


#58

No, this is not the problem.