3

I'm brand new to Arduinos. I'm currently trying to modify a script from here which at the moment, simply turns an LED on or off depending on which HTTP GET request it receives. However, I want to modify this to have the LED modulate on/off when presented with the On HTTP GET request and then simply turn off all together when presented with the Off HTTP GET request. However, I have two conflicting methods that I have found about how to achieve this; the first is:

int value = LOW;
    if (request.indexOf("/LED=ON") != -1)  {
      value = HIGH;
      do
      {
      digitalWrite(ledPin, HIGH);
      delay(3000);
      digitalWrite(ledPin, LOW);
      delay(5000);
      value = HIGH;
      } while (value = HIGH);
    }
    if (request.indexOf("/LED=OFF") != -1)  {
      digitalWrite(ledPin, LOW);
      value = LOW;
    }

Wheareas the other is:

  int value = LOW;
    if (request.indexOf("/LED=ON") != -1)  {
      value = HIGH;
      void loop() {
      digitalWrite(ledPin, HIGH);
      delay(3000);
      digitalWrite(ledPin, LOW);
      delay(5000);
      value = HIGH;
      }
    }
    if (request.indexOf("/LED=OFF") != -1)  {
      digitalWrite(ledPin, LOW);
      value = LOW;
    }

As you can see, the first uses do...while and the second uses loop(). I have a feeling that the first of the two scripts would be more suitable as it should cut out as soon as value is set to LOW (i.e. when requested to turn off)

However, I would really appreciate it if someone could look through the two scripts and tell me which of the two is the correct way to achieve this sort of cut off when the HTTP GET Off is requested (if either for them even is the correct way to do this!).

Thank you in advance for your help,

Kind regards, Tom

  • Unfortunately they're both completely wrong. Use a state machine instead. – Ignacio Vazquez-Abrams Jul 20 '18 at 17:40
  • @IgnacioVazquez-Abrams Thank you for your response; that's a shame, how would I do that? – Tom Jul 20 '18 at 17:42
  • how does the do...while loop exit in the first snippet? – jsotola Jul 20 '18 at 18:00
  • I assumed that when the off get request is made, you can see from the script that"value" is set to LOW and when this condition is met, the loop should exit – Tom Jul 20 '18 at 18:01
  • 1
    With the do...while value never changes to LOW, since the code that changes it is outside of the do...while and isn’t being run while value is HIGH. try moving the int value... outside the loop function (i.e. global variable), and removing the do...while (only leave the code inside it). Not sure if checking http requests only every 8 seconds will work. – Gerben Jul 20 '18 at 18:33
4

As pointed out by Ignacio Vazquez-Abrams and jsotola, you need to use a state machine, like the one from the Blink Without Delay Arduino tutorial. Yours would be slightly more complicated because you need a way to turn it on and off, and because you use different on and off periods. Here is an example code that abstract the blinking logic into three functions:

  • update_led() takes care of the blinking, and it has to be called quite often in order to achieve a smooth blinking
  • start_blinking() and stop_blinking(), as their names suggest, are used to turn the blinking state machine on and off.
const uint32_t ON_TIME  = 3000;
const uint32_t OFF_TIME = 5000;

bool led_blinking;     // are we blinking the LED?
bool led_on;           // is the LED currently on?
uint32_t last_toggle;  // last time it toggled while blinking

void update_led() {
    if (!led_blinking) return;
    uint32_t now = millis();
    if (led_on && now - last_toggle >= ON_TIME) {
        digitalWrite(ledPin, LOW);
        led_on = false;
        last_toggle = now;
    }
    if (!led_on && now - last_toggle >= OFF_TIME) {
        digitalWrite(ledPin, HIGH);
        led_on = true;
        last_toggle = now;
    }
}

void start_blinking() {
    digitalWrite(ledPin, HIGH);
    led_blinking = true;
    led_on = true;
    last_toggle = millis();
}

void stop_blinking() {
    digitalWrite(ledPin, LOW);
    led_blinking = false;
    led_on = false;
}

You then have to call these functions from loop() as follows:

void loop() {
    // Keep the LED blinking if necessary.
    update_led();
    ...
    if (request.indexOf("/LED=ON") != -1)  {
        start_blinking();
    }
    if (request.indexOf("/LED=OFF") != -1)  {
        stop_blinking();
    }
    ...
}

Edit, in response to jsotola’s comments:

When answering the question, I was initially tempted to make a BlinkingLed class, with the functions as its public interface and the variables private. I resisted the temptation on the grounds that, given the apparent OP’s level of C++ knowledge, that could end up being well over his head. Instead, I used documentation as an informal way of separating the public from the private: you are only supposed to use what is documented, i.e. the functions. You don't “take a shortcut in your program and clear led_blinking directly”.

You could, of course, make the led_blinking variable part of the public interface. That could even be a nice simplification, as you don't need the start/stop function anymore. But then you loose control over the initial phase of the blinking pattern. That may not be an issue, but if you want the LED to turn on as soon as you set led_blinking to true, then you have to do something like

void update_led() {
    uint32_t now = millis();
    if (!led_blinking) {
        digitalWrite(ledPin, LOW);
        led_on = false;
        last_toggle = now - OFF_TIME;  // fix initial phase
        return;
    }
    ...
}

Note that the subtraction can overflow, but that's not as issue because it follows the rules of modular arithmetics.

I actually kind of like this option, with all the logic in a single function. This way the variables led_on and last_toggle can be made static local to the function, which provides true encapsulation without a class. However, from a pedagogic point of view, I think my initial proposal is easier to understand.

  • Thank you so much for your answer! This is exactly what I was looking for! I appreciate all the help that people have supplied me with in the comments but I really appreciate the fact that you have been able to break down the problem into understandable chunks and provide a well structured answer. Thank you! – Tom Jul 20 '18 at 20:10
  • change this line if (!led_blinking) return; so that it turns off the LED and then returns – jsotola Jul 20 '18 at 22:17
  • @jsotola I'm confused as to what you mean. Are you saying that this line he has written is incorrect? If so, what should the line read? – Tom Jul 20 '18 at 23:06
  • no, i am not saying that it was written wrong ... the LED will certainly stop blinking when led_blinking==false ..... if led_blinking is cleared (set to false) by the stop_blinking() function, then the LED will be off ..... however, if you take a shortcut in your program and clear led_blinking directly without calling stop_blinking(), then the LED will stop blinking, but it may be on – jsotola Jul 21 '18 at 1:24
  • @jsotala Ah, I see, that makes sense. How exactly would I adapt that code to have it that when led_blinking==false, the led will turn off completely even if currently on? Would you be able to post some code which may help fix this problem? Thank you very much :) – Tom Jul 21 '18 at 1:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.