Memory problem? dtostr() and strcat()

Question

I've been dealing with this for quite a while now and searching all over for answers.. I'm trying to put together two floats into a char*. Separated by a ",". This is the code I'm working with.

#include <DHT.h>
#include <RH_ASK.h>
#include <SPI.h>

#define DHT_pin 10
#define DHT_type DHT11

DHT dht(DHT_pin, DHT_type);
RH_ASK rf_driver;

float temp;
float hum;

char ch_temp[6];
char ch_hum[6];
char msg_out[12] = "";

void setup() {
  Serial.begin (115200);
  dht.begin();
  if (rf_driver.init()) {
    Serial.println("Radio transmitter initiated..");
  }
  else {
    Serial.println("Radio transmitter failed to initiate..");
    delay(1000);
  }
}
void loop() {
  temp = dht.readTemperature();
  hum = dht.readHumidity();

  dtostrf(temp, 5, 2, ch_temp);
  dtostrf(hum, 5, 2, ch_hum);
  strcat(msg_out, ch_temp);
  strcat(msg_out, ",");
  strcat(msg_out, ch_hum);

  Serial.println(msg_out);
  rf_driver.send((uint8_t *)msg_out, strlen(msg_out));
  rf_driver.waitPacketSent();
  Serial.print("data sent, package size: ");
  Serial.println(strlen(msg_out));
  delay(1000);

}

I have tried using different sizes of the char arrays, but it just goes crazy. This is the console output:

Radio transmitter initiated..
22.00,34.00
data sent, package size: 11
⸮⸮Radio transmitter initiated..
22.00,34.00
data sent, package size: 11

So on its way through the second loop it fails, restarts and then on its third way through it just gives up... What am I doing wrong? :( Really sad over this.

Answer 1

If you remove the RF and sensor code, the sketch seems to work correctly when printing to the serial monitor. The first time you call rf_driver.send() in the loop, it works. The second time through the loop, and every time there after, it starts printing corrupted data. I don't have a definitive answer why this is happening, but I do have a way to "fix" it (or a "hack", if you prefer to call it that).

Using memset() to set the msg_out char array to all 0's before you use it seems to fix the problem. Here is a test sketch which is very similar to yours. I'm using some hard coded values for the sensor data because I don't have one of those sensors.

#include <RH_ASK.h>
#include <SPI.h>
RH_ASK rf_driver;

float temp;
float hum;

char ch_temp[6];
char ch_hum[6];
char msg_out[12] = "";

void setup() {
  Serial.begin (9600);
  if (rf_driver.init()) {
    Serial.println("Radio transmitter initiated..");
  }
  else {
    Serial.println("Radio transmitter failed to initiate..");
    delay(1000);
  }
}
void loop() {

  // Set the buffer contents to all 0's
  memset(msg_out, 0, sizeof(msg_out));

  temp = 12.345;
  hum = 67.890;

  dtostrf(temp, 5, 2, ch_temp);
  dtostrf(hum, 5, 2, ch_hum);
  strcat(msg_out, ch_temp);
  strcat(msg_out, ",");
  strcat(msg_out, ch_hum);

  Serial.println(msg_out);
  rf_driver.send((uint8_t *)msg_out, strlen(msg_out));
  rf_driver.waitPacketSent();
  Serial.print("data sent, package size: ");
  Serial.println(strlen(msg_out));
  delay(1000);
}

Answer 2

Your sketch is running on borrowed memory.
Every time the loop function runs, the new text is concatenated with strcat() and the buffer msg_out is never reset. The first time 12 bytes are used, the second time 23 bytes, the third time 34, and so on.

The solution in the answer by @VE7JRO will fix that, but I prefer not to use dtostrf() and transmit binary data.

I see a few other problems as well:

• Why do you want to transmit readable ASCII text? Use binary data. Either float numbers or integers. For example an integer in 1/100 degrees.
• The RadioHead library uses a lot of memory. You can not use an other library that uses also a lot of memory.
• The value of '5' for dtostrf is the minimal width, so you better make those arrays for example 20 bytes. I prefer to put those arrays temporarily on the stack.
• The DHT11 is not accurate. If you upgrade to a better sensor with I2C, then you need more libraries (the Wire library and a sensor library), and you might run out of sram memory.
• When using the RadioHead library in default mode, it uses three pins (rxPin=11, txPin=12, pttPin=10). You better add a comment section to your sketch to explain that you should not use those pins. You have already a conflict at pin 10 for the DHT sensor and the pttPin.

---

The old and no longer supported VirtualWire library has some bugs, but it uses less memory than the RadioHead library. The RadioHead library is not optimized for Arduino boards like the Uno, Nano, Leonardo, and others. It even requires to #include <SPI.h> because that is often used with transceivers, even if the SPI bus is not used in RH_ASK mode (thanks to @VE7JRO for explaining this in the comment below).

---

The easiest way to transmit more than one value is with an array.

float myData[2];
myData[0] = 23.456;
myData[1] = 68.0;
rf_driver.send((uint8_t *)myData, sizeof(myData));

The sizeof() is not a real function, it tells the compiler to fill in the size of that variable. Since a float is 4 bytes, you may also fill in the number 8 for the second parameter.

There are a number of solutions for the receiver. I like to use a pointer to the buffer, but perhaps it is easier to copy them to variables with memcpy().

float myData[2];
if (driver.recv(buf, &buflen)) {
  if (buflen == sizeof(myData) {  // extra check
    memcpy((uint_8 *)myData, buf, sizeof(myData));
    Serial.println(myData[0]);
    Serial.println(myData[1]);
  }
}

When the receiver is also an Arduino board, then there should be no problem. When the receiver is something else, you might have to check if the byte-order is the same. The float variable of a Arduino is 100% IEEE compatible, but sometimes it is easier to use only integers.

The most common way is to use a struct. A struct is a package that can hold all sorts of data (integers, bytes, arrays, and so on). The receiver and the sender should have exactly the same definition for the struct of course.

Answer 3

I'm late to the party, but here goes:

msg_out[] is a global but it doesn't need to be. Move it's definition and initialization into loop(), the only function that uses it. Then it gets initialized at every entry to loop() rather than just once at program initialization. There's no need to clear the array; just zeroing the zero-th element of it initializes it for character-string use, and that is what your initializer now does (but only once).