Tank Control not working as intended

The eventual purpose of this sketch is to start filling a tank when the lower float switch is low, and shut off when the upper float switch is high.

Whereas I am thrilled that the sketch is publishing, it’s not accurate. No matter whether the floats are “triggered” or not, the published content remains the same.

To test, I’ve got a test Photon sitting on the desk and am using manual connections to assure there are no irregularities having to do with bad switches and such.
I am very new, please help.
Here is the sketch:


const int lowFS = D1;    //lower float swithch
const int highFS = D2;   //High float swithch

int buttonState;
int buttonState1;

void setup() {
pinMode(lowFS,INPUT_PULLUP);
pinMode(highFS,INPUT_PULLUP);
}

void loop() {
    delay(500);
        
      delay(2000);
    buttonState = digitalRead(lowFS);
    if (lowFS == HIGH)
    {   Particle.publish("low_FS is low");  }
    else 
    {   Particle.publish("low_FS is high");  }   
      
         delay(2000);
    buttonState1 = digitalRead(highFS);
    if (highFS == HIGH)
    {   Particle.publish("high_FS is low");  }
    else 
    {   Particle.publish("high_FS is high");  }   
   
}

You are not checking whether the pin state is HIGH (1) but whether the pin number is 1.
If you substitute tha variables you'd end up with this

  
  if (1 == 1) // since D1 == 1, HIGH == 1
  if (2 == 1) // since D2 == 2, HIGH == 1

You actually want to check if(buttonState == HIGH) - although I'd argue whether buttonState is a good name for a float switch :wink:

BTW, I'd rather check for if(buttonState != LOW) or if(buttonState) since there is only one FALSE (aka LOW) state, but many more TRUE states that are not the same as HIGH.
Boolean TRUE would be anything other than FALSE (0) but HIGH is 1 and only 1.

Thank you so much for your help.
Here is what I ended up with.
It compiles and it looks like it will work.

const int lowerFS = D1; //lower float swithch
const int upperFS = D2; //High float swithch
const int ROvalve = D6;   //RO valve
const int MainValve = D4; //Main valve the main valve needs to be on before any water will flow

int lowerFSvalue = 0;
int upperFSvalue = 0;

void setup() {
  pinMode(lowerFS,INPUT_PULLUP);
  pinMode(upperFS,INPUT_PULLUP);
  pinMode(ROvalve,OUTPUT);
  pinMode(MainValve,OUTPUT);
}

void loop() { 
  delay(2000);
  lowerFSvalue = digitalRead(lowerFS);
  if (lowerFSvalue == HIGH)
    Particle.publish("lowerFS is low"); 
  else 
    Particle.publish("lowerFS is high");
      
  delay(2000);
  upperFSvalue = digitalRead(upperFS);
  if (upperFSvalue == HIGH)
    Particle.publish("upperFS is low");   
  else 
    Particle.publish("upperFS is high");  
    
  if (upperFSvalue == LOW & lowerFSvalue == LOW)
  {
    digitalWrite(ROvalve,HIGH);
    digitalWrite(MainValve,HIGH);
  }
  else if (upperFSvalue == HIGH & lowerFSvalue == HIGH)
  {
    digitalWrite(ROvalve,LOW);
    digitalWrite(MainValve,LOW);
  }
}

(ScruffR: Format and indentation adjusted)

In conditional checks you actually want to use && (logic AND) instead of the binary &.

You can also rewrite that entire block like this

  if (upperFSvalue == lowerFSvalue)
  {
    digitalWrite(ROvalve, !upperFSvalue);
    digitalWrite(MainValve, !upperFSvalue);
  }

You only act when both states are the same and then the state of the floats will just be inverted.

You should also limit the scope of your publishes to PRIVATE and also consider only publishing when one of the states changes.

This would be how I'd do it

const uint32_t REPORT_RATE  = 60 * 60 * 1000; // report at least once per hour
const int      lowerFS      = D1;             // lower float switch
const int      upperFS      = D2;             // upper float switch
const int      ROvalve      = D6;             // RO valve
const int      MainValve    = D4;             // Main valve the main valve needs to be on before any water will flow

int            lowerFSvalue = -1;             // ensure the initial state will cause a state update
int            upperFSvalue = -1;             //   by initialising with an otherwise impossible value

void setup() {
  pinMode(lowerFS  , INPUT_PULLUP);
  pinMode(upperFS  , INPUT_PULLUP);
  pinMode(ROvalve  , OUTPUT);
  pinMode(MainValve, OUTPUT);
}

void loop() {
  static uint32_l msLastReport = 0;
  char msg[64];
  int curLower = !digitalRead(lowerFS);       // invert the value for ease of interpretation later on
  int curUpper = !digitalRead(upperFS);
  if(curLower == lowerFSvalue && curUpper == upperFSvalue && millis() - msLastReport < REPORT_RATE) 
    return;                                   // no change in state within last hour, so nothing new to report
  msLastReport = millis();                    // store time of latest report
  lowerFSvalue = curLower;                    // update the global variables to match the current state
  upperFSvalue = curUpper;
 
  // the numeric representation of the state should normally be clear enough
  snprintf(msg, sizeof(msg), "lowerFS: %d, upperFS: %d", curLower, curUpper);
 
  // but if you REALLY need the verbal description of the state, you can use this instead 
  //snprintf(msg, sizeof(msg)
  //        , "lowerFS: %s, upperFS: %s"
  //        , curLower ? "HIGH" : "LOW"
  //        , curUpper ? "HIGH" : "LOW");

  Particle.publish("FloatState", msg, PRIVATE);

  if(upperFSvalue == lowerFSvalue) {
    digitalWrite(ROvalve  , upperFSvalue);
    digitalWrite(MainValve, upperFSvalue);
  }
}
4 Likes