Weather Forecasts, Returned Data, and Blinking LEDs with millis()

Using Arduino Project Guidance
#1

Thanks in advance for those who are able provide guidance and to educate me on my issue.

Some background on what I’m trying to accomplish. I’m building a sketch that displays the weather forecast using ambient design. Information is pulled from a PHP script that builds a page that displays a unique character indicating the weather forecast (S=Same C=Cold W=Warm). The LEDs display the forecast temperature and any precipitation.

Using the TwitterClient ethernet example, I’ve been able to retrieve the data from my site and turn on the correct LED. In the process of building this sketch I’ve also learned the difference between delay() and millis(), which is where I’m stumbling now. In the example sketch below, I’d like the LEDs (for now) to blink if the forecast matches, which does not occur and the LED simply turns on. My understanding is that the Arduino is running through the loop very quickly and I’m certain my blink timer is not able to run fully because of this. Ideally the LED should blink in the duration between server calls. My hunch is that the blink timer does initiate, but it’s too quick to fully run before readingforecast = false

Using Uno with ethernet shield.

Looking forward to your responses.

#include <SPI.h>
#include <Ethernet.h>

byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
char serverName[] = "lessismorecast.com";
EthernetClient client;

const int whitePin = 9;
const int redPin = 6;
const int bluePin = 5;
const unsigned long requestInterval = 30000;  // delay between requests
unsigned long lastAttemptTime = 0;  // last time connected to the server
unsigned long currentTime;            
unsigned long loopTime;
String currentLine = "";  // string to hold the text from server
String forecast = "";  // string to hold the forecast
boolean readingforecast = false;  // if you're currently reading the forecast
boolean requested;  // whether you've made a request since connecting

void setup() {
  pinMode(whitePin, OUTPUT);
  pinMode(redPin, OUTPUT);
  pinMode(bluePin, OUTPUT);  

  currentTime = millis();
  loopTime = currentTime;  

  currentLine.reserve(256);  // reserve space for the strings
  forecast.reserve(150);  // reserve space for forecast strings

  // Start up the ethernet shield
  Serial.begin(9600);
  if (Ethernet.begin(mac) == 0) {  // start ethernet using mac & IP address
    Serial.println("Failed to configure Ethernet using DHCP");  
    while(true);  // no point in carrying on, so stay in endless loop
  }

  connectToServer();
}

void loop()
{

  if (client.connected()) {
    if (client.available()) {  // read incoming bytes
      char inChar = client.read();
      currentLine += inChar;  // add incoming byte to end of line
      if (inChar == '\n') {  // if you get a newline, clear the line
        currentLine = "";
      } 
      
      if ( currentLine.endsWith("<text>")) {  // if the current line ends with <text>, it will be followed by the forecast
        readingforecast = true;  // Clear the forecast string
        forecast = "";
      }

      if (readingforecast) {  // Add bytes to the forecast String
        if (inChar == 'S') {
          Serial.println("tomorrow is same");
          currentTime = millis();
          if(currentTime >= (loopTime + 1000)){  
            digitalWrite(whitePin, !digitalRead(whitePin));  // toggles white LED on/off
            loopTime = currentTime;  // Updates loopTime
          }
        }
      }

      if (readingforecast) {
        if (inChar == 'W') {
          Serial.println("tomorrow is warmer");
          currentTime = millis();
          if(currentTime >= (loopTime + 1000)){  
            digitalWrite(redPin, !digitalRead(redPin));  // toggles red LED on/off
            loopTime = currentTime;  // Updates loopTime
          }
        }
      }

      if (readingforecast) {
        if (inChar == 'C') {
          Serial.println("tomorrow is colder");
          currentTime = millis();
          if(currentTime >= (loopTime + 1000)){  
            digitalWrite(bluePin, !digitalRead(bluePin));  // toggles blue LED on/off
            loopTime = currentTime;  // Updates loopTime
          }
        }
      }

      if (readingforecast) {
        if (inChar != '<') {
          forecast += inChar;
        } 
        else {
          readingforecast = false;  // End of forecast if "<" character is reached
          Serial.println(forecast);   
          Serial.println("disconnecting...");
          client.stop();  // close the connection to the server
        }
      }
    }   
  }
  else if (millis() - lastAttemptTime > requestInterval) {
    connectToServer();  // if you're not connected, and two minutes have passed since your last connection, then attempt to connect again
  }
}

void connectToServer() {
  digitalWrite(whitePin, LOW);  // Reset LED to off
  digitalWrite(redPin, LOW);  // Reset LED to off
  digitalWrite(bluePin, LOW);  // Reset LED to off
  
  // attempt to connect, and wait a millisecond:
  delay(1000);  // give the Ethernet shield a second to initialize
  Serial.println("connecting...");
  if (client.connect(serverName, 80)) {  // Connect to server
    Serial.println("connected...");
    // make HTTP GET request to twitter:
    client.println("GET /test.html HTTP/1.0");  // Make a HTTP request
    client.println("HOST: lessismorecast.com");
    client.println("User-Agent: Arduino 1.0");
    client.println();
  }

  lastAttemptTime = millis();  // note the time of this connect attempt
}
#2

xmasons:

digitalWrite(whitePin, !digitalRead(redPin));  // toggles red LED on/off

That doesn't do anything to the red LED as you're writing to the white LED.

digitalWrite(whitePin, !digitalRead(bluePin));  // toggles blue LED on/off

That doesn't do anything to the blue LED as you're writing to the white LED.

#3

Henry_Best:
That doesn't do anything to the red/blue LED as you're writing to the white LED.

Good catch. I've updated the sketch above to reflect your observation.

#4

xmasons:

Henry_Best:
That doesn't do anything to the red/blue LED as you're writing to the white LED.

Good catch. I've updated the sketch above to reflect your observation.

That seems very relevant to the problem, so what impact did this correction have on the symptoms?

The design is not quite right - the logic to flash the LEDs is mixed up with the input processing which means the flashing will only happen at certain points in the input processing sequence. I'm not certain this is causing the problem, but it seems pretty likely to me. I suggest you declare some global variables which record what each LED is supposed to be doing (on, off, flashing) and a millis() value for each one for use when they're flashing. Separate the code into two parts. The input processing code is executed when there is input available from the client - this is the logic to read the next character, keep track of where you are within the response, parse the forecast message and update the global state variables to indicate what the LED outputs should be doing. As an aside, the way you have implemented the parsing control using several discrete flags and buffers relies on you updating all the flags correctly and would be much simpler, clearer and more robust if you implemented it as a finite state machine. Also, while it probably isn't causing your problem I recommend you stop using the String class and use simple c-strings (null-terminated char arrays) instead since the String class exposes a memory corruption problem up to version 1.0.4 and introduces the risk of memory fragmentation in all versions - what you're doing with the String class can be done just as easily using c-strings.

Separately, you should have some code which runs on a regular basis using the approach demonstrated in the 'blink without delay' example sketch, which makes the LEDs turn on/off/blink as required.

Also, the code you're using to make the millis comparisons is not quite right. You have:

if(currentTime >= (loopTime + 1000))

You should use:

if(currentTime - loopTime >= 1000)

At first glance they appear equivalent, but the second version handles timer rollover correctly whereas the first version doesn't.

#5

PeterH:
That seems very relevant to the problem, so what impact did this correction have on the symptoms?

Sadly, the change made no difference.

The design is not quite right - the logic to flash the LEDs is mixed up with the input processing which means the flashing will only happen at certain points in the input processing sequence. I’m not certain this is causing the problem, but it seems pretty likely to me. I suggest you declare some global variables which record what each LED is supposed to be doing (on, off, flashing) and a millis() value for each one for use when they’re flashing. Separate the code into two parts. The input processing code is executed when there is input available from the client - this is the logic to read the next character, keep track of where you are within the response, parse the forecast message and update the global state variables to indicate what the LED outputs should be doing.

With my limited understanding, this is what I’m suspecting as well. I’m not exactly sure how to enact your suggestion (I’m learning by code samples from the Playground and other sources online). Could you provide an example of how you’d see this done?

As an aside, the way you have implemented the parsing control using several discrete flags and buffers relies on you updating all the flags correctly and would be much simpler, clearer and more robust if you implemented it as a finite state machine.

My sketch is a programming Frankenstein monster of pieces cobbled together and I’ll take your advice on cleaning it up for the future.

Also, while it probably isn’t causing your problem I recommend you stop using the String class and use simple c-strings (null-terminated char arrays) instead since the String class exposes a memory corruption problem up to version 1.0.4 and introduces the risk of memory fragmentation in all versions - what you’re doing with the String class can be done just as easily using c-strings.

Separately, you should have some code which runs on a regular basis using the approach demonstrated in the ‘blink without delay’ example sketch, which makes the LEDs turn on/off/blink as required.

Also, the code you’re using to make the millis comparisons is not quite right. You have:

if(currentTime >= (loopTime + 1000))

You should use:

if(currentTime - loopTime >= 1000)

At first glance they appear equivalent, but the second version handles timer rollover correctly whereas the first version doesn’t.

Oddly enough it works with the former and not the latter. You can try this is the following sketch:

const int whitePin = 9;
unsigned long currentTime;            
unsigned long loopTime;

void setup() {
  pinMode(whitePin, OUTPUT);
}

void loop()
{
          currentTime = millis();
          if(currentTime >= (loopTime + 1000)){  
            digitalWrite(whitePin, !digitalRead(whitePin));  // toggles white LED on/off
            loopTime = currentTime;  // Updates loopTime
          }
}

Thank you so far in your advice on this project. I really appreciate the assistance that you’ve been able to give me and any further that you may have to give.

#6