next topic »

[Reply #30 on:]

Hi Krodel.
Been playing around with my sketch. I ran 'verify' on it (whatever that does) and everything was ok , but in practice it stopped after digitalWrite (9,HIGH). Obviously something is wrong, but to me the flow of it seems ok. What are your comments please?

#

Code:

unsigned long startTime = millis();  // current value of millis is assigned


// here I want to enter the variables of a,b,c,etc so that they will be entered automatically later

const unsigned long a = 30;           // camera trigger duration
const unsigned long b = 55;           //  1st valve open duration
const unsigned long c = 100;          // time from beginning to 2nd valve opening
const unsigned long d = 55;           //  2nd valve open duration
const unsigned long e = 110;          // time from beginning to 3rd valve opening
const unsigned long f = 55;           //  3rd valve open duration
const unsigned long g = 200;          // time from beginning to 4th valve opening
const unsigned long h = 55;           //  4th valve open duration

const unsigned long x = 10000;        // 10 sec recycling period

void setup() {               
  // initialize the digital pins as an output.


  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT); 
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);

}


void loop() {
  // triggering camera (immediately)
  if (startTime < millis())         
  {
    digitalWrite (9,HIGH);
  }
  if (startTime + a  < millis())  // if  current millis is  greater than startTime  + a then proceed
  {
    digitalWrite (9,LOW);
  }

  //triggering first valve (immediately)
  if (startTime < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + b <millis())
  {
    digitalWrite (6,LOW);
  }

  //triggering second valve
  if (startTime +c < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + d <millis())
  {
    digitalWrite (6,LOW);
  }

  //triggering third valve     
  if (startTime + e < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + f <millis())
  {
    digitalWrite (6,LOW);
  }

  //triggering fourth valve
  if (startTime + g < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + h <millis())
  {
    digitalWrite (6,LOW);
  } 

  // I now want to wait for x seconds and start all over again.
 // so I need to check that various things have all happend
  // Can this be done by checking that all pins 5 to 9 are low
  // and then adding x to the startTime value
  if ( 5, LOW && 6,LOW && 7,LOW && 8,LOW && 9,LOW)
  {
    startTime += x;

  }
}

[Reply #31 on:]

Please edit your post, select the code, and put it between [code] ... [/code] tags.

You can do that by hitting the # button above the posting area.

[Reply #32 on:]

Code:

if ( 5, LOW && 6,LOW && 7,LOW && 8,LOW && 9,LOW)

No.
Just, no.

[Reply #33 on:]

Code:

const unsigned long a = 30;           // camera trigger duration
const unsigned long b = 55;           //  1st valve open duration
const unsigned long c = 100;          // time from beginning to 2nd valve opening

Here's an idea! Meaningful data names:

Code:

const unsigned long cameraTriggerDuration = 30;       
const unsigned long firstValveOpenDuration = 55;
const unsigned long timeTillSecondValveOpening = 100;

And so on. Then the whole sketch becomes self-documenting, largely.

[Reply #34 on:]

This is indeed wrong code:

Code:

if ( 5, LOW && 6,LOW && 7,LOW && 8,LOW && 9,LOW)

This is how to read the digital pins:

Code:

if (digitalRead(5) == LOW && digitalRead(6) == LOW && digitalRead(7) == LOW && digitalRead(8) == LOW && digitalRead(9) == LOW)

http://arduino.cc/en/Reference/digitalRead

You are making it harder for yourself, if you don't use the serial monitor. I can't do without it.
In setup(), you initialize with the baudrate: Serial.begin(9600);
In loop(), you can print messages and variables to the serial monitor: Serial.println("hello");
The Serial.println() function will delay your code, so use it only during testing.

You need variables for the valves, to make it more readable. Like so:

Code:

const int pinTrigger = 9;
const int pinValve1 = 6;
...
digitalWrite (pinTrigger, HIGH);

You have copied the code, so you use pin '6' for all valves.

I have tested your code in my Arduino Uno, and placed Serial.println() messages to show what it does. I'm sorry to say, but your code is not working.
At start, all pins are low, and millis() has not increased yet, so the "startTime += x;" is executed every time.
After x is added to startTime, no code is executed since no 'if' condition is true, so startTime is again increased.

That is why I needed to keep track of the state for every valve.
A simple solution is to check also for the same value of millis():

Code:

if (startTime <= millis())

Can you add a variable for the time to open the first valve.
I would even add a variable for the trigger.
That would make your code more readable.

Code:

const unsigned long start_trigger = 0;      // time from beginning to trigger
const unsigned long start_1 = 2;      // time from beginning to 1nd valve opening

I am fond of meaningfull names. If you change the a,b,c...h variables into start_1, duration_1 and so on, you will make it easier for yourself if you have to change something later.

[Reply #35 on:]

Rami, those are quote tags like the word balloon button makes. Just to the left of the word balloon button is the # button that generates code tags. Or you can modify your post and replaces the two words quote with the word code.

Code:

if ( 5, LOW && 6,LOW && 7,LOW && 8,LOW && 9,LOW)

Big mystery to me is how people come up with syntax like that, and they do!

[Reply #36 on:]

You quoted your previous post.

Can you follow my advice in my previous post. If you follow that, and upload it, it is easier for everyone to comment.

[Reply #37 on:]

Hi Krodel. Sorry about that. I have inplemented some of the suggestions so far.. viz meaningful titles. Got this far

Code:

unsigned long startTime = millis();  // current value of millis is assigned


// here I want to enter the variables  so that they will be entered automatically later

const unsigned long time_to_trigger_camera = 0;             // time to trigger camera from beginning
const unsigned long camera_trigger_duration = 30;           // camera trigger duration
const unsigned long time_to_open_first_valve = 0;           // time from beginning to 1st valve opening
const unsigned long first_valve_open_duration = 55;         //  1st valve open duration
const unsigned long time_to_open_second_valve = 100;        // time from beginning to 2nd valve opening
const unsigned long second_valve_open_duration = 55;        //  2nd valven open duration
const unsigned long time_to_open_third_valve = 110;         // time from beginning to 3rd valve opening
const unsigned long third_valve_open_duration = 55;         //  3rd valve open duration
const unsigned long time_to_open_fourth_valve = 200;        // time from beginning to 4th valve opening
const unsigned long fourth_valve_open_duration = 55;        //  4th valve open duration

const unsigned long recycling_period = 10000;        // 10 sec recycling period

void setup() {                
  // initialize the digital pins as an output.


  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);  
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  Serial.begin(9600);

}


void loop() {
  // triggering camera (immediately)
  if (startTime + time_to_trigger_camera< millis())        
  {
    digitalWrite (9,HIGH);
  }
  if (startTime +  camera_trigger_duration < millis())  // if  current millis is  greater than startTime  + a then proceed
  {
    digitalWrite (9,LOW);
  }

  //triggering first valve (immediately)
  if (startTime + time_to_open_first_valve< millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + first_valve_open_duration <millis())
  {
    digitalWrite (6,LOW);
  }

  //triggering second valve
  if (startTime + time_to_open_second_valve < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + second_valve_open_duration <millis())
  {
    digitalWrite (6,LOW);
  }

  //triggering third valve    
  if (startTime + time_to_open_third_valve < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + third_valve_open_duration <millis())
  {
    digitalWrite (6,LOW);
  }

  //triggering fourth valve
  if (startTime + time_to_open_fourth_valve < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + fourth_valve_open_duration <millis())
  {
    digitalWrite (6,LOW);
  }  

  // I now want to wait for recycling_period seconds and start all over again.
 // so I need to check that various things have all happend
  // Can this be done by checking that all pins 5 to 9 are low
  // and then adding recycling_period to the startTime value
  if (digitalRead(5) == LOW && digitalRead(6) == LOW && digitalRead(7) == LOW && digitalRead(8) == LOW && digitalRead(9) == LOW);
  {
    startTime += recycling_period;

  }
}

[Reply #38 on:]

I have inplemented some of the suggestions so far.. viz meaningful titles.

Sure you did.

Code:

  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT); 
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);

Now, we know exactly what was designated as an output.

Code:

  if (startTime + time_to_trigger_camera< millis())         

Subtraction involving unsigned longs is guaranteed to work. Addition is not. If now minus then is greater than in interval is what you want here.

Once you do something, you want to update the relevant "i did this then" variable. I don't see you doing that anywhere.

Got this far

Which describes point A, but says nothing about where point B is, or what issues remain in getting from point A to point B.

[Reply #39 on:]

Code:

if (startTime + time_to_open_first_valve< millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + first_valve_open_duration <millis())
  {
    digitalWrite (6,LOW);
  }

  //triggering second valve
  if (startTime + time_to_open_second_valve < millis())
  {
    digitalWrite (6,HIGH);
  }
  if (startTime + second_valve_open_duration <millis())
  {
    digitalWrite (6,LOW);
  }

Better get started on "6" now.

[Reply #40 on:]

Hi,
I have spent all evening reading and testing. I know my hardware works ok because it works ok with my  delay sketch. I learnt how to use the serial monitor and used it  to check the values when  the program was running .The values for the unsigned long variables that are increased by the recycling period are increasing at the correct rate. The camera is releasing at the correct time, but the valves are not opening. Why ? Have I missed something? Guess it's time for bed!

Code:

/* sequence of events
   camera triggered, and valves triggerd at a time according  to' time to open valve'
   for a period of time 'duration'
   at the end of the sequence there is a predetermined pause and all starts again.*/

unsigned long startTime = millis();  // current value of millis is assigned

unsigned long time_to_trigger_camera = 0;             // time to trigger camera from beginning
unsigned long camera_trigger_duration = 30;           // camera trigger duration
unsigned long time_to_open_first_valve = 0;           // time from beginning to 1st valve opening
unsigned long first_valve_open_duration = 55;         //  1st valve open duration
unsigned long time_to_open_second_valve = 100;        // time from beginning to 2nd valve opening
unsigned long second_valve_open_duration = 55;        //  2nd valve open duration
unsigned long time_to_open_third_valve = 110;         // time from beginning to 3rd valve opening
unsigned long third_valve_open_duration = 55;         //  3rd valve open duration
unsigned long time_to_open_fourth_valve = 200;        // time from beginning to 4th valve opening
unsigned long fourth_valve_open_duration = 55;        //  4th valve open duration

unsigned long recycling_period = 10000;        // 10 sec recycling period

void setup() {                
  // initialize the digital pins as an output.


  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);  
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  Serial.begin(9600);  // setup serial

}


void loop() {
  
  
  
  
  // triggering camera (immediately)
  if ( (millis()-startTime) >= time_to_trigger_camera )        
  {
    digitalWrite (9,HIGH);
    time_to_trigger_camera += recycling_period;
  }
  if ( (millis()-startTime) >=  camera_trigger_duration )  
  {
    digitalWrite (9,LOW);
    
  }

 // triggering first valve (immediately)
  if ( (millis() - startTime) >= time_to_open_first_valve)
  {
    digitalWrite (6,HIGH);
    //time_to_open_first_valve += recycling_period;
  }
  if ( (millis() - startTime) >= first_valve_open_duration )
  {
    digitalWrite (6,LOW);
  }

  //triggering second valve
  if ( (millis() - startTime) >= time_to_open_second_valve)
  {
    digitalWrite (7,HIGH);
    time_to_open_second_valve += recycling_period;
  }
  if ( (millis() - startTime) >= second_valve_open_duration)
  {
    digitalWrite (7,LOW);
  }

 //triggering third valve    
  if ( (millis() - startTime) >= time_to_open_third_valve)
  {
    digitalWrite (8,HIGH);
    time_to_open_third_valve += recycling_period;
  }
  if ( (millis() - startTime) >= third_valve_open_duration)
  {
    digitalWrite (8,LOW);
  }

  //triggering fourth valve
  if ( (millis() - startTime) >= time_to_open_fourth_valve )
  {
    digitalWrite (5,HIGH);
    time_to_open_fourth_valve += recycling_period;
  }
  if ( (millis() - startTime) >= fourth_valve_open_duration)
  {
    digitalWrite (5,LOW);  
  }  
}

[Reply #41 on:]

You have improved it, but there are still some things to do.
I have tested the sketch, and the first valve is opening and closing (just once).
You could make the delays a few seconds. Perhaps the valve doesn't react to 55ms.

You increase the time for the next period, that's okay, but you should do that also for the other times.

The duration is not the moment in time, but the time after the valve is opened.
So you have to make a choice: define the duration as the duration or define it as the moment in milliseconds.
Suppose you define it as the duration, then this :

Code:

// triggering first valve (immediately)
if ( (millis() - startTime) >= time_to_open_first_valve)
{
  digitalWrite (6,HIGH);
  time_to_open_first_valve += recycling_period;
}
if ( (millis() - startTime) >= first_valve_open_duration )
{
  digitalWrite (6,LOW);
}

should be this:

Code:

if ( (millis() - startTime) >= time_to_open_first_valve)
{
  digitalWrite (6,HIGH);
}
if ( (millis() - startTime) >= (time_to_open_first_valve + first_valve_open_duration) )
{
  digitalWrite (6,LOW);
  // Now that the first valve has been opened and closed,
  // the new time can be set for the next period.
  time_to_open_first_valve += recycling_period;
}

Do you know that the digitalWrite() is called many times ?
To set an output, calling digitalWrite() just once is nicer.
In your code the conditions are valid for some time, and digitWrite() is called during every loop.

I also would call millis() just once per loop.
In the loop function you could do this: unsigned long time_millis = millis() - starttime;
And use the variable time_millis in the code.

[Reply #42 on:]

Hi Krodel.
It works
Yes I got your point. I drew a timeline, and can see where I was going wrong.  eg if millis()-startTime <= camera_trigger_duration is always going to be greater, so the pin goes low immediately
The reason it worked with the camera was that it only needs to be triggered to work.
The valves are very specialist ones that have a response time of 10ms
The format as it is makes it easy for me to cut and paste the sketch if I want to trigger a valve for a second or third time during a loop
If you are interested you can see my images of my equipment and images taken using software using 'delay' on
http://www.flickr.com/photos/ramigrafx/

Do you know that the digitalWrite() is called many times ?
To set an output, calling digitalWrite() just once is nicer.
In your code the conditions are valid for some time, and digitWrite() is called during every loop.

I also would call millis() just once per loop.
In the loop function you could do this: unsigned long time_millis = millis() - starttime;
And use the variable time_millis in the code.

Refinements next!
Here is the working sketch so far

Code:

unsigned long startTime = millis();  // current value of millis is assigned


// here I want to enter the variables  so that they will be entered automatically later

unsigned long time_to_trigger_camera = 0;             // time to trigger camera from beginning
unsigned long camera_trigger_duration = 30;           // camera trigger duration
unsigned long time_to_open_first_valve = 0;           // time from beginning to 1st valve opening
unsigned long first_valve_open_duration = 55;         //  1st valve open duration
unsigned long time_to_open_second_valve = 100;        // time from beginning to 2nd valve opening
unsigned long second_valve_open_duration = 55;        //  2nd valven open duration
unsigned long time_to_open_third_valve = 110;         // time from beginning to 3rd valve opening
unsigned long third_valve_open_duration = 55;         //  3rd valve open duration
unsigned long time_to_open_fourth_valve = 200;        // time from beginning to 4th valve opening
unsigned long fourth_valve_open_duration = 55;        //  4th valve open duration

unsigned long recycling_period = 10000;        // 10 sec recycling period

void setup() {                
  // initialize the digital pins as an output.


  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);  
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  Serial.begin(9600);

}


void loop() {
  // triggering camera (immediately)
  if (millis()-startTime >= time_to_trigger_camera)        
  {
    digitalWrite (9,HIGH);  
  }
  if (millis()-startTime >=  (time_to_trigger_camera + camera_trigger_duration)  )
  {
    digitalWrite (9,LOW);
    time_to_trigger_camera += recycling_period;
  }

  //triggering first valve (immediately)
  if (millis() - startTime >= time_to_open_first_valve)
  {
    digitalWrite (6,HIGH);
  }
  if (millis() - startTime >= (time_to_open_first_valve + first_valve_open_duration))
  {
    digitalWrite (6,LOW);
    time_to_open_first_valve += recycling_period;
  }

  //triggering second valve
   if (millis() - startTime >= time_to_open_second_valve )
   {
   digitalWrite (7,HIGH);
      }
   if (millis() - startTime >= time_to_open_second_valve + second_valve_open_duration)
   {
   digitalWrite (7,LOW);
    time_to_open_second_valve += recycling_period;
   }
  
   //triggering third valve    
   if (millis() - startTime >= time_to_open_third_valve)
   {
   digitalWrite (8,HIGH);
      }
   if (millis() - startTime >= time_to_open_third_valve + third_valve_open_duration)
   {
   digitalWrite (8,LOW);
   time_to_open_third_valve += recycling_period;
   }
  
   //triggering fourth valve
   if (millis() - startTime >= time_to_open_fourth_valve )
   {
   digitalWrite (5,HIGH);
     }
   if (millis() - startTime >= time_to_open_fourth_valve + fourth_valve_open_duration)
   {
   digitalWrite (5,LOW);
   time_to_open_fourth_valve += recycling_period;
   }  
}

[Reply #43 on:]

That's very interesting !

[Reply #44 on:]

Thanks Krodel, I now have a  working sketch which does everything that I want at the moment. I have written it so that I can trigger any of 6 valves at any time either once or as many times as I want, at any time. I can alter the values easily whist the system is running

I also would call millis() just once per loop.
In the loop function you could do this: unsigned long time_millis = millis() - starttime;
And use the variable time_millis in the code.

And done this, so looks a lot neater.

Looking back at my old sketch using delay ( which was the first that I wrote) I have made considerable progress in writting sketches, even though it was a bit traumatic on the way.