The function does not work in ESP8266

Hello. There is a code for ESP8266 in which I control the LED via wi-fi. I would like to know how to start the led_blink function at /On and how to exit it at /Off. Since I did, it doesn't work. The LED just turns on when?On and off on /Off. How do I do this?

#include <ESP8266WiFi.h>


const char* ssid = "1234567890";
const char* password = "1234567890";



WiFiServer server(80);

const byte R1 = 5;

boolean led_state = 0;
boolean flag = 0;


void setup() {
  pinMode(R1, OUTPUT);
  digitalWrite(R1, LOW);
  Serial.begin(115200);
  delay(10);
  Serial.println();
  Serial.println();
  Serial.print("Conecting to");
  Serial.println(ssid);

  WiFi.begin(ssid, password);

  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }
  Serial.println("");
  Serial.println("WiFi connected");
  server.begin();
  Serial.print("Use this URL to connect:");
  Serial.print("http://");
  Serial.print(WiFi.localIP());
  Serial.println("/");



}

void loop() {
  WiFiClient client = server.available();
  if (!client) {
    return;
  }
  while (!client.available()) {
    delay(1);
  }
  String request = client.readStringUntil('/r');
  client.flush();
  client.println("HTTP/1.1 200 OK");
  client.println("Content-Type: text/html");
  client.println("");
  client.println("<!DOCTYPE HTML>");
  client.println("<html>");

  if (request.indexOf("/On") != -1) {
    flag = !flag;
  }
  else if (request.indexOf("/Off") != -1) {
    digitalWrite(R1, LOW);
  }
  if(flag){
    led_blink();
  }
  

}

void led_blink(){
  unsigned long timing = 0;
  if (millis() - timing > 2000){
    timing = millis();
    led_state = !led_state;
    digitalWrite(R1, led_state);
  }
}

This will not work, because your 'timing' variable is a local dynamic variable, that will be newly created and set to 0 with every call to led_blink. You must define this as a static variable.

static unsigned long timing = 0;
1 Like

The LED doesn't blink anyway

Move the lines

at the very start of loop()

And show the resulting code

1 Like

Good catch!

@arduino556665
Because of these lines:

your blinking function is never called after receiving a command from the client.
And there are more problems in your sketch:
This:

  String request = client.readStringUntil('/r');

should be

  String request = client.readStringUntil('\r');

And this:

  if (request.indexOf("/On") != -1) {
    flag = !flag;
  }
  else if (request.indexOf("/Off") != -1) {
    digitalWrite(R1, LOW);
  }

should be

  if (request.indexOf("/On") != -1) {
    flag = true;
  }
  else if (request.indexOf("/Off") != -1) {
    digitalWrite(R1, LOW);
    flag = false;
  }

Otherwise you will not be able to switch the blinking off with /Off

I also would be add the call to led function to the while() loop:

 while (!client.available()) {
    delay(1);
  }

changing it to

while (!client.available()) {
    delay(1);
    if (flag) led_blink();
  }

Wouldn't it be better to omit the while at all and change it to a simple if?
Also the return; isn't really a good idea. loop() should loop - from start to end :wink: .

Agreed
Instead of

it would be better something like it:

 if (client  && client.available()) {
   String request = client.readStringUntil('\r');
   client.flush();
  ...  // work with request

Why ?

It is legitimate for loop() to return to main() if there is no more code for it to execute. main() will then call loop() again

Because it is a nasty trap to do so - as can be seen in this thread.

Why is it a nasty trap and what would you propose is used instead ?

I agree with @MicroBahner. Using return as quick 'n dirty escape from the rest of loop() is not good code structure. Not as bad as using goto, but not good.

Instead, use if/else to skip the remaining code in loop().

I try to use return only in non-void functions, and even then, have only one return statement where the result is returned to the caller.

I agree with @MicroBahner and you can see in #8 how it would be changed

That is exactly what the return does. What is so special about the code reaching the closing brace of loop() ?

Because if you add some extra code just before the end of loop(), as @arduino556665 did, it won't be executed because of the return. But if you structure your if/else correctly, you would skip only the parts of the code you intended to skip (in this case when no client is connected) but still execute other code later in loop() that was intended to execute.

This isn't a law, just an opinion! I probably got my opinion from my uni lecturer in structured programming ~40 years ago.

1 Like

I agree that it a valid reason not to use return in the way that I phrased my question

Perhaps I should have asked, why not use return when all of the required code in loop() has been executed ?

As you say, it is a matter of opinion, but if the logic is followed to its conclusion then the code should never return early from a function even when a value is being returned, but it is common to do so

That's exactly the trap. And I've seen it snap shut so many times in the many decades I deal with programming.
So my experience is not to do so. A function should be left with the last closing brace. And a return that's returns a value should be immediately before that brace. It can save you a lot of troubleshooting.
The only circumstances where I see it valuable to have a return at the beginning of a function, if an error condition is detected that makes it useless to execute the function at all.

1 Like

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.