Need help understanding Buffer size

callen · January 5, 2019, 6:05am

I am not much of a programming buff outside of web languages. So I need to understand what I think might be a buffer. Consider the following code:

#include <Seeed_DHT11.h>

#define DHTPIN D2

DHT dht(DHTPIN);

void setup() {
    dht.begin();
}

void loop() {
    
    delay(2000);
    
    float h = dht.getHumidity();
    float f = dht.getTempFarenheit();
    float voltage = analogRead(BATT) * 0.0011224;
    
    String device = "XPX-Xenon-01";
    
    char data[128];
    // sprintf(data, "{\"Temperature\":\%.2f\,\"Humidity\":\%.2f\,\"Battery Voltage\":\%.2f\,\"Device\":\%s\}", f, h, voltage, device);
    sprintf(data, "{\"Temperature\":\%.2f\, \"Humidity\":\%.2f\, \"Battery Voltage\":\%.2f\, \"Device\":\"XPX-Xenon-1\"}", f, h, voltage);
    
    if(isnan(h) || isnan(f))
    {
        Particle.publish("Not reading temp or humi");
    }
    
    Particle.publish("Environment Reading", data);

}

So before I was using a char data [64] and when uploaded to the xenon it would crash. The I arbitrarily decided to double that to 128.Voila! no more crashing and even before it is formatted in pretty JSON in the console! But I think it’s important to understand how to properly size it. So what are we counting here?

Cheers,

callen · January 5, 2019, 6:08am

And might I ask why

String device = "XPX-Xenon-01";

and

sprintf(data, "{\"Temperature\":\%.2f\,\"Humidity\":\%.2f\,\"Battery Voltage\":\%.2f\,\"Device\":\%s\}", f, h, voltage, device);

Outputs really ugly JSON and the value for %s Spits out random characters?

nrobinson2000 · January 5, 2019, 6:30am

We are counting the number of characters in the string (buffer). You really should use snprintf instead of sprintf so that you can limit the size of the resulting string to the size of data so that if it is too long it will truncate the string instead of crashing.

snprintf(data, sizeof(data), "...", f, h, voltage, device);

%s requires a "c-style string", AKA a char array.

Instead of using String you should use const char* so that device will be properly formatted in snprintf.

const char* device = "XPX-Xenon-01";

Your JSON looks like you need some more quotes to go with the backslashes.

This should work.

snprintf(data, sizeof(data), "{\"Temperature\":\"%.2f\",\"Humidity\":\"%.2f\",\"Battery Voltage\":\"%.2f\",\"Device\":\"%s\"}", f, h, voltage, device);

callen · January 5, 2019, 6:51am

Hmmm. Why would truncating be the solution here? Does this mean that part of device string could be cut short thus rendering the data useless I were trying to integrate into a Firebase Webhook for example?

On the characters count, am I literally counting every single character including the colons and commas?

128 is probably overkill here so I want to land on the best number with a little wiggle room for the float numbers.

In terms of quotes, the way I am outputting the data currently displays very nicely in the console. Would wrapping the data in quotes make it easier to consume on the backend of a web application?

Very awesome of you to provide great input.

Cheers

callen · January 5, 2019, 6:53am

nrobinson2000 · January 5, 2019, 7:10am

It does, but when you leave enough wiggle room there shouldn’t be a problem.

All characters are counted, including colons, commas, spaces, etc.

Wrapping the data in quotes is defintiely necessary for the device name in the JSON but you don’t need to bother wrapping the floats in quotes. It’s important to make sure the JSON is valid so it can be consumed by applications properly. You can use this site to check if the resulting JSON is valid: https://jsonlint.com

Also, in JSON its conventional to use camelCase for key names, like batteryVoltage.

ScruffR · January 5, 2019, 8:48am

Since you saw your program crash, preventing a crash can be considered more important than data integrity. After all, after a crash your correct data couldn't be processed anymore anyway And even without a crash, your string may be valid, but at the cost of other variables being corrupted in the process.

In an ideal world, you'd check the return value of snprintf() to know whether or not your data did fit into the provided buffer and if it didn't you could print a message saying: "Ey man! Your buffer was too short - revisit your code ".
With a buffer overflow resulting in corrupted data in your other variables a crash preventing that message from being printed is highly likely.

As for your buffer size - to expand on @nrobinson2000's answer - you need to count all the characters you literally have in your format string (excluding the escaping backslash (\)) plus the maximum length of all your variables that will be inserted plus 1 for the terminating zero.

Or if you are lazy like me, you may just do this

  char huge[1024];
  int len = snprintf(huge, sizeof(huge)
                    , "{\"Temperature\":\"%.2f\",\"Humidity\":\"%.2f\",\"Battery Voltage\":\"%.2f\",\"Device\":\"%s\"}"
                    ,  -999.99, 99.99, 99.99, "TheLongestDeviceNameImagninable");
  Serial.println(len+1); // this should be the worst case length required

That is because you are using a String object as input variable for %s but %s only takes C strings (aka character arrays). If you want to use a String you can write it like this

sprintf(data, "\Device\":\%s\"", (const char*)device);
// or
sprintf(data, "\Device\":\%s\"", device.c_str());

But the general idea is to avoid String where possible.

callen · January 6, 2019, 6:22am

Hahaha! Never thought of doing that. I haven’t been using serial on these devices. I guess you could say I console log using publishes to the cloud. Data abuse much? Sorry Particle.

I bought a c++ book today. Hopefully it will help me learn more about programming languages outside of ES(Whichever Number they call it these days)

Cheers,

callen · January 7, 2019, 2:28am

@ScruffR Why doesn’t this work. I flashed it to a Xenon and it’s just breathing blue?

SYSTEM_MODE(MANUAL); // Will controll manually for mesh connections

#include <Seeed_DHT11.h>

#define DHTPIN D2
#define SENDLED D7

DHT dht(DHTPIN);

unsigned long previousTime;
int LEDSTATE = LOW;

const long interval = 2000; // Delay Amount
const char* device = "XPX-Xenon-01";

void setup() {
    pinMode(SENDLED, OUTPUT);
    dht.begin();
}

void loop() {
    
    unsigned long currentTime = millis();
    
    if ( currentTime - previousTime >= interval)
    {
        previousTime = currentTime;
        
        Mesh.on();
        
        while ( Mesh.connecting() )
        {
            // pause for connection
        }
        
        if ( Mesh.ready() )
        {
            char data[128];
            sprintf(data, "{\"Temperature\":\%.2f\, \"Humidity\":\%.2f\, \"Battery Voltage\":\%.2f\, \"Device\":\%s\"}", checkTemp(), checkHumidity(), checkBattery(), device);
            Mesh.publish("Environment Read", data);
            delay(interval);
            Mesh.off();
        }

    }
    
}

float checkHumidity()
{
    return dht.getHumidity();
}

float checkTemp()
{
    return dht.getTempFarenheit();
}

float checkBattery()
{
    return analogRead(BATT) * 0.0011224;
}

The intention here is to eventually increase the interval time and turn on/off the Mesh as needed. It’s not quite panning out that way.

Cheers,

callen · January 7, 2019, 2:52am

Sorry,

@ScruffR I got it working with:

SYSTEM_MODE(MANUAL); // Will controll manually for mesh connections

#include <Seeed_DHT11.h>

#define DHTPIN D2
#define SENDLED D7

DHT dht(DHTPIN);

unsigned long previousTime;
int LEDSTATE = LOW;

const long interval = 20000; // Delay Amount
const char* device = "XPX-Xenon-01";

void setup() {
    pinMode(SENDLED, OUTPUT);
    dht.begin();
}

void loop() {
    
    unsigned long currentTime = millis();
    
    if ( currentTime - previousTime >= interval)
    {
        previousTime = currentTime;
        
        Mesh.on();
        Mesh.connect();
        
        while ( Mesh.connecting() )
        {
            // pause for connection
        }
        
        if ( Mesh.ready() )
        {
            char data[128];
            sprintf(data, "{\"Temperature\":\%.2f\, \"Humidity\":\%.2f\, \"Battery Voltage\":\%.2f\, \"Device\":\"%s\"}", checkTemp(), checkHumidity(), checkBattery(), device);
            Mesh.publish("Environment Read", data);
            digitalWrite(SENDLED, HIGH);
            delay(2000);
            digitalWrite(SENDLED, LOW);
            Mesh.off();
        }

    }
    
}

float checkHumidity()
{
    return dht.getHumidity();
}

float checkTemp()
{
    return dht.getTempFarenheit();
}

float checkBattery()
{
    return analogRead(BATT) * 0.0011224;
}

And then just for fun I flashed this to my gateway device (Xenon) It’s really cool to see the Mesh working like this.

void setup() {
    Mesh.subscribe("Environment Read", myHandler);
}

void loop() {

}

void myHandler(const char *event, const char *data)
{
  Serial.printlnf("event=%s data=%s", event, data ? data : "NULL");
  Particle.publish(event, data);
}

I am going to look at making my gateway a little more clever. Like seeing if I can store all my floats into an array and only do a publish if there is a change +/- a certain threshold. Because if the data remains constant why waste battery power or data publishing the same old data.

Cheers,

Topic		Replies	Views
[Xenon] Serial Missing Data Troubleshooting xenon	14	781	November 10, 2019
JSONBufferWriter - how big should buf be? Firmware	3	576	June 3, 2021
Photon vs Argon/Xenon for emonlib Hardware	15	615	April 15, 2021
Particle.publish -> sprintf -> Hard fault Troubleshooting	3	930	January 18, 2018
Many unresponsive Xenons with Ethernet Troubleshooting xenon	34	1579	July 2, 2019

Need help understanding Buffer size

Related Topics