Code order of execution - Arduino newbie

Hi All Im trying to get a pressure switch to open a solonoid and alow it achieve a set pressure before triggering a switch on pin5 pulling it low which then releases the pressure on the solonoid for a pre set time then adding a value to the counter before repeating.

once the counter has achieved 30 cycles it should delay indefinately

the problem I am having is how the code is executing it seems to be skipping the loop I have tried to put in place and jumps straight to delaying indefinitely after the switch pulls low for the first time.

See linked flow chart explaining what I am trying to achieve;

The serial monitor log:

Void setup complete
Loop started
Switch pressed beginning 30 cycles
pin high pause
cycles completed:
1
cycles complete!!
1
delay indefinetely

and the code here:

// Pressure cycling code change variable 'solonoidlowtimer' to adjust time delay between solonoid active and inactive
// Try to avoid solonoid on time exeeding the time duration of solonoid off due to overheating

int solonoidpin = 9;   // Solonoid is connected to digital pin 9
int switchPin = 5;   // switch connected to digital pin 5
int startButton = 6; // button to start cycle code
int counter = 0;
int solonoidlowtimer = 1000;  //Time delay for the solonoid being in relaxed position


void setup()
{
  Serial.begin(9600);
  delay (200);
   pinMode(solonoidpin, OUTPUT);   // sets the Solonoid to be an output
    pinMode(startButton, INPUT);   // sets the switchPin to be an input
  pinMode(switchPin, INPUT);   // sets the switchPin to be an input
    pinMode(startButton, INPUT);   // sets the switchPin to be an input
  digitalWrite(switchPin, HIGH);   // sets the default (unpressed) state of switchPin to Low
   
  Serial.println("Void setup complete");
}


void loop()   // run over and over again

{
  Serial.println("Loop started");
  counter =0;   
   
    
    if (counter < 30)         
     { for (int ii = 0; ii < 30; ii++);
      { Serial.println("Switch pressed beginning 30 cycles");
          delay (20);

         digitalWrite(solonoidpin, HIGH);

       Serial.println("pin high pause");

   

    while  (digitalRead(switchPin)== HIGH); 
    
      { 
        digitalRead(switchPin);
        delay(20);
            }
          

      counter++;

    digitalWrite(solonoidpin, LOW);
      delay(solonoidlowtimer);   //off period to return pressure to zero


    Serial.println("cycles completed:");
    Serial.println(counter);

    }
      }


     else;

     {


if (counter > 29);    //  Check to see if cycling is still required

{
  Serial.println("cycles complete!!");
  Serial.println(counter);
  Serial.println("delay indefinetely");
  solonoidpin = 1;
delay (1000000000000000); 
}
}
}

Im 99% sure its to do with how I stack my '}'s I’ve tried numerous ways of trying to fix it an it just ends up breaking in a new and unsuspecting way haha… now i’m seeking a degree of insight from someone with abilities above my own… (not a challange haha!)

Many thanks in advance!

Ok, so I have just solved it… after spending a few hours looking at it 2mins after I post I’ve resolved it with a couple of simple changes

// Pressure cycling code change variable 'solonoidlowtimer' to adjust time delay between solonoid active and inactive
// Try to avoid solonoid on time exeeding the time duration of solonoid off due to overheating

int solonoidpin = 9;   // Solonoid is connected to digital pin 9
int switchPin = 5;   // switch connected to digital pin 5
int startButton = 6; // button to start cycle code
int counter = 0;
int solonoidlowtimer = 1000;  //Time delay for the solonoid being in relaxed position


void setup()
{
  Serial.begin(9600);
  delay (200);
   pinMode(solonoidpin, OUTPUT);   // sets the Solonoid to be an output
    pinMode(startButton, INPUT);   // sets the switchPin to be an input
  pinMode(switchPin, INPUT);   // sets the switchPin to be an input
    pinMode(startButton, INPUT);   // sets the switchPin to be an input
  digitalWrite(switchPin, HIGH);   // sets the default (unpressed) state of switchPin to Low
    counter =0;
   
  Serial.println("Void setup complete");
}


void loop()   // run over and over again

{
  Serial.println("Loop started");

   
    
    if (counter < 30)         
     { for (int ii = 0; ii < 30; ii++);
      { Serial.println("Switch pressed beginning 1000 cycles");
          delay (20);

         digitalWrite(solonoidpin, HIGH);

       Serial.println("pin high pause");

   

    while  (digitalRead(switchPin)== HIGH); 
    
      { 
        digitalRead(switchPin);
        delay(20);
            }
          

      counter++;

    digitalWrite(solonoidpin, LOW);
      delay(solonoidlowtimer);   //off period to return pressure to zero


    Serial.println("cycles completed:");
    Serial.println(counter);

    }
      }


     else;

if (counter > 29)    //  Check to see if cycling is still required

{
  Serial.println("cycles complete!!");
  Serial.println(counter);
  Serial.println("delay indefinetely");
  solonoidpin = 1;
delay (1000000000000000); 
}
}

Any constructive criticism of what I have written would still be appreciated :slight_smile:

First, try and format the code better so it’s easier to read. The IDE can help you with auto format (press Ctrl+T). But I did to do some clean up myself. A lot of extra new lines. Also, after a {, go to a new line.

// Pressure cycling code change variable 'solonoidlowtimer' to adjust time delay between solonoid active and inactive
// Try to avoid solonoid on time exeeding the time duration of solonoid off due to overheating

int solonoidpin = 9;   // Solonoid is connected to digital pin 9
int switchPin = 5;   // switch connected to digital pin 5
int startButton = 6; // button to start cycle code
int counter = 0;
int solonoidlowtimer = 1000;  //Time delay for the solonoid being in relaxed position

void setup()
{
  Serial.begin(9600);
  delay (200);
  pinMode(solonoidpin, OUTPUT);   // sets the Solonoid to be an output
  pinMode(startButton, INPUT);   // sets the switchPin to be an input
  pinMode(switchPin, INPUT);   // sets the switchPin to be an input
  pinMode(startButton, INPUT);   // sets the switchPin to be an input
  digitalWrite(switchPin, HIGH);   // sets the default (unpressed) state of switchPin to Low

  Serial.println("Void setup complete");
}

void loop()   // run over and over again
{
  Serial.println("Loop started");
  counter = 0;

  if (counter < 30)
  { 
    for (int ii = 0; ii < 30; ii++);
    { 
      Serial.println("Switch pressed beginning 30 cycles");
      delay (20);

      digitalWrite(solonoidpin, HIGH);

      Serial.println("pin high pause");

      while  (digitalRead(switchPin) == HIGH);
      {
        digitalRead(switchPin);
        delay(20);
      }

      counter++;

      digitalWrite(solonoidpin, LOW);
      delay(solonoidlowtimer);   //off period to return pressure to zero

      Serial.println("cycles completed:");
      Serial.println(counter);
    }
  }
  else;
  {
    if (counter > 29);    //  Check to see if cycling is still required
    {
      Serial.println("cycles complete!!");
      Serial.println(counter);
      Serial.println("delay indefinetely");
      solonoidpin = 1;
      delay (1000000000000000);
    }
  }
}

And after that I spotted the same error twice:

 else;
  {
    if (counter > 29);    //  Check to see if cycling is still required

OEPS! The statements ends after a ; :wink:

Extra tips:

pinMode(switchPin, INPUT);   // sets the switchPin to be an input

You do know you don’t need an external resistor if you do

pinMode(switchPin, INPUT_PULLUP);   // sets the switchPin to be an input

Some examples of bad comments

 pinMode(startButton, INPUT);   // sets the switchPin to be an input
  pinMode(startButton, INPUT);   // sets the switchPin to be an input
  digitalWrite(switchPin, HIGH);   // sets the default (unpressed) state of switchPin to Low

Untrue comments are worse than no comment at all. To avoid this, don’t repeat yourself in the comments

 pinMode(solonoidpin, OUTPUT);   // sets the Solonoid to be an output
  //[...]
  digitalWrite(switchPin, HIGH);   // sets the default (unpressed) state of switchPin to Low

If you want to have the pin HIGH as default, switch the two statements. First a digitalWrite() and a pinMode() after that. Now the pin is LOW for a very small time. Might not hurt but can be avoided.

int solonoidpin = 9;   // Solonoid is connected to digital pin 9
int solonoidlowtimer = 1000;  //Time delay for the solonoid being in relaxed position

Make it easier for yourself by using the same name scheme throughout. Aka solenoidPin and solenoidLowTime (it’s not a timer).

Also, ever needed to have pin -12678? An byte is a better suited variable type. And make it const because you never change it. And I like to start the variable name with a capital to remember me it’s a const.

const byte SolenoidPin = 9;

And your code talks about a start button (which seems smart) but you never use it.

Also, the flow diagram talks about 1000 cycles.

while  (digitalRead(switchPin) == HIGH);
      {
        digitalRead(switchPin);
        delay(20);
      }

The digitalRead() inside does nothing…

Also, this is a good time to talk about blocking code. It’s better to let the loop() loop (hence the name). So instead of locking the code in a loop full of waiting, just let the loop() do the looping and simply do nothing if you don’t want to :smiley: But then again, you need to get rid of delay() as well. For now this isn’t a real problem because you can do what you want. But note it’s nearly impossible to add different stuff to the code if you want to do more.

    else
    {
      if (counter > 29);    //  Check to see if cycling is still required
      {

No need to check again. If it’s not < 30 then you know it’s above 29;)

delay (1000000000000000);

For sure it’s a long time but it’s NOT indefinitely…

while(true);

is. But looking code like that is usually seen as ugly. And that’s where a nice start button would be handy.

Thank you very much septillion, alot of the minor typos/ comments come from trouble shooting and changing little bits obviously missing the good practice to double check the comments beside

the same goes for the 1000 cycles - I dropped it down to 30 so I could check the delay function worked correctly, and removal of the start button was simply to make everything a touch less complex to trouble shoot.. its back in now :)

I'll give it a revamp incorporating your comments

Thank you for the time you spared on this!