Need code "proof reading" help

Like many, I am working remotely (as much as possible) and working on a sensor package that passively monitors water temp, turbidity & pH but will regulate the pH via CO2 through a gate valve. The code I have written compiles but I cannot get into work until next week to upload to the Arduino to verify the code runs as desired. Can someone proof read the code to see if it appears to do as intended?

Goal:
Every 4hrs:

  • check temp
  • check optical density (OD/turbidity)
  • check pH
    IF pH is >= threshold
  • open CO2 gate valve for 1 minute
  • wait 10 minutes (from most recent pH check time point)
  • recheck pH
    IF pH is still >= threshold repeat previous IF statement

Question: because the "pH_INTERVAL" timer is started at the beginning of the loop, regardless of changes in pH sensor readings, I will still get a pH reading at the same time I get the regularly scheduled temp & OD readings, correct?

Thank you in advance!

//     **********     ADJUSTABLE TIME POINTS BELOW     **********


#define Temp_INTERVAL             14400000ul   // 4hrs         * * * CHANGE TO FIT REQUIREMENTS * * * 
#define OD_INTERVAL               14400000ul   // 4hrs         * * * CHANGE TO FIT REQUIREMENTS * * *
#define pH_INTERVAL               14400000ul   // 4hrs         * * * CHANGE TO FIT REQUIREMENTS * * * 
#define pH_CO2_ADD_INTERVAL          60000ul   // 1 minute     * * * CHANGE TO FIT REQUIREMENTS * * *
#define pH_CO2_CHECK_INTERVAL       600000ul   // 10 minute    * * * CHANGE TO FIT REQUIREMENTS * * * 

//     **********     END OF ADJUSTABLE TIME POINTS     **********




// Optical Density Sensor
const byte OD_Probe = A14;


// CO2 GateValve
const byte CO2_GateValve = 5;      // FILL IN/CORRECT PIN #     ***************************************

// Temp Probe
const byte Temp_Probe = A2;        // FILL IN/CORRECT PIN #     ***************************************

// pH Probe
const byte pH_Probe = A3;          // FILL IN/CORRECT PIN #     ***************************************
const byte pH_HIGH_THRESHOLD = 7;  // ADJUST TO DESIRED pH      ***************************************
void setup() {

  Serial.begin(9600);

  pinMode (OD_Probe, INPUT);
  pinMode (CO2_GateValve, OUTPUT);
  pinMode (Temp_Probe, INPUT);
  pinMode (pH_Probe, INPUT);

}

void loop()
{
  int OD_Val, pH_Val, Temp_Val;
  static unsigned long timeOD_INTERVAL, timeOD_RunTimer;
  static unsigned long timepH_INTERVAL, timepH_CO2_ADD_RunTimer, timepH_CO2_CHECK_RunTimer;
  static unsigned long timeTemp_INTERVAL, timeTemp_RunTimer;
  unsigned long timeNow;
  timeNow = millis ();

  timeOD_INTERVAL = timeNow;
  timepH_INTERVAL = timeNow;
  timeTemp_INTERVAL = timeNow;

  // -----------------------------------------------------------------------------------

  // * * * * * Temp INTERVAL * * * * *

  if ( (timeNow - timeTemp_INTERVAL) >= Temp_INTERVAL )
  {
    Temp_Val = analogRead(Temp_Probe);
    Serial.print( "Temp: " );
    Serial.println( Temp_Val );
  }

  // -----------------------------------------------------------------------------------

  // * * * * * OD INTERVAL * * * * *

  if ( (timeNow - timeOD_INTERVAL) >= OD_INTERVAL )
  {
    OD_Val = analogRead(OD_Probe);
    Serial.print( "OD Reading ");
    Serial.println( OD_Val );
  }

  // -----------------------------------------------------------------------------------

  // * * * * * pH INTERVAL * * * * *

  if ( (timeNow - timepH_INTERVAL) >= pH_INTERVAL )
  {
    pH_Val = analogRead(pH_Probe);
    Serial.print( "pH Reading ");
    Serial.println( pH_Val );

    if (pH_Val >= pH_HIGH_THRESHOLD)
    {
      digitalWrite(CO2_GateValve, HIGH);
      timeNow = timepH_CO2_ADD_RunTimer;
      timeNow = timepH_CO2_CHECK_RunTimer;

      if ( (timeNow - timepH_CO2_ADD_RunTimer) >= pH_CO2_ADD_INTERVAL )
      {
        digitalWrite(CO2_GateValve, LOW);
      }
      if ( (timeNow - timepH_CO2_CHECK_RunTimer) >= pH_CO2_CHECK_INTERVAL)
      {
        analogRead(pH_Probe);
        if (pH_Val >= pH_HIGH_THRESHOLD)
        {
          digitalWrite(CO2_GateValve, HIGH);
          timeNow = timepH_CO2_ADD_RunTimer;
          timeNow = timepH_CO2_CHECK_RunTimer;
        }
      }
    }
  }
  // -----------------------------------------------------------------------------------

}// loop

in loop() you get a timestamp, timeNow and then set timepH_INTERVAL, as well as other vars to timeNow.

later you compare if timeNow - timepH_INTERVAL to an interval. but if both timeNow and timepH_INTERVAL have the same value, how can their difference be anything but zero.

shouldn't the timestamps for each case, be initialized to some start time (in setup()) and then set to timeNow when the condition is met and that case exercised?

of course since the three intervals (e.g. Temp_INTERVAL, OD_INTERVAL and pH) are the same value (14400000), each case should be performed at the same time

don't understand the point of the test for pH_CO2_ADD_INTERVAL (60000) inside the case for pH_INTERVAL since the CO2 interval expires sooner than pH_INTERVAL. same for CO2_CHECK_INTERVAL

gcjr:
in loop() you get a timestamp, timeNow and then set timepH_INTERVAL, as well as other vars to timeNow.

later you compare if timeNow - timepH_INTERVAL to an interval. but if both timeNow and timepH_INTERVAL have the same value, how can their difference be anything but zero.

These intervals are arbitrary at this point--since currently I can't do any in-situ testing. There is a good chance the temp interval could be longer and I expect the pH will rise much faster as the OD rises thus leading to needing to decrease their intervals.

gcjr:
shouldn't the timestamps for each case, be initialized to some start time (in setup()) and then set to timeNow when the condition is met and that case exercised?

Good chance I am wrong, but if the interval start time timestamps are moved to setup, wouldn't that only allow the interval to be met once?

gcjr:
of course since the three intervals (e.g. Temp_INTERVAL, OD_INTERVAL and pH) are the same value (14400000), each case should be performed at the same time

don't understand the point of the test for pH_CO2_ADD_INTERVAL (60000) inside the case for pH_INTERVAL since the CO2 interval expires sooner than pH_INTERVAL. same for CO2_CHECK_INTERVAL

If the initial pH reading is above the threshold, I want to add CO2 (1min worth), give it a chance to effect the pH (10min--from the time it starts being added) and recheck so that more CO2 can be added (if needed) before the next 4hr test time point. Since the pH increase is likely to be exponential I need a way to add more CO2 as needed and not simply add a set volume at set intervals.

These intervals are arbitrary at this point

regardless – two variables with the same value subtracted from one another will never be > zero

  timeNow = millis ();
  timeOD_INTERVAL = timeNow;


  if ( (timeNow - timeTemp_INTERVAL) >= Temp_INTERVAL )

wouldn’t that only allow the interval to be met once?

not if they are set to timeNow when the condition is met so that the conditional will be met after another INTERVAL period

the way the code is now, each condition will be met each time loop() runs after 4 hours

If the initial pH reading is above the threshold, I want to add CO2 (1min worth),

sounds like you’re expecting the code to remain in the pH expired condition for a period of time to wait for the CO2 ADD INTERVAL to expire. the timestamps have the same problem as above (can never be < zero)

if there is a procedure to make a pH measurement that takes time, then the code needs a concept of state. there are several ways to do this.

one, is to have a state variable initialized to zero. the pH_INTERVAL condition will repeatedly be met until the timepH_INTERVAL is reset to timeNow. that allows it to be invoked many times to complete a ph procedure.

when state is zero, when the pH_INTERVAL initially expires, a pH_PROC timestamp is captured (i.e. set to timeNow),

  • the CO2 valve opened and
  • the state advanced to “1”

in state 1, the CO2_ADD time is checked. when it expired,

  • the CO2 valve is closed and
  • the state advanced to “2”

in state 2, the CO2_CHECK interval is checked. when it expires

  • measure the pH
  • set state to zero
  • set pH_INTERVAL to timeNow

i hope you understand the concepts i’m explaining.

shouldn’t the timestamps for each case, be initialized to some start time (in setup()) and then set to timeNow when the condition is met and that case exercised?

Good chance I am wrong, but if the interval start time timestamps are moved to setup, wouldn’t that only allow the interval to be met once?

As, @gcr said, the start time timestamps need to be updated to time now when the condition is met. See the “blink without delay” ide example.

if ( (timeNow - timeTemp_INTERVAL) >= Temp_INTERVAL )
  {
     timeTemp_INTERVAL = timeNow;
    Temp_Val = analogRead(Temp_Probe);
    Serial.print( "Temp: " );
    Serial.println( Temp_Val );
  }

gcjr:
in loop() you get a timestamp, timeNow and then set timepH_INTERVAL, as well as other vars to timeNow.

later you compare if timeNow - timepH_INTERVAL to an interval. but if both timeNow and timepH_INTERVAL have the same value, how can their difference be anything but zero.

cattledog:
As, @gcr said, the start time timestamps need to be updated to time now when the condition is met. See the “blink without delay” ide example.

Ahhh! Ok, I see what you were saying. Thank you and my apologies, did not realize/understand initially.

gcjr:
regardless – two variables with the same value subtracted from one another will never be > zero

  timeNow = millis ();

timeOD_INTERVAL = timeNow;

if ( (timeNow - timeTemp_INTERVAL) >= Temp_INTERVAL )





not if they are set to timeNow when the condition is met so that the conditional will be met after another INTERVAL period

the way the code is now, each condition will be met each time loop() runs after 4 hours

sounds like you're expecting the code to remain in the pH expired condition for a period of time to wait for the CO2 ADD INTERVAL to expire. the timestamps have the same problem as above (can never be < zero)


if there is a procedure to make a pH measurement that takes time, then the code needs a concept of state. there are several ways to do this.

one, is to have a state variable initialized to zero. the pH_INTERVAL condition will repeatedly be met until the timepH_INTERVAL is reset to timeNow. that allows it to be invoked many times to complete a ph procedure. 

when state is zero, when the pH_INTERVAL initially expires, a pH_PROC timestamp is captured (i.e. set to timeNow), 
- the CO2 valve opened and
- the state advanced to "1"

in state 1, the CO2_ADD time is checked. when it expired, 
- the CO2 valve is closed and 
- the state advanced to "2"

in state 2, the CO2_CHECK interval is checked. when it expires
- measure the pH
- set state to zero
- set pH_INTERVAL to timeNow


i hope you understand the concepts i'm explaining.

I have a basic understanding of the concept of a state machine–in a prior forum post I was led to believe I would not need one. Now, that might have been poor guidance or (and more likely) I didn’t adequately explain my goal.

Question: if I write a state machine for the pH/CO2, I don’t also need one for the temp & ODs since those are simply returning values at given intervals, correct?

Question: if I write a state machine for the pH/CO2, I don't also need one for the temp & ODs since those are simply returning values at given intervals, correct?

no. the other two case don't require and procedure spanning time.

it's hardly a state machine (involving multiple stimuli), just a time driven sequencer. a switch statement will do

Ok, I think I understand the suggestions and think I've made the suggested edits. It compiles, but again, not comfortable enough to know if it runs as desired without actually running on the Arduino.

Does this look as though it will work as desired?

//     **********     ADJUSTABLE TIME POINTS BELOW     **********


#define Temp_INTERVAL             14400000ul   // 4hrs         * * * CHANGE TO FIT REQUIREMENTS * * * 
#define OD_INTERVAL               14400000ul   // 4hrs         * * * CHANGE TO FIT REQUIREMENTS * * *
#define pH_INTERVAL               14400000ul   // 4hrs         * * * CHANGE TO FIT REQUIREMENTS * * * 
#define pH_CO2_ADD_INTERVAL          60000ul   // 1 minute     * * * CHANGE TO FIT REQUIREMENTS * * *
#define pH_CO2_CHECK_INTERVAL       600000ul   // 10 minute    * * * CHANGE TO FIT REQUIREMENTS * * * 

//     **********     END OF ADJUSTABLE TIME POINTS     **********



// Optical Density Sensor
const byte OD_Probe = A14;


// CO2 GateValve
const byte CO2_GateValve = 5;      // FILL IN/CORRECT PIN #     ***************************************

// Temp Probe
const byte Temp_Probe = A2;        // FILL IN/CORRECT PIN #     ***************************************

// pH Probe
const byte pH_Probe = A3;          // FILL IN/CORRECT PIN #     ***************************************
const byte pH_HIGH_THRESHOLD = 7;  // ADJUST TO DESIRED pH      ***************************************


void setup() {

  Serial.begin(9600);

  pinMode (OD_Probe, INPUT);
  pinMode (CO2_GateValve, OUTPUT);
  pinMode (Temp_Probe, INPUT);
  pinMode (pH_Probe, INPUT);


  //static unsigned long timepH_INTERVAL, timepH_CO2_ADD_RunTimer, timepH_CO2_CHECK_RunTimer;
  //static unsigned long timeTemp_INTERVAL, timeTemp_RunTimer;
  unsigned long timeOD_INTERVAL;
  unsigned long timepH_INTERVAL;
  unsigned long timeTemp_INTERVAL;
  unsigned long timeNow;
  timeNow = millis ();

  timeOD_INTERVAL = timeNow;
  timepH_INTERVAL = timeNow;
  timeTemp_INTERVAL = timeNow;

}

void loop()
{
  pH_StateMachine();
}

// pH state names
#define ST_pH_INTERVAL             0
#define ST_pH_CO2_ADD             1
#define ST_pH_CO2_CHECK          2

void pH_StateMachine( void )
{
  int pH_Val;
  static byte  state_pH = ST_pH_INTERVAL;
  static unsigned long timepH_INTERVAL, timepH_CO2_ADD_RunTimer, timepH_CO2_CHECK_RunTimer;
  unsigned long timeNow;
  timeNow = millis();

  switch (state_pH)
  {
    case ST_pH_INTERVAL:
      timepH_CO2_ADD_RunTimer = timeNow;
      state_pH = ST_pH_CO2_ADD;

      break;
    case ST_pH_CO2_ADD:
      if ( (timeNow - timepH_CO2_ADD_RunTimer) >= pH_CO2_ADD_INTERVAL )
      {
        digitalWrite(CO2_GateValve, HIGH);
        timepH_CO2_CHECK_RunTimer = timeNow;
        state_pH = ST_pH_CO2_CHECK;

        break;
      case ST_pH_CO2_CHECK:
        if ( (timeNow - timepH_CO2_CHECK_RunTimer) >= pH_CO2_CHECK_INTERVAL )
        {

        }
      }
  
  // -----------------------------------------------------------------------------------

  // * * * * * Temp INTERVAL * * * * *
  int Temp_Val;
  static unsigned long timeTemp_INTERVAL, timeTemp_RunTimer;

  if ( (timeNow - timeTemp_INTERVAL) >= Temp_INTERVAL )
  {
    timeTemp_INTERVAL = timeNow;

    Temp_Val = analogRead(Temp_Probe);
    Serial.print( "Temp: " );
    Serial.println( Temp_Val );
  }
}

// -----------------------------------------------------------------------------------

// * * * * * OD INTERVAL * * * * *
int OD_Val;
static unsigned long timeOD_INTERVAL, timeOD_RunTimer;

if ( (timeNow - timeOD_INTERVAL) >= OD_INTERVAL )
{
  timeOD_INTERVAL = timeNow;
  OD_Val = analogRead(OD_Probe);
  Serial.print( "OD Reading ");
  Serial.println( OD_Val );
}
}

go back to your original code, with the fixes for timestamps, and add a switch statement inside the if statement for the pH case

sorry, looks like that what you've done. but you reorganized the code so that it harder to see the differences

the switch is only executed when the time is expired

i think your braces aren't lined up. looks like your Temp case is inside the switch statement

what about the body of CO2_CHECK. that's where the pH time interval and state need to be reset

consider the following including optionally pumping CO2 (but i don't understand the check period)

#define TEMP_INTERVAL             14400000ul   // 4hrs      
#define OD_INTERVAL               14400000ul   // 4hrs      
#define PH_INTERVAL               14400000ul   // 4hrs      
#define PH_CO2_ADD_INTERVAL          60000ul   // 1 minute  
#define PH_CO2_CHECK_INTERVAL       600000ul   // 10 minute 


// Optical Density Sensor
const byte OD_Probe          = A14;
const byte Temp_Probe        = A2;
const byte pH_Probe          = A3;
const byte CO2_GateValve     = 5;
const byte pH_HIGH_THRESHOLD = 7;

unsigned long timeOD;
unsigned long timepH;
unsigned long timeTemp;
unsigned long timeCo2Add;
unsigned long timeCo2Chk;

unsigned long timeNow;

enum { ST_INIT, ST_ADD, ST_CHK };
int state;

// -----------------------------------------------------------------------------
void setup () {
    Serial.begin (9600);
    pinMode (OD_Probe,      INPUT);
    pinMode (CO2_GateValve, OUTPUT);
    pinMode (Temp_Probe,    INPUT);
    pinMode (pH_Probe,      INPUT);

    timeNow  = millis ();
    timeOD   = timeNow;
    timepH   = timeNow;
    timeTemp = timeNow;
}

// -----------------------------------------------------------------------------
void loop ()
{
    timeNow  = millis ();

    // * * * * * Temp INTERVAL * * * * *
    if ((timeNow - timeTemp) >= TEMP_INTERVAL)
    {
        timeTemp     = timeNow;
        int temp_Val = analogRead (Temp_Probe);
        Serial.print ("Temp: ");
        Serial.println (temp_Val);
    }

    // * * * * * OD INTERVAL * * * * *
    if ((timeNow - timeOD) >= OD_INTERVAL)
    {
        timeOD     = timeNow;
        int od_Val = analogRead (OD_Probe);
        Serial.print ("OD Reading ");
        Serial.println (od_Val);
    }

    // * * * * * pH INTERVAL * * * * *
    if ((timeNow - timepH) >= PH_INTERVAL)
    {
        int pH_Val;

        switch (state)  {
        case ST_INIT:
            pH_Val = analogRead (pH_Probe);
            Serial.print ("pH Reading ");
            Serial.println (pH_Val);

            if (pH_Val >= pH_HIGH_THRESHOLD)  {
                state = ST_ADD;
                timeCo2Add = timeNow;
                timeCo2Chk = timeNow;

                digitalWrite (CO2_GateValve, LOW);
            }
            else
                timepH = timeNow;
            break;

        case ST_ADD:
            if ((timeNow - timeCo2Add) >= PH_CO2_ADD_INTERVAL)  {
                state = ST_CHK;
                digitalWrite (CO2_GateValve, HIGH);
            }
            break;


        case ST_CHK:
        default:
            if ((timeNow - timeCo2Chk) >= PH_CO2_CHECK_INTERVAL)  {
                state  = ST_INIT;
                timepH = timeNow;
            }
            break;
        }
    }
}

gcjr:
consider the following including optionally pumping CO2 (but i don't understand the check period)

That looks much cleaner & clearer. Thank you.

As for checking the pH shortly after adding it, my concern is as time passes, larger volumes of CO2 will be needed to maintain the pH below the threshold. Thus I want to verify that the amount of CO2 added has reduced the pH enough without waiting for the "PH_INTERVAL" period to elapse again.

That looks much cleaner & clearer. Thank you.

using shorter variable names usually helps and only Capitalizing constants

Thus I want to verify that the amount of CO2 added has reduced the pH enough without waiting for the “PH_INTERVAL” period to elapse again.

i was wondering about that.

does that mean you would want to repeat the pH procedure until the pH level is < threshold? in that case, only reset timepH to timeNow for the case when the pH < threshold

gcjr:
using shorter variable names usually helps and only Capitalizing constants

Clearly, I’m still learning HOW to write code so I’ve been extremely verbose but it has been both helpful and overwhelming.

gcjr:
i was wondering about that.

does that mean you would want to repeat the pH procedure until the pH level is < threshold? in that case, only reset timepH to timeNow for the case when the pH < threshold

This is correct: once the pH > threshold, stay in a loop of add CO2, wait 10min, recheck pH until the pH has dropped below the threshold.

I expect all three pH related intervals to change once I’m able to see some data from in-situ testing but I think the concept of the shorter “CHECK_INTERVAL” is crucial to maintaining the pH–expect it to be an exponential curve, not linear.

This is correct: once the pH > threshold, stay in a loop of add CO2, wait 10min, recheck pH until the pH has dropped below the threshold.

I expect all three pH related intervals to change once I'm able to see some data from in-situ testing but I think the concept of the shorter "CHECK_INTERVAL" is crucial to maintaining the pH--expect it to be an exponential curve, not linear.

so in order to loops, you need to make the change I suggested - delete setting timepH to timeNow in the ST_CHK case. In other words, PH_CO2_ADD_INTERVAL becomes a variable

i also wonder wouldn't the amount of CO2, the time the value is left open depend on how much the pH level > threshold

gcjr:
so in order to loops, you need to make the change I suggested - delete setting timepH to timeNow in the ST_CHK case. In other words, PH_CO2_ADD_INTERVAL becomes a variable

I'll make that change.

gcjr:
i also wonder wouldn't the amount of CO2, the time the value is left open depend on how much the pH level > threshold

This could be the case. My concern about varying the time the valve is open is based on returned pH value is this will be used on chambers of various volumes and thus different amounts of CO2 will be needed to adjust the same pH reading.

With my level of coding knowledge I thought it would be easier to write for constant (but changeable) timing periods and have the code loop until the pH is back below the threshold vs. having the length of the valve open depend on the initial returned pH value.

My concern about varying the time the valve is open is based on returned pH value is this will be used on chambers of various volumes and thus different amounts of CO2 will be needed to adjust the same pH reading.

his will be used on chambers of various volumes and thus different amounts of CO2 will be needed

so then it make sense to add minimal amounts of CO2 and cycle as many times as needed

but i wonder, if immediately after adding CO2 you can periodically measure pH while waiting for it to stabilze and estimate volume by the rate of change. Yes, this introduces a second loop which might be considered in the future after getting the basic code to work.

things like this are what computers are good for.

gcjr:
but i wonder, if immediately after adding CO2 you can periodically measure pH while waiting for it to stabilze and estimate volume by the rate of change. Yes, this introduces a second loop which might be considered in the future after getting the basic code to work.

things like this are what computers are good for.

Increasing the frequency of checking the pH post CO2 add would provide useful data as well as guide any timing changes needed.

Thank you again!