Problem with a loop and solenoids

Hi guys,

I currently have a function that, when being called:

1/ moves 2 servos to 90 degrees
2/ toggle some solenoids (HIGH/LOW) at a specific millisec speed

here it is:

void toggle20 () {
   servoe.write(90, 0, true);
   servos.write(90, 0);
   while ( true )
  {
    static unsigned long previousTime = 0;
    
    currentTime = millis();
    if(currentTime - previousTime >= 20)
    {
      digitalWrite (10, !digitalRead(10));
      digitalWrite (23, !digitalRead(23));
      digitalWrite (9, !digitalRead(9));
      digitalWrite (25, !digitalRead(25));
      digitalWrite (26, !digitalRead(26));
      digitalWrite (27, !digitalRead(27));
      digitalWrite (28, !digitalRead(28));
      digitalWrite (29, !digitalRead(29));
      if(currentTime < previousTime) // check for rollover
        previousTime = 0;
        
      previousTime += 20; // increment previous time by delay factor "20"
    }
    
    if (Serial.available() > 0)
    {
      incomingByte = Serial.read();
      if (incomingByte == lowNote)
        break; // if incomingByte is equal to chan+27
    }
  }
 digitalWrite (10, LOW);
 digitalWrite (23, LOW);
 digitalWrite (9, LOW);
 digitalWrite (25, LOW);
 digitalWrite (26, LOW); 
 digitalWrite (27, LOW);
 digitalWrite (28, LOW);
 digitalWrite (29, LOW); 
}

What I’m trying to achieve now is to toggle each of those solenoids at different speed within the same loop. Is that even remotely possible?

Here’s what I’ve done but it only toggle the first one the way I want to.

void random () {
 servoe.write(90, 0, true);
 servos.write(90, 0);
   while ( true )
  {
    static unsigned long previousTime = 0;
    
    currentTime = millis();
    if(currentTime - previousTime >= 30)
    {
      digitalWrite (10, !digitalRead(10));
      digitalWrite (23, !digitalRead(23)); 
      if(currentTime < previousTime) // check for rollover
        previousTime = 0;
        
      previousTime +=30; // increment previous time by delay factor "30"
    }
    
    currentTime = millis();
    if(currentTime - previousTime >= 100)
    {
      digitalWrite (9, !digitalRead(9));
      digitalWrite (25, !digitalRead(25)); 
      if(currentTime < previousTime) // check for rollover
        previousTime = 0;
        
      previousTime += 100; // increment previous time by delay factor "100"
       }
    
    currentTime = millis();
    if(currentTime - previousTime >= 35)
    {
      digitalWrite (26, !digitalRead(26));
      digitalWrite (27, !digitalRead(27)); 
      if(currentTime < previousTime) // check for rollover
        previousTime = 0;
        
      previousTime += 35; // increment previous time by delay factor "35"
             }
    
    currentTime = millis();
    if(currentTime - previousTime >= 65)
    {
      digitalWrite (28, !digitalRead(28));
      digitalWrite (29, !digitalRead(29)); 
      if(currentTime < previousTime) // check for rollover
        previousTime = 0;
        
      previousTime += 65; // increment previous time by delay factor "65"
      
    }
    
    if (Serial.available() > 0)
    {
      incomingByte = Serial.read();
      if (incomingByte == lowNote)
        break; // if incomingByte is equal to 12
    }
  }
 
}

Any help would be gladly appreciated :slight_smile:

What I’m trying to achieve now is to toggle each of those solenoids at different speed within the same loop. Is that even remotely possible?

Have a look at the technique used in the program below which flashes 2 LEDs at independant rates

const byte ledPins[] = {12, 13};
const byte NUMBER_OF_LEDS = sizeof(ledPins);
unsigned long startTimes[NUMBER_OF_LEDS];
unsigned long periods[] = {500, 700};

void setup()
{
  Serial.begin(115200);
  for (int p = 0; p < NUMBER_OF_LEDS; p++)
  {
    pinMode(ledPins[p], OUTPUT);
  }
}

void loop()
{
  for (int p = 0; p < NUMBER_OF_LEDS; p++)
  {
    unsigned long currentTime = millis();
    if (currentTime - startTimes[p] >= periods[p])
    {
      digitalWrite(ledPins[p], !digitalRead(ledPins[p]));
      startTimes[p] = currentTime;
    }
  }
}

Thanks UKHeliBob for your answer,

I get what it does and it seems xactly what I’m trying to achieve,

Here my modified piece of code within my function, I think I’m not far from making it work but I have an error: 932:5: error: expected unqualified-id before ‘if’

const byte solenoidspin[] = {10, 9, 7};
const byte NUMBER_OF_SOLENOIDS = sizeof(solenoidspin);
unsigned long startTimes[NUMBER_OF_SOLENOIDS];
unsigned long periods[] = {100, 50, 250};

// the loop function runs over and over again forever

void setup(){
 Serial.begin(38400);
   for (int p = 0; p < NUMBER_OF_SOLENOIDS; p++)
  {
    pinMode(solenoidspin[p], OUTPUT);
  }

and here is the function:

void random () {
 servoe.write(90, 0, true);
 servos.write(90, 0);
 while (true)
      for (int p = 0; p < NUMBER_OF_SOLENOIDS; p++)
  {
    unsigned long currentTime = millis();
    if (currentTime - startTimes[p] >= periods[p])
    {
      digitalWrite(solenoidspin[p], !digitalRead(solenoidspin[p]));
      startTimes[p] = currentTime;
    }
  }
    }
    
    if (Serial.available() > 0)
    {
      incomingByte = Serial.read();
      if (incomingByte == lowNote)
        break; // if incomingByte is equal to 12
    }

What am I doing wrong?

False alert, I've changed some stuff.

It does work!

Thanks a LOT for your help!

Good that you got it working.

If you post your working code then it may be possible to tidy it up some more and/or make it more efficient.

Sure,

#include <VarSpeedServo.h> 


byte type = 0;

byte incomingByte;
byte note;
byte velocity;
int chan = 1;  //specify what MIDI channel we're listing to
byte boucle=0;   // pour boucle

int action=2; //0 =note off ; 1=note on ; 2= null
int currentTime;
int previousTime;
const int lowNote = 60; //what's the first note?  36 is C1 in Ableton

VarSpeedServo servoe;   // create servo object to control a servo that inflate 1
VarSpeedServo servos;   // create servo object to control a servo that deflate 1


const byte solenoidspin[] = {10, 9, 7, 23, 25, 27, 28, 29};
const byte NUMBER_OF_SOLENOIDS = sizeof(solenoidspin);
unsigned long startTimes[NUMBER_OF_SOLENOIDS];
unsigned long periods[] = {100, 50, 250, 75, 50, 300, 100, 75};

// the loop function runs over and over again forever

void setup(){
 Serial.begin(38400);
   for (int p = 0; p < NUMBER_OF_SOLENOIDS; p++)
  {
    pinMode(solenoidspin[p], OUTPUT);
  }

 
//setup our outputs

  pinMode(10, OUTPUT);
  digitalWrite (10, LOW);
    pinMode(23, OUTPUT);
  digitalWrite (23, LOW);
  
    pinMode(9, OUTPUT);
  digitalWrite (9, LOW);
    pinMode(25, OUTPUT);
  digitalWrite (25, LOW);
  
    pinMode(7, OUTPUT);
  digitalWrite (7, LOW);
    pinMode(27, OUTPUT);
  digitalWrite (27, LOW);
  
    pinMode(28, OUTPUT);
  digitalWrite (28, LOW);
    pinMode(29, OUTPUT);
  digitalWrite (29, LOW);
 
  servoe.attach(30);   // Set I1 servo to digital pin 10
  servos.attach(31);  //  Set D1 servo to digital pin 23
}
void random () {
 servoe.write(90, 0, true);
 servos.write(90, 0);
 while (true)
 {
      for (int p = 0; p < NUMBER_OF_SOLENOIDS; p++)
  {
    unsigned long currentTime = millis();
    if (currentTime - startTimes[p] >= periods[p])
    {
      digitalWrite(solenoidspin[p], !digitalRead(solenoidspin[p]));
      startTimes[p] = currentTime;
    }
  }
    
    if (Serial.available() > 0)
    {
      incomingByte = Serial.read();
      if (incomingByte == lowNote+21)
        break; // if incomingByte is equal to 22
    }
 }
 digitalWrite (10, LOW);
 digitalWrite (9, LOW);
 digitalWrite (7, LOW);
 digitalWrite (23, LOW);
 digitalWrite (25, LOW);
 digitalWrite (27, LOW);
 digitalWrite (28, LOW);
 digitalWrite (29, LOW);
}

// then comes the void loop part that allow me to call that function with a specific midi note

I’m sure it could be more efficient but it does work. I guess that it’s as good as it can get with my low developer skills.

but it does work.

That’s the important part. Now to improve it.

The first thing that I notice is that you are doing the same thing with a number of output pins. This often means that it would be more efficient and neater to use an array to hold the pin numbers and a for loop to do the actions. For example, you already have the pin numbers in a global array so how about doing this in setup ?

for (int p = 0; p < NUMBER_OF_SOLENOIDS; p++)
{
  pinMode(solenoidspin[p], OUTPUT);
  digitalWrite (solenoidspin[p], LOW);
}

or better still write a function to set them all LOW and call it when needed, which you do twice in the program.

I get what you’re saying and it does make perfect sense.

The piece of code I’ve added is disturbing the rest of my other functions. I suspect it’s related to what you’re saying, I’m using the same outpins names for multiple functions which must cause issues.

Here it is:

const byte solenoidspin[] = {10, 9, 7, 23, 25, 27, 28, 29};
const byte NUMBER_OF_SOLENOIDS = sizeof(solenoidspin);
unsigned long startTimes[NUMBER_OF_SOLENOIDS];
unsigned long periods[] = {100, 50, 250, 75, 50, 300, 100, 75};

// the loop function runs over and over again forever

void setup(){
 Serial.begin(38400);
   for (int p = 0; p < NUMBER_OF_SOLENOIDS; p++)
  {
    pinMode(solenoidspin[p], OUTPUT);
  }

 
//setup our outputs // I suspect this part below is not needed anymore

  pinMode(10, OUTPUT);
  digitalWrite (10, LOW);
    pinMode(23, OUTPUT);
  digitalWrite (23, LOW);
  
    pinMode(9, OUTPUT);
  digitalWrite (9, LOW);
    pinMode(25, OUTPUT);
  digitalWrite (25, LOW);
  
    pinMode(7, OUTPUT);
  digitalWrite (7, LOW);
    pinMode(27, OUTPUT);
  digitalWrite (27, LOW);
  
    pinMode(28, OUTPUT);
  digitalWrite (28, LOW);
    pinMode(29, OUTPUT);
  digitalWrite (29, LOW);

Am I allowed to do this instead?

// setups our outputs

const byte solenoidspin[] = {10, 9, 7, 23, 25, 27, 28, 29};
const byte NUMBER_OF_SOLENOIDS = sizeof(solenoidspin);
unsigned long startTimes[NUMBER_OF_SOLENOIDS];
unsigned long periods[] = {100, 50, 250, 75, 50, 300, 100, 75};

// the loop function runs over and over again forever

void setup(){
 Serial.begin(38400);
   for (int p = 0; p < NUMBER_OF_SOLENOIDS; p++)
  {
    pinMode(solenoidspin[p], OUTPUT);
  }

and delete the rest of the outpins KNOWING the fact that I have some functions written like this below:

void leftfast () {
 servoe.write(90, 0, true);
 servos.write(90, 0);
 digitalWrite (9, HIGH);
 digitalWrite (7, HIGH);
 digitalWrite (23, HIGH);
 digitalWrite (29, HIGH);
 delay (1000);
 digitalWrite (9, LOW);
 digitalWrite (7, LOW);
 digitalWrite (23, LOW);
 digitalWrite (29, LOW); 
}

This function doesn’t function well anymore and I suspect it is related to the new array pins I’ve set up at the begining. Therefore I have to write this function differently. The issue is that this function wasn’t functioning as a toggle. It was just, when being called, moving 2 servos at 90degrees, and turning on 4 specific solenoids, waiting 1 sec and then turning them LOW.
I’m not sure how I can change it to work with the array outputs instead of my previous method.

I also know that delays are to be avoided at any case but my concern is that for my specific application I need to make sure that those solenoids does turned OFF after 1 second precisely. The loop doesn’t seems to be able to do that is it?

About your one function to turn them all off. I agree, however, how would it work when I have specific functions that only have some solenoids to be turned off but not all of them? (as see in the function right above which only have 4 solenoids to be turned OFF)

There is no problem in using the pin numbers from the array and explicit pin numbers in the same program. As to turning four pins on or off you could write a function to do that which takes a list of 4 pin numbers and a HIGH/LOW parameter telling the function what to do. Something like

void setFourOutputs(byte output0, byte output1, byte output2, byte output3, byte state)
{
  digitalWrite(output0, state);
  digitalWrite(output1, state);
  digitalWrite(output2, state);
  digitalWrite(output3, state);
}

and call it like thus

setFourOutputs(9, 7, 23, 29, HIGH);

then later

setFourOutputs(9, 7, 23, 29, LOW);

thanks for your useful answer, I'll try it tonight at home.

i have an issue I don't seem to be able to resolve by my own.

here are the functions being called in the void loop using MIDI notes:

void loop () {
  
  servoe.write(90);    // move to 90 degrees
  servos.write(90);    // move to 90 degrees
 
    if(Serial.available() > 0) {
    // read the incoming byte:
    incomingByte = Serial.read();       

   // wait for as status-byte, channel 1, note on or off
    if (incomingByte== 143+chan){ // note on message starting starting
      action=1;
    }
    
    if (incomingByte== 127+chan){ // note off message starting
      action=0;
 
    }
    
    if ( (action==0)/*&&(note==0)*/ ){ // if we received a "note off", we wait for which note (databyte)
      note=incomingByte;
    } 
    
    if ( (action==1)/*&&(note==0)*/ ){ // if we received a "note on", we wait for the note (databyte)
      note=incomingByte;
      moveServo(note);
  
    }
    }
 
}

void moveServo(byte note){    //  call a function based on the note
    
switch (note){
  case lowNote:
  vibe20 ();
  break;
}
switch (note){
  case lowNote+1:
  vibe25 ();
  break;
}
switch (note){
  case lowNote+2:
  vibe30 ();
  break;
}
switch (note){
  case lowNote+3:
  vibe45 ();
  break;
}
switch (note){
  case lowNote+4:
  vibe40 ();
  break;
}
switch (note){
  case lowNote+5: //if it's not 60
  vibe45 ();
  break;
}
switch (note){
  case lowNote+13:
  backslow ();
  break;
}
switch (note){
  case lowNote+14:
  forthslow ();
  break;
}
switch (note){
  case lowNote+15:
  rightslow ();
  break;
}
switch (note){
  case lowNote+16:
  leftslow ();
  break;
}
switch (note){
  case lowNote+17:
  backfast ();
  break;
}
switch (note){
  case lowNote+18:
  forthfast ();
  break;
}
switch (note){
  case lowNote+19:
  rightfast ();
  break;
}
switch (note){
  case lowNote+20:
  leftfast ();
  break;
}
switch (note){
  case lowNote+21:
  randomvibe ();
  break;
}
switch (note){
  case lowNote+22:
  upslow ();
  break;
}
switch (note){
  case lowNote+23:
  upfast ();
  break;
}

When I'm calling function right fast, it should do this:

void rightfast() {   // that's one of my basic function without any loop. 
 servoe.write(90, 0);
 servos.write(90, 0, true);
 digitalWrite (22, HIGH);
 digitalWrite (28, HIGH);
 delay (200);
 digitalWrite (22, LOW);
 digitalWrite (28, LOW);
}

However, it just start toggling the 8 solenoids without stopping I have to reset the arduino to make it stop. I've tried to pinpoint the issue and it is located in the void loop section.

I'm solving the issue when i'm doing this:

void loop () {
  
  servoe.write(90);    // move to 90 degrees
  servos.write(90);    // move to 90 degrees
 
    if(Serial.available() > 0) {
    // read the incoming byte:
    incomingByte = Serial.read();       

   // wait for as status-byte, channel 1, note on or off
    if (incomingByte== 143+chan){ // note on message starting starting
      action=1;
    }
    
    if (incomingByte== 127+chan){ // note off message starting
      action=0;
 
    }
    
    if ( (action==0)/*&&(note==0)*/ ){ // if we received a "note off", we wait for which note (databyte)
      note=incomingByte;
    } 
    
    if ( (action==1)/*&&(note==0)*/ ){ // if we received a "note on", we wait for the note (databyte)
      note=incomingByte;
      moveServo(note);
  
    }
    }
 
}

void moveServo(byte note){    //  call a function based on the note

// by removing all of the cases above, it does work
switch (note){
  case lowNote+19:
  rightfast ();
  break;
}
switch (note){
  case lowNote+20:
  leftfast ();
  break;
}
switch (note){
  case lowNote+21:
  randomvibe ();
  break;
}
switch (note){
  case lowNote+22:
  upslow ();
  break;
}
switch (note){
  case lowNote+23:
  upfast ();
  break;
}

It works. in other words if the case with the function being called is the first in line it does work, otherwise it gets crazy. Why? How can can I get all my different cases in line and call the one I want with the right midi note without the case being first in line?

FYI it used to have only ONE switch note at the beginning and then only cases and breaks. I've tried to add switch for every functions but this doesn't solve the issue.

You have misunderstood how to use switch/case. There should be one switch statement and multiple cases, each with code to be executed if the case is true like this

void moveServo(byte note)   //  call a function based on the note
{
  switch (note)
  {
    case lowNote:
      vibe20 ();
      break;

    case lowNote+1:
      vibe25 ();
      break;

    case lowNote+2:
      vibe30 ();
      break;

    case lowNote+3:
      vibe45 ();
      break;

      // etc, etc
  }
}

you're 100% right and that's why I've added this in my previous comment

FYI it used to have only ONE switch note at the beginning and then only cases and breaks. I've tried to add switch for every functions but this doesn't solve the issue.

I think I'm starting to pinpoint the issue by removing and testing function by function.

Talking to you helps me find issues :slight_smile: