Eyes from a far more proficient programmer needed :)

Howdy,

As you’ll soon see I’ve had a very good stab at coding this myself, where I need your help is in making it use less memory, any general suggestions on how I can make it “better” and crucially how to improve my own coding as well at the same time!

Currently it’s compiling at:

Sketch uses 24,224 bytes (75%) of program storage space. Maximum is 32,256 bytes.

And as you can imagine I’m getting rather concerned as I still have some further processing needed of the received GET data via the Ethernet connection and I’d really like to add an IR remote to this build as well (why not!)

What the device is to do:
It’s an UNO with an Ethernet shield at the front door of the house with a PIR, Lightsensor, DHT & RGB LED Strip (via a couple of transistors).

The device is POST’ing data every N seconds to a PHP script and also able to receive GET requests back to turn things on/off etc…

Specifically I’d really value your views on the follow:

#1 Is there anything glaring obvious that I could code better? If so, what and a hint on how to do that would be “Ace”

#2 In the ethernet connections there is a lot of hard-coded string data, could this be improved by setting these values via PROGMEM ?

#3 For the line “ethClient.println( F(“Host: 192.168.0.2”) );”, how would you approach making the server address “192.168.0.2” a variable? Noting it’s used twice, once here and further up at line 277

#4 When the project is finished, is there a simple way to remove/disable all the Serial.print messages? And does this even matter?

The project so far

I’ve posted the code on GitHub as it’s easier to read there, the link is here current code and posted it as an attachment below as well.

Any feedback, good, bad or outright ugly is very welcomed! :slight_smile:

Matt

Front_Door.ino (11.8 KB)

If space available is a concern, compiling it on my Due gave me that result:

Sketch uses 44,824 bytes (8%) of program storage space. Maximum is 524,288 bytes.

c0rsa1r:
If space available is a concern, compiling it on my due gave my that result:

Sketch uses 44,824 bytes (8%) of program storage space. Maximum is 524,288 bytes.

That's just showing off :slight_smile:

I'm using a UNO board at the moment to develop against, the plan was to use a Pro Mini with an separate Ethernet board because space is an issue where the final device is to be located.

Although point taken! There are boards with more memory available. I'm trying to use up the 15 Pro Mini's I have here at the moment :wink:

Matt

Using a board with more memory would be the Intel solution. :wink:

There are many not really necessary (or unnecessary long) string constants.

You could find a way to use the DHT without using floats, so you will not pull in all that code.

Not using snprintf will further shrink your code.

String eth_received_Str;

That's just uselessly pissing away memory. If the String instance can wrap all the character data that is going to be received, then the same data could be stored in a character array without the overhead and memory mangling of the String class.

  snprintf(
          data2send, 
          sizeof(data2send), 
          "node=%&pir_1=%slight_sensor=%s&dht_h=%s&dht_t=%s&dht_f=%s&dht_f=%s&dht_hif=%s&dht_hic=%",
          NODE_NAME,
          pir_1_state,
          val_lightsensor,
          dht_h,
          dht_t,
          dht_f
        );

A % is a format specifier indicator. Why is there a % with no format specifier after it after the node= bit? Why is there no & after the pir_1=value part (between the s and light)?

Why is it necessary to post the temperature and heat index (incorrectly) in Fahrenheit AND Celsius? Don't you think the server could compute one from the other faster than the Arduino can?

Why does the number of format specifiers not match the number of additional arguments?

  // Disconnect from the server
  if (ethClient.connected()) { 
    ethClient.stop();  
  }

Who gives a rats ass if the POST succeeded?

  val_lightsensor = map(val_lightsensor, 0, 1023, 0, 255);

The map() function is compute intensive.

val_lightsensor /= 4;

would be a lot faster and nearly as accurate.

Howdy @PaulS & @Whandall,

Firstly thanks!

@Whandall

I looked into ditching the DHT library, tbh I got lost fast so decided it was far easier to include the lib and worry about the issue I had with the snprintf part.

Ironically, snprintf worked out less last time, see my earlier forums post here: https://forum.arduino.cc/index.php?topic=379807.msg2619003

@PaulS

I’ve pulled out the conversions as suggested and /= 4 is a much simpler approach.

You’re absolutely right, dump ANY processing that can be done externally, well externally!

Yea, you caught me with the snprintf lines.

Was battling with that while posting the message. At first I thought it was something I was doing (hence no mention of it) and it turns out you can’t easily convert floats using %f on an Arduino, so had to take this approach instead:

char spf_val_lightsensor[10];
  char spf_dht_h[10];
  char spf_dht_t[10];
  char spf_dht_f[10];
  
  dtostrf(val_lightsensor, 4, 2, spf_val_lightsensor);
  dtostrf(dht_h, 4, 2, spf_dht_h);
  dtostrf(dht_h, 4, 2, spf_dht_t);
  dtostrf(dht_f, 4, 2, spf_dht_f);
 
  // create data to send
  snprintf(
          data2send, 
          sizeof(data2send), 
          "node=%s&pir_1=%o&light_sensor=%s&dht_h=%s&dht_t=%s&dht_f=%s",
          NODE_NAME,
          pir_1_state,
          spf_val_lightsensor,
          spf_dht_h,
          spf_dht_t,
          spf_dht_f
        );

With regards to:

String eth_received_Str;

That’s just uselessly pissing away memory. If the String instance can wrap all the character data that is going to be received, then the same data could be stored in a character array without the overhead and memory mangling of the String class.

Can you nudge me in the right direction with that one please?

Kind regards,

Matt

Yea, you caught me with the snprintf lines.

snprintf_P would be better.

Can you nudge me in the right direction with that one please?

moggiex:
Ironically, snprintf worked out less last time, see my earlier forums post here...

If you compare it to a String version. A version using only Serial is much shorter.

#define V_STRINGCLASS
//#define V_SNPRINTF
//#define V_PRINT

void setup() {
  Serial.begin(115200);
  bool pir_1_state = true;
  bool pir_2_state = false;
  bool alarmStatus = false;
  bool alarmActive = true;
  int zone = 42;

#ifdef V_STRINGCLASS
  String data;
  data = "1=" + String("1");
  //data += "&2=" + bool_cast(pir_1_state);
  data += "&2=" + String( ((pir_1_state) ? "1" : "0") ); // Bool's
  data += "&3=" + String( ((pir_2_state) ? "1" : "0") );
  data += "&4=" + String( ((alarmStatus) ? "1" : "0") );
  data += "&5=" + String( ((alarmActive) ? "1" : "0") );
  data += "&6=" + String(zone); // Int
  Serial.println(data);
#endif

#ifdef V_SNPRINTF
  char data2send[32];
  snprintf(data2send, sizeof(data2send), "1=1&2=%s&3=%s&4=%s&5=%s&6=%d",
           (pir_1_state) ? "1" : "0",
           (pir_2_state) ? "1" : "0",
           (alarmStatus) ? "1" : "0",
           (alarmActive) ? "1" : "0",
           zone);
  Serial.println(data2send);
#endif

#ifdef V_PRINT
  Serial.print(F("1=1&2="));
  Serial.write(pir_1_state ? '1' : '0');
  Serial.print(F("&3="));
  Serial.write(pir_2_state ? '1' : '0');
  Serial.print(F("&4="));
  Serial.write(alarmStatus ? '1' : '0');
  Serial.print(F("&5="));
  Serial.write(alarmActive ? '1' : '0');
  Serial.print(F("&6="));
  Serial.println(zone);
#endif
}

void loop() {}
V_STRINGCLASS 4.142 Bytes (13%) 236 Bytes (11%)
V_SNPRINTF 3.400 Bytes (11%) 232 Bytes (11%)
V_PRINT 2.362 Bytes (7%) 200 Bytes (9%)

PROGMEM and the F() macro don't change the size of your sketch very much. They change how much RAM it uses, but in both cases the same length string has to be stored in the program memory.

I've got some sketches that are up to 99% of the program memory on a Uno. If I ever have to add any functions to them, I have to look through all of my strings and other storage to find a string that can be abbreviated. "Press Enter to go to menu" becomes "Press enter", and so on.

Got some EEPROM you could put stuff into also.
Or jump up into a '1284P board. 128K flask, 16K SRAM, 4K EEPROM. Lots of room to grow.
Dual hardware serial ports and support to run at 20 MHz has been proven out.
I offer boards in several form factors if interested, altho I've only been running at 16 MHz.
http://www.crossroadsfencing.com/BobuinoRev17/
Here's a basic Duemilanove-like board with FT232 for USB/Serial, feel free to connect your own adapter also:

PaulS:
Who gives a rats ass if the POST succeeded?

If you insist on using profanity please do so correctly. The rat is possessive of his anatomy. :wink: