Arduino Stack Exchange is a question and answer site for developers of open-source hardware and software that is compatible with Arduino. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have an outdoors Arduino ethernet client that's connected via ethernet to an indoors Arduino ethernet server. The client requests a series of settings from the server every 30 seconds, and the server sends the settings as a string of 1's and 0's. Currently there are 6 settings to be read, but I might add more in the future.

Could you please examine my client code and tell me if you see any problems? So far it seems to be working, but I'm worried that maybe I'm not doing things optimally, or maybe I'm overlooking some problems in the code.

As a secondary question, I'm wondering if you think the code would be faster and more stable (and worth the extra complexity) if instead of sending 1's and 0's as strings, I somehow made it send hex values that could be interpreted as bit field settings.

This is the client code:

EthernetClient client;
const char server[] = "192.168.0.106";
byte brightness = 0, failedCounter = 0, xbeeEnable = 0;
unsigned long lastConnectionTime = 0; // last time you connected to the server
boolean lastConnected = false; // state of the connection last time through the main loop
const word postingInterval = 30 * 1000; // delay between updates
String readString;

void setup() {
  Serial.begin(9600);
  startEthernet();
  delay(3000);
}

void loop() {
  if (client.available()) {
    char c = client.read();
    if (readString.length() < 7) readString += c;
  }

  if (!client.connected() && lastConnected) client.stop();

  if (!client.connected() && (millis() - lastConnectionTime > postingInterval)) {
    httpRequest();
    if (readString.length() > 2) { //check in case it didn't receive the entire string, because sometimes the string was empty

//below are 2 examples of reading settings. I removed the others for cleaner code    
      if (readString[4] == '1') brightness = 255; else brightness = 10;
      if (readString[5] == '1') xbeeEnable = 1; else xbeeEnable = 0;

      readString = "";
    }
  }
  lastConnected = client.connected();
  if (failedCounter > 2 ) startEthernet();
}

void startEthernet()
{
  client.stop();
  delay(3000);
  failedCounter = 0;
  Ethernet.begin(mac);
  delay(5000);
}

And this is what the server is sending (an example output would be: "100011"):

client.print(redMailOn);
client.print(whiteMailOn);
client.print(blueMailOn);
client.print(yellowMailOn);
client.print(mailNotifierBrightness);
client.println(enableXbeeTransmitter);
delay(1); client.stop();
share|improve this question
2  
Write your code for people first, machines second. Design the code - and if sensible, the communication protocols too - to be as readable as possible. They will way more debugable! Then optimize IF, and only WHERE, you need to. My own rule of thumb is, if I'll be able to understand what I wrote after a year of not thinking about it, someone else should be able to pick it up cold. I haven't studied your code but it looks like you send ASCII commands that would be readable on a terminal. No reason to change if it works, and at the level of performance you need. – JRobert Oct 12 '15 at 13:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.