Issues with my code for reading data

[xinjie00]

issues: so it work reading data from webpage to get data, but it only will get once after that it wont update the data even though it's in the loop

#include <math.h>
#include <Wire.h>
#include "rgb_lcd.h"
#include <Ethernet.h>
#include <PubSubClient.h>
//server
byte mac[]    = {  0xA8, 0x61, 0x0A, 0xAE, 0x2E, 0x03 };   //pls change to your arduino MAC, refer to the sticker on the side
IPAddress server(192, 168, 10, 122);   //change to the php server ip      
EthernetClient client;
//preset if statement 
int reading = LOW;
int state = HIGH;
int previous = LOW;
int buttonPin = 6;
int stateTime = 0;
int debounce = 100;
//lcd colour 
rgb_lcd lcd;
const int colorR = 255;
const int colorG = 255;
const int colorB = 255;
//define
int buzz=4;
int light=3;
String coolant;
String temperature;
void setup() {
  pinMode(buttonPin, INPUT);
  pinMode(buzz, OUTPUT);
  pinMode(light, OUTPUT);
  Serial.begin(9600);
  Serial.println("Read data from database");
  Ethernet.begin(mac); 
  delay(1500); //wait for connection  
  //print connection detail 
  Serial.print("Arduino IP Address        : ");   //ip
  Serial.println(Ethernet.localIP());   
  Serial.print("Subnet Mask       : ");
  Serial.println(Ethernet.subnetMask());
}
void loop() {
  read_arduinodata();
  // read the pushbutton input pin
  reading = digitalRead(buttonPin);
  // determine if the button is pressed with debounce
  if (reading == HIGH && previous == LOW && (millis() - stateTime) > debounce)
  {
    Serial.println("Button PUSHED");
    if (state == HIGH)
    {
      state = LOW;
      Serial.println("Status changed to LOW");
      lcd.begin(16, 2);
      lcd.print("ON");
      delay(2000);
    }
    else {
      state = HIGH;
      Serial.println("Status changed to HIGH");
      lcd.begin(16, 2);
      lcd.print("OFF");
      delay(2000);
      lcd.noDisplay();
      delay(2000);
      lcd.setRGB(colorR, colorG, colorB);
    }
    stateTime = millis();
  }
  if (state == LOW & client.connect(server, 80))
  { 
    lcd.begin(16, 2);
    lcd.setRGB(colorR, colorG, colorB);
    if (coolant == "1" )
    {
      digitalWrite(buzz,HIGH);
      lcd.setCursor(0, 0);
      lcd.print("coolant:");
      lcd.print("ABNORM");
    }
    if (temperature == "1")
    {
      digitalWrite(light,HIGH);
      lcd.setCursor(0, 1);
      lcd.print("temp:");
      lcd.print("ABNORM" );
    }
    else
    {
      digitalWrite(buzz,LOW);
      digitalWrite(light,LOW);
        //lcd
      lcd.setCursor(0, 0);
      lcd.print("coolant:");
      lcd.print("NORM");
      lcd.setCursor(0, 1);
      lcd.print("temp:");
      lcd.print("NORM" );
      delay(1000); 
    }
}
  // update previous button state reading
  previous = reading;
}
void read_arduinodata(){  
  if (client.connect(server, 80))
  {
    Serial.println("-> Connected");
    Serial.println("Reading");
    client.print("GET http://localhost/test.php");   //change based on the server ip
    client.println( " HTTP/1.1");
    client.print( "Host: " );
    client.println( "Connection: close" );
    client.println();
    String receive = client.readString();   
    //Serial.println(receive); //activate this line to see the actual text you receive
    //Serial.println(receive.length()); //activate this line to see the length of text you receive
    coolant=receive.substring(receive.length()-93,receive.length()-94);
    temperature=receive.substring(receive.length()-36,receive.length()-37);
    Serial.print("coolant:");
    Serial.println(coolant);
    Serial.print("temperature:");
    Serial.print(temperature);
    client.println();
  }


}
    
    

[Railroader]

The delays of 2000 milliseconds looks bad too me. Lots can take place during that time.

[xinjie00]

i tried changing the delay it still doesn work

[Railroader]

Okey.(Try to erase all white, empty lines in the code. They makes the scrolling finger tired)

This sequence looks strangely placed in the code. Maybe not the fault but it sticks out. Constants ought to be defined once and for all earlier in the code.

@xinjie00

     const int colorR = 0;
      const int colorG = 0;
      const int colorB = 0;

[xinjie00]

tried doesn't work...

[Idahowalker]

@xinjie00

int stateTime = 0;

@xinjie00

stateTime = millis();

That is a bad combination. Perhaps you can figure out why? Clicky, clicky for clues.

@xinjie00

reading = digitalRead(buttonPin);

Your program has over 3000ms of delay. How long is the button presser supposed to hold the button down before the program detects a button press? Expecting the button presser to hold the button down for 3+ seconds?

Rethink the use of delay.

In this case it may be a good idea to set a variable to true in a ISR attached to the button press. Then you can have your delays and check on the variable state in loop(), once the thing is done with the button you can set the variable back to false.

Still with the delays the program may wait for more then 3 seconds before it responds to the button press.

@xinjie00

  if (reading == HIGH && previous == LOW && (millis() - stateTime) > debounce)
  {

Put serial prints before this line so you can see the states so you can determine which state is not being met.

[Paul_B]

@Railroader

Okey.(Try to erase all white, empty lines in the code. They makes the scrolling finger tired)

You should have only a single blank line between distinct functions or between major modules in loop().

@Idahowalker

In this case it may be a good idea to set a variable to true in a ISR attached to the button press.

Using an interrupt to respond to a manual button press suggests extremely disorganised code. Nothing in loop() should require any more than a a few tens of milliseconds to pass through.

[Idahowalker]

@Paul_B

Using an interrupt to respond to a manual button press suggests extremely disorganised code. Nothing in loop() should require any more than a a few tens of milliseconds to pass throug

As does using over 3000ms of delay().