switch to test variables

I’m attempting to cycle through ball valves that are 3 wire, 2 return leads and 1 load. there is a built in cut off in the hardware to cut power once it has cycled open or closed, but I still need to tell my circuit to shut off the connection. I’m using an array of clock times to determine when these action should occur, but i’m unable to test against them in a switch statement. firstly, is it possible to evaluate a variable in a switch statement or will I need to use an if/then. the switch in question is on line 87.

/*
 * This code controls the nutrient solution delivery, allowing delivery of nutrient solutions to 
 * individual root chambers, alternating between individual root chambers and diverting flow
 * back to the nutrient resevuor to stir the solution
 */

  // Ball valve array  variables
  int valveOpen[] = {4,6,8}; // even elements represent open
  int valveClose[] = {5,7,9}; // odd elements represent close
  int valveActionOn[] = {100,250,550,251};
  int valveActionOff[] = {151,300,1,301};
  
  // Pump varriable
  int nutPump = 10;

  //Array Modifiers
  int arrayMod =0;
  
//counter variables **reset all counter variables to 0 for upload to production controller
int timer = 0;
int clockSecond = 0;
int clockMinute = 0;

  String cycleLight;
  int cycleInt;

//variable variables
int valveSwitchOn =0;
int valveSwitchOff  =0;

void setup() {

  //initialize ball valve array as outputs
  for(int i; i<3; i++)
    {
      pinMode(valveOpen[i], OUTPUT);
      pinMode(valveClose[i], OUTPUT);
    }

  //Initialize pump
  pinMode (nutPump, OUTPUT);
  digitalWrite(valveOpen[4], HIGH);
//  delay 100;
  digitalWrite(nutPump, HIGH);  

  //initialize variable variables
  int varValveOpen;
  int varValveClose;
  int varValveAction;

  //time string conversion
  String cycleTime;


  //begin serial transmition
   Serial.begin(9600);
  Serial.println("--Start Serial Monitor SEND_RCVE --:");


}

void loop() {
  // 
//Timer code to count clock time  
  timer++;
  if (timer == 20) {   //timer must be changed and tested for accuracy if additional code is added
    clockSecond++;
    timer = 0;
  }
  if (clockSecond == 60) {
    clockMinute++;
    clockSecond = 0;
  }
  if (clockMinute == 5) {
    clockMinute = 0;
  }

  cycleLight = (String(timer)+String(clockSecond)+String(clockMinute));
  cycleInt = cycleLight.toInt();
  
      Serial.print("clockSecond = " ), Serial.print(clockSecond), Serial.println("");
      Serial.print("ClockMinute = " ), Serial.print(clockMinute), Serial.println("");
      Serial.print("Timer = " ), Serial.print(timer), Serial.println("");
      Serial.print("Cycle number = "), Serial.print(cycleInt), Serial.println("");
valveSwitchOn = valveActionOn[arrayMod];
valveSwitchOff = valveActionOff[arrayMod];
  switch(cycleInt){
    case valveSwitchOn:
    transition();
    break;
    case valveSwitchOff:
    PowerOff();
    break;
  }
  
}

void Transition(){
  digitalWrite(valveOpen[arrayMod], HIGH);
  digitalWrite(valveClose[arrayMod], HIGH;
}

void PowerOff(){
  digitalWrite(valveOpen[arrayMod], LOW);
  digitalWrite(valveClose[arrayMod], LOW);
  arrayMod++;

  switch (arrayMod){
    case 3:
      arrayMod=0;
      break;
  
  }
}

switch/case works with integer ‘equal’ comparison.

Sounds like you might need if() and else if()

yeah, that's what I figured. I was trying to avoid unnecessary line reads, but it looks like it's unavoidable in this instance

What are “line reads”?
What is “unnecessary” about them?

I don’t think this code does anything like what you are expecting...

cycleLight = (String(timer)+String(clockSecond)+String(clockMinute));
cycleInt = cycleLight.toInt();

What about leading zeros?
Consider the values of 0 timer, 12 seconds and 3 minutes (“0123”). How is that cycle value distinguishable from 0 timer, 1 second and 23 minutes (also “0123”). There are many such examples.
Also to yield a meaningful result, even with leading zeros, the most significant digit must come first (minutes before seconds, seconds before fractions).

You have made keeping track of time far more complicated than necessary. It is far easier to store the time in a single variable which represents total whole numbers of some unit (seconds, or in your case 1/20th of a second).

Then whenever a unit of time has elapsed just...

cycleInt = cycleInt + 1;

That avoids all that adding to timer, check for overflow, add to seconds, check for overflow, add to minutes check for overflow, and then trying to convert back to a cycle at the end.

When you require to display the time, calculate the necessary components...

seconds = (cycleInt / 20) % 60;
minutes = (cycleInt / 1200) % 60;
hours = (cycleInt / 72000) % 24;
//etc

No experienced programmer would dream of keeping track of any value (feet+miles, cm+meters, etc) in its individual parts just to satisfy a display requirement.

You probably also want to check out the millis() function, rather than relying on delay, if you want anything like accurate timing.

in general, i don’t understand what the code is intended to do. i would guess to open the valves, one at a time, for a specified period of time each day

comments

  • based on this US Solid video it looks like the ball valve needs to have 12V applied to one lead to open and the other to close the valve. you may want an additional relay to turn off power to all valves after a sufficient time to open/close a valve

  • the variables: varValveOpen, varValveClose varValveAction and cycleTime defined in setup() can not and are not used anywhere else and those lines should be deleted.

  • you can use millis() to keep track of time.

  • the case values in a switch need to be constant integers not variables

  • don’t understand why Transistion() and PowerOff() set both valveOpen and valveClose to the same value

  • would think “arrayMod” would be incremented in loop() and set back to zero with a simple test

  if (3 <= ++led)
        led = 0;
  • i would expect there to be a pin that controls a common relay providing 12V.
  • i would expect an array of pins that control the relay to open or close a valve
  • i would expect an array indicating the time each value needs to be open
  • i would expect to see a test that involving millis() that turns on valve for a specified period of time each day

a better explanation of what the code is intended to do and how to do it would be helpful

a better explanation of how the ball valve work or if the video is not correct would help

PCCBBC, while your code examples have merit, GCJR made similar points as you, without being insulting or degrading. I'm not a professional programmer, that much is evident by my code...

GCJR, those are the valves i'm using. the ball valve itself will kill power to the mechanism, but the relay switching the power on is still energized. 5 seconds after the transition, it shuts down the relay connecting the return. valveOpen and valveClose are using the same modifier because they are connected to the same device. The elements in each array relate to the same device, ie, pins 4 and 5 control the open and close, respectively. the serial.print lines are only for testing and are going to be commented out on final. i'm counting cycles this way to be able to predict what cycle exactly I need to enable disable expantions to the system

my understanding is one lead to the ball valve is ground and the other 2 to motors that turn the valve clockwise (CW) or CCW. It's not clear how you provide power to those 2 leads. Is there a single DPST relay that provide 12V to one lead or the other or are there two separate SPST relays.

midiean:
the ball valve itself will kill power to the mechanism, but the relay switching the power on is still energized. 5 seconds after the transition, it shuts down the relay connecting the return.

this is unclear to me. does the ball valve have some electronics that disconnects the lead to the motor after 5 seconds or simply a limit switch.

does "it" refer to the ball value or your SW?

midiean:
valveOpen and valveClose are using the same modifier because they are connected to the same device. The elements in each array relate to the same device, ie, pins 4 and 5 control the open and close, respectively.

having an open and close pin makes sense if there are 2 relays providing power to each of the leads to a single ball valve. but if a relay is energized by either a LOW/HIGH on the output pin, shouldn't only one output pin /relay be active at a time (i.e. LOW)

if there is a limit switch, it will only disconnect power at the limit position and power on that lead should be disconnected before moving the value in the opposite direction, away from the limit.

it's still unclear what your timing is? is each valve opened, one at a time for a specified period once a day?

as pcbbc probably explained, your "timer" is counting iterations of loop(), not seconds. I've already commented that case values need to be constants not variables. But based on what it looks like tje code is attempting to do, "cycleInt" is some measure of time and
transition() or valveSwitchOff() are invoked when cycleInt is equal to value of valveActionOn/Off .

while a real time clock (RTC) would be more accurate over long periods of time, even millis() can be used to track the number of seconds.

it would be more conventional to test for the time measure (e.g. cycleInt) to exceeds/equals (>=) rather than equal (==) a specified time. An "if" statement would be a conventional way to test that a time period has expired and some action is performed.

   if (cycleInt - cycleLst > timePeriod)  {
           cycleLst = cycleInt;
           timePeriod = ???
           someAction () ???
    }

That processing would include determining a new time period, the next action, advancing arrayMod and handling the wraparound (setting it back to zero).

i would also suggest better names. valveOpen maybe better name valveOpenPin. valveActionOn maybe better names valueOnTime??. the preceding "??" suggests units. I find it better to include msec, sec, min, hour rather that just time. Similarly, cycleInt would be better name time??.

hope you understand and can answer my questions.

midiean:
PCCBBC, while your code examples have merit, GCJR made similar points as you, without being insulting or degrading. I'm not a professional programmer, that much is evident by my code...

I read that 3 times and don't see anything insulting or degrading.

the ball valve has a common load, and independent returns. i'm still working on figuring the switching. I have a mosfet shield that i'm attempting to a common load and switch the return.

I also had my valveClose array out of order. 9 needed to be leading as it is the flow through valve.

the timing code represents a 5 minute cycle and then repeats

in that instance of the use of "it" was referring to cutting power to the relay.

this is the mosfet board i'm using

aarg:
I read that 3 times and don't see anything insulting or degrading.

well.. I did.. I know my experience level, I don't need it trotted out in front of everyone that I don't know all the best ways to code. my code is obviously that of someone who is still learning and to be told that no experience programmer does it that way, is a degrading statement that serves no purpose other than to shame.

Before any more programming discussion, we need some hardware information, for example, the valve motors have to be switched HIGH side, between +V and the OPEN / CLOSE wires, you can't do that with IRF520 N-channel MOSFETs which are NOT logic level (3.3 - 5V) and can only switch very small currents with Arduino's 5V output anyway.
What are the valve motor's voltage and current requirements? Post a link to the valve's datasheet and a wiring diagram of your project.

midiean:
well.. I did.. I know my experience level, I don't need it trotted out in front of everyone that I don't know all the best ways to code. my code is obviously that of someone who is still learning and to be told that no experience programmer does it that way, is a degrading statement that serves no purpose other than to shame.

It wasn’t meant that way I assure you.

I was simply meant to emphasise that although your intuitive method with individual variables works (and kudos to you for getting that far) it is probably not the best method ongoing. Perhaps I should have done more to underline that aspect instead.

Regardless, I do apologise it made you feel that way.

besides the challenge of finding a problem in an OP’s code is understanding what the code is intended to do and properly interpreting the OP’s questions and answers before running out of patience.

sometimes it’s just easier to post some code and see if is what the OP is looking for.

i tested this simulation with LEDs and accelerated timing where 1 minute is 200 and 50 msec. Two relays are simulated but both are the same LED where the open relay is simulated by turning the LED on and the close relay turns the LED off.

// open/close sequence of valves periodically each day

byte valveOpenPin  [] = { 10, 11, 12 };
byte valveClosePin [] = { 10, 11, 12 };
#define N_VALVE    sizeof(valveOpenPin)

#define VALVE_OPEN_ON  LOW
#define VALVE_CLOSE_ON HIGH 

byte pumpPin         = 13;
#define PUMP_ON LOW

#define ONE_MINUTE   200         // should be 60000
#define ONE_DAY     (24 * 60)
#define HourMin(hr, min)    (60*hr + min)

int valveOpenMinutes  [] = { 15,  5, 10 };
int valveCycleMinutes [] = { HourMin(0, 10), HourMin(1, 0), HourMin(20, 0) };
#define N_CYCLE (sizeof(valveCycleMinutes) / sizeof(int))

enum { M_CYCLE, M_SEQUENCE };
int mode = M_CYCLE;

unsigned long msecLst;

unsigned cycleIdx;
unsigned valveIdx;
unsigned minutes;
unsigned alarm;

int  dbg = 1;
char s [40];

// -----------------------------------------------------------------------------
void valveOpen (byte valveIdx) {
    if (2 & dbg)  {
        sprintf (s, "   %s: %d valve pin", __func__, valveOpenPin [valveIdx]);
        Serial.println (s);
    }
    digitalWrite (valveOpenPin [valveIdx], VALVE_OPEN_ON);
}

// ---------------------------------------------------------
void valveClose (byte valve) {
    if (2 & dbg)  {
        sprintf (s, "   %s: %d valve", __func__, valveIdx);
        Serial.println (s);
    }
    digitalWrite (valveClosePin [valveIdx], VALVE_CLOSE_ON);
}

// ---------------------------------------------------------
void pumpOnOff (byte on) {
    if (2 & dbg)  {
        sprintf (s, "  %s: %d on", __func__, on);
        Serial.println (s);
    }
    digitalWrite (pumpPin, on);
}

// ---------------------------------------------------------
void loop() {
    unsigned long msec  = millis();

    if (msec - msecLst > ONE_MINUTE)  {
        msecLst = msec;
        if (++minutes >= ONE_DAY)
            minutes = 0;

        if (4 & dbg)  {
            sprintf (s, "%s: %4d minute, %4d alarm", __func__, minutes, alarm);
            Serial.println (s);
        }
    }

    if (minutes == alarm)  {    // handle when alarm is next day
        if (M_CYCLE == mode)  {
            if (8 & dbg)  {
                sprintf (s, "  %s: cycle", __func__);
                Serial.println (s);
            }

            mode     = M_SEQUENCE;
            valveIdx = 0;
            alarm    = minutes + valveOpenMinutes [valveIdx];

            valveOpen (valveIdx);
            pumpOnOff (PUMP_ON);
        }

        // valve sequence
        else  {
            if (8 & dbg)  {
                sprintf (s, "  %s: sequence", __func__);
                Serial.println (s);
            }

            valveClose (valveIdx);
            if (N_VALVE > ++valveIdx)  {
                alarm    = minutes + valveOpenMinutes [valveIdx];
                valveOpen (valveIdx);
            }
            else  {                     // next cycle
                pumpOnOff (! PUMP_ON);
                if (N_CYCLE <= ++cycleIdx)
                    cycleIdx = 0;

                mode   = M_CYCLE;
                alarm  = valveCycleMinutes [cycleIdx];
            }
        }

        sprintf (s, " %s: %4d minute, %4d alarm, %d mode", 
            __func__, minutes, alarm, mode);
        Serial.println (s);
    }
}

// -----------------------------------------------------------------------------
void setup() {
    Serial.begin (115200);

    for (unsigned i = 0; i < N_VALVE; i++)
    {
        digitalWrite (valveClosePin [i], ! VALVE_CLOSE_ON);
        pinMode      (valveClosePin[i], OUTPUT);

        digitalWrite (valveOpenPin [i], ! VALVE_OPEN_ON);
        pinMode      (valveOpenPin[i],  OUTPUT);
    }

    digitalWrite (pumpPin, ! PUMP_ON);
    pinMode      (pumpPin, OUTPUT);

    if (16 & dbg)  {
        for (unsigned i = 0; i < N_CYCLE; i++)  {
            sprintf (s, " %s: cycle %d, minutes %6d",
                __func__, i, valveCycleMinutes [i]);
            Serial.println (s);
        }
    }

    minutes  = 0;
    valveIdx = 0;
    cycleIdx = 0;
    alarm    = valveCycleMinutes [cycleIdx];
}

so, lets see if this code makes any more sense. GCJR, I understand some of your code, but you’re using some reserved statements that I’m not familiar with or have a firm grasp of their use yet.

/*
 * This code controls ball valves to divert the flow of a solution between multiple exit points before diverting back to the resevour
 *  system operates on a 5 minute cycle, with valves transitioning simultaniously
 *  between each valve to maintain pressure between transitions. the last valve will then divert flow back to the tank for the
 *  remainder of the cycle before repeating.
 * 
 * All display lines are for testing purposes. 
 */

  // Ball valve array  variables
  int valveOpen[] = {4,6,8}; // even elements represent open
  int valveClose[] = {9,5,7}; // odd elements represent close
  int valveActionOn[] = {100,250,550,251}; 
  int valveActionOff[] = {1050,10300,1101,11301};
  
  // Pump varriable
  int nutPump = 10;

  //Array Modifiers
  int arrayMod =0;
  
//counter variables **reset all counter variables to 0 for upload to production controller
int timer = 0;
int clockSecond = 0;
int clockMinute = 0;

  String cycleLight;
  int cycleInt;

//variable variables
int valveSwitchOn =0;
int valveSwitchOff  =0;

void setup() {

  //initialize ball valve array as outputs
  for(int i; i<3; i++)
    {
      pinMode(valveOpen[i], OUTPUT);
      pinMode(valveClose[i], OUTPUT);
    }

  //Initialize pump
  pinMode (nutPump, OUTPUT);
  digitalWrite(valveOpen[4], HIGH);
//  delay 600;
  digitalWrite(nutPump, HIGH);  

  //begin serial transmition
   Serial.begin(9600);
  Serial.println("--Start Serial Monitor SEND_RCVE --:");


}

void loop() {
  // 
//Timer code to count clock time  
  timer++;
  if (timer == 20) {   //timer must be changed and tested for accuracy if additional code is added
    clockSecond++;
    timer = 0;
  }
  if (clockSecond == 60) {
    clockMinute++;
    clockSecond = 0;
  }
  if (clockMinute == 5) {
    clockMinute = 0;
  }

/*
 * convert to the integer values to a string, merge the string and convert to integer. these time cycle values are used to then
 * determin whether or not to call the action functions Transision() and PowerOff(). 
 */
  cycleLight = (String(timer)+String(clockSecond)+String(clockMinute)); 
  cycleInt = cycleLight.toInt();
  
      Serial.print("clockSecond = " ), Serial.print(clockSecond), Serial.println("");
      Serial.print("ClockMinute = " ), Serial.print(clockMinute), Serial.println("");
      Serial.print("Timer = " ), Serial.print(timer), Serial.println("");
      Serial.print("Cycle number = "), Serial.print(cycleInt), Serial.println("");


  if (cycleInt == valveActionOn[arrayMod]) {
    Transition();
  }
  if (cycleInt == valveActionOff[arrayMod]) {
    PowerOff();
  }
}

void Transition(){ 
  //transition function to open and close ball valves
  digitalWrite(valveOpen[arrayMod], HIGH);
  digitalWrite(valveClose[arrayMod], HIGH);

  Serial.print("--------open valve "), Serial.print(valveOpen[arrayMod]), Serial.print(" is on ---------"), Serial.println("");
  Serial.print("--------close valve "), Serial.print(valveClose[arrayMod]), Serial.print(" is on--------"), Serial.println("");
}

void PowerOff(){
  //cut power to the mosfets and auto increment the arrayMod variable
  digitalWrite(valveOpen[arrayMod], LOW);
  digitalWrite(valveClose[arrayMod], LOW);
   Serial.print("--------valve "), Serial.print(valveOpen[arrayMod]), Serial.print(" is off--------"), Serial.println("");
   Serial.print("--------valve "), Serial.print(valveClose[arrayMod]), Serial.print(" is off--------"), Serial.println("");
  arrayMod++;

if (arrayMod == 4){
  arrayMod = 0;  
  }
}

I appreciate that PCBBC, thank you. I’m still learning all of this and it’s not easy for this 40 year old man to translate my objectives into computer code. I imagine much of what i’m coding is very simplistic.

what reserved statements? it would help to know what you don’t understand

when i compile your code i get

C:\stuff\SW\Arduino\_Others\Download\Tst\Tst.ino:29:13: warning: 'i' is used uninitialized in this function [-Wuninitialized]

     for(int i; i<3; i++)

i think i finally understand your valveOpen/Close . they are indexed by time or event index, not valve index. in transition, the code is opening one value while closing another using the same index, “arrayMod”.

this also suggests that the code is constantly cycling one valve off and another on.

but arrayMod is allowed to increment from 0 thru 3. there are 4 values in valveActionOn/Off but only 3 values in valveOpen/Close .

while it looks like valveActionOn specifies relative times to switch valves, i don’t understand the purpose of valveActionOff;

i still don’t understand how you track time. it looks like you increment seconds and reset timer when it reaches 20 and similar for seconds and minutes when seconds reaches 60.

cycleInt is converted from the concatenation of timer, minutes and seconds. Since there are at least 3 characters in the string, the sequence of cycleInt values begins with 0, 100, 200, … when seconds become 1, the sequence is 1, 101, 201, …

in general, this is very confusing.

most professional organizations have style guidelines for how code is written. Since developers need to review, debug and maintain code they may not have written, conventions are followed so that they can somewhat glance at code, see code organize and structured in conventional ways so that it is easy to understand and read.

being “clever” or “novel” is not appreciated, especially for mundane code. The days of using novel coding techniques (e.g. self modifying code) to save memory are long gone and readability has greater priority. (early DEC minicomputers had 4k of memory. the arduino UNO has 32k)

if you would like continued help from me, i would like to read your description of what the code is doing and see you use more conventional coding techniques.

i'm not familiar with unsigned, sprint, #define, sizeof() and millis() for how they are used and when they should be used. basically, your code is too advanced for me to understand how its working without comments explaining a lot of it.

the structure of your code is confusing as well, I haven't seen the setup() and loop() functions after all the called functions. New coding standard or just your style? I know it doesn't matter what order they come in, I just found it disorienting when trying to map out the code. didn't help that I'm not that advanced in the language as of yet, lol.

I'm sorry for the confusion about the valve action* arrays, i'm a little ahead of myself in the programming and there should only be 3 elements in those arrays right now. there will be a 4th element to the valve* arrays when the components come in.

if millis() is a better way of accounting for time, I would definitely like to understand how it works. I know the system is supposed to operate in cycles of milliseconds, but as the code increases in size, doesn't this create the possibility that the processing of a particular line of code will be missed because the time doesn't match the condition of enabling/disabling of a particular pin or function?

for example, I know this is the valve open and close functions, but I don't understand what is going on with sprintf.

void valveOpen (byte valveIdx) {
    if (2 & dbg)  {
        sprintf (s, "   %s: %d valve pin", __func__, valveOpenPin [valveIdx]);
        Serial.println (s);
    }
    digitalWrite (valveOpenPin [valveIdx], VALVE_OPEN_ON);
}

// ---------------------------------------------------------
void valveClose (byte valve) {
    if (2 & dbg)  {
        sprintf (s, "   %s: %d valve", __func__, valveIdx);
        Serial.println (s);
    }
    digitalWrite (valveClosePin [valveIdx], VALVE_CLOSE_ON);
}

I appreciate your patience, the last time I did any real code learning was back in the 286 days when memory was an issue. coding standards have changed quite a bit since I was in a programming class

midiean:
the structure of your code is confusing as well, I haven't seen the setup() and loop() functions after all the called functions. New coding standard or just your style? I know it doesn't matter what order they come in, I just found it disorienting when trying to map out the code. didn't help that I'm not that advanced in the language as of yet, lol.

Functions must normally have function prototypes ("function declarations") to allow them to be called prior to their implementation in the code sequence. Those contain no code. The Arduino IDE produces them automatically but invisibly so that people can get into the bad habit of not writing them. :wink: It's supposed to be easier. It sort of goes along with the idea of having everything in one file - in the rest of the C++ world, the prototypes are usually kept separately in a .h file while the implementation is kept in a .cpp file.

i still don't understand what your code is intended to do.

it appears to open one valve at a time for a sequence of 3 valves. why not open all the valves?

Does it do this continuously throughout the day so that it is repeatedly closing one value and opening another or does it sequence through all the valves at specific times during the day and shutting a pump off in between?

i'm not familiar with unsigned, sprint, #define, sizeof() and millis() for how they are used and when they should be used.

these aren't exotic or advanced. google is at your fingertips

they aren't exotic, no, but the explanations i'm finding aren't helping much to figure out why it is you chose to use these types.

the code is meant to cycle every 5 minutes. this is a continuous operation for a months long project. this code is intended to alternate diverting water between 2 points and then back to the reservoir. opening all the valves at once would just divert back to the reservoir as its the path of least resistance.

midiean:
the code is meant to cycle every 5 minutes. this is a continuous operation for a months long project. this code is intended to alternate diverting water between 2 points and then back to the reservoir. opening all the valves at once would just divert back to the reservoir as its the path of least resistance.

much clearer.