Using millis()

Hello,

I am using a valve in a project and I want to open the valve initially for sometime (around 1~5 sec) and then close it. Later, when a certain condition is fulfilled, I am opening the valve again.

Though it opens initially, it does not open for the time I mentioned in the code. If i set it open for 5 sec, it opens for less that 3 sec and if I set it for 1 sec, it opens for more than 1 sec. I am not sure what am doing wrong. Also, the peak counter is supposed to count counts 1, open, count 2 and opens but sometimes, it just counts 2,3,4,5. It misses counting 1 and am guessing this is due to the timing of the initially opened valve. Can you please help me figure out the mistake?

The valve speed mentioned in the data sheet is : Speed from closed to opened: ~100ms

(deleted)

This code in setup(), which gets executed exactly once

  initialTime = millis();
  solstate = true;

  if (solstate == true && millis() - initialTime >= ontime)

is not going to work. How is solstate ever going to be something other than true? How is millis()-initialTime ever going to be anything other than 0 or 1?

You have a lot of code and I don't immediately see which line opens the valve and which line closes it. Perhaps you can point them out.

...R

I want to open the valve initially for sometime (around 1~5 sec) and then close it.

vaj4088 has pointed out that Setup does not loop and repeat, so you can not use the millis() - initialTime construct as millis() will only have one value.

If this is really a setup issue, I don't see the problem with using delay(). What are you going to block for 1~5 seconds that you want to do? If there is something, then the initial valve opening needs to be in loop() with a "do it once" flag and not in setup().

If this is really a setup issue, I don't see the problem with using delay(). What are you going to block for 1~5 seconds that you want to do? If there is something, then the initial valve opening needs to be in loop() with a "do it once" flag and not in setup().

I dont want to use delay as I going to be collecting the pressure values continuously and I dont want to interrupt that.

How do I put the valve open in the loop() and get it open only once for either 1 /5 sec?

dpoornima: How do I put the valve open in the loop() and get it open only once for either 1 /5 sec?

As @cattledog said, use a a "do it once" flag. Or, an "I've already done it" flag.

How do I put the valve open in the loop() and get it open only once for either 1 /5 sec?

unsigned long startTime = 0;
const unsigned long interval = 1000;  // adjust for time required
boolean timerRunning = false;
boolean doneItOnce = false;

void setup()
{
}


void loop()
{
  //if (condition to activate timer)
  if(doneItOnce == false) //or if(!doneItOnce)
  {
    startTime = millis(); // Begin timing sequence
    timerRunning = true;
    doneItOnce = true;
    //open valve
    // do stuff here when timing sequence starts
  }

  unsigned long currentTime = millis();
  if (timerRunning==true && currentTime - startTime >= interval) // End timing sequence
  {
    timerRunning = false;
     //close valve
    // do stuff here when time is up
  }
}

Why have you all these repeats

        if (peakcounter == 1)
          {
            digitalWrite(4, HIGH);  //valve open
            startTime = millis();
            solstate = true;
            peakcounter = 0;
            currentmax++;
          }

          if (peakcounter == 2) {
            digitalWrite(4, HIGH);  //valve open
            startTime = millis();
            solstate = true;
            peakcounter = 0;
            currentmax++;
          }

          if (peakcounter == 3) {
            digitalWrite(4, HIGH);  //valve open
            startTime = millis();
            solstate = true;
            peakcounter = 0;
            currentmax++;
          }

          if (peakcounter == 4) {
            digitalWrite(4, HIGH);  //valve open
            startTime = millis();
            solstate = true;
            peakcounter = 0;
            currentmax++;
          }
          if (peakcounter == 5) {
            digitalWrite(4, HIGH);  //valve open
            startTime = millis();
            solstate = true;
            peakcounter = 0;
            currentmax++;
          }

wouldn't this be equivalent

       if (peakcounter >= 1) {
            digitalWrite(4, HIGH);  //valve open
            startTime = millis();
            solstate = true;
            peakcounter = 0;
            currentmax++;
          }

...R

startTime, initialTime, currentTime, time... Only currentTime conveys any information. initial time? What, exactly, happened then? start time? What started? And don't get me started on the generic time name.

    senval[1] = getPressure();//pressure_mmhg;

    senval[1] = (senval[1] + getPressure()) / 2;

Why do you need to do this in two steps?

senval[1] = getPressure() / 2; // The comment adds NO value
      // [b]If the following conditions are fulfilled, valve opens[/b]

I can not possibly imagine how bold tags ANY value to the comment.

          digitalWrite(4, HIGH);  //valve open
          startTime = millis();

So, startTime is the time the valve opened. Of course. I can't imagine how I missed that.

if (solstate == true && millis() - startTime >= interval)

interval is another generic, meaningless, name. Can't you use a name that means something?

And, exactly what does solstate mean?

  Serial.print(time);
  Serial.print(',');
  Serial.println(getPressure());
  Serial.print(',');

Sometime, way back when, we noted what time it was. Now, we'll tell you. Why doesn't that pass the sniff test?

I followed your suggestion but the valve doesnt open for 1 sec. I tried it with 2 sec too, but it simply opens fro less than 0.2 sec and continues with the rest of the program.

Check your brackets and the hierarchy of the if statements. This piece of code which closes the valve does not lie within the conditions that run the cal routine and it is executed first pass through the loop.

if (solstate == true && millis() - startTime >= interval)
    {
      digitalWrite(4, LOW);  //valve close
      solstate = false;
      peakcounter = 0;
    }

I haven’t thought this through in detail but this change may fix your issue

if (Calaverage == true && solstate == true && millis() - startTime >= interval)

Another possibility might be

if (timerRunning == false && solstate == true && millis() - startTime >= interval)

Though the code works perfectly for initial open time of 1 sec,but if I increase the initial open time of the valve to 2 sec, it opens for 2 sec and stays off. It doesnt open for 1,2,3..peaks at all.

You are going to need Serial print statements to debug all the variables in this section which fails when the initial valve is open for 2 seconds. What variable is not correct for the code to enter a condition? Put print statements before each if conditional.

You may not have a logic issue, but a physical issue, if Pdelta, can never get back above average if the value has been open for two seconds. Still, you'll find that if you print out the values of all the variables.

if (Calaverage == true && solstate == false && Pdelta >= average)
{
  ++peakcounter;
}

if (peakcounter > currentmax )
{

  if (peakcounter >= 1)
  {
    digitalWrite(4, HIGH);
    startTime = millis();
    solstate = true;
    peakcounter = 0;
    currentmax++;
  }

}


if (Calaverage == true  && solstate == true && millis() - startTime >= interval)
{
  digitalWrite(4, LOW);
  solstate = false;
  peakcounter = 0;

}

You have the value of “average” declared twice with different scopes.

 float average;//at the start of loop()
float average = total / 10;//within an if (Pdelta>=50) condition

The second value is being printed when you see the printing of “AVG:”
It is not being printed when its value is <50.

When Pdelta is less than 50, the value of average can be anything, because it is being reinitialized without specific assignment every pass through loop. But, it’s likely being set to 0, so any value of Pdelta will be greater than average. This is what you currently see.

For now, try this.

 static float average = 0;//at the start of loop()
 average = total / 10;//within an if (Pdelta>=50) condition

If the program works correctly, I think that you want to revise the program so that after the average is only calculated from the first 10 readings over 50, you stop making useless calculations of average which do not change in value.

You said earlier, that when the initial valve time was one second, the program worked correctly, but when it was two, it did not. Give this bug, with the average value I can not see how that would be true.

Regarding that initial valve opening time, do you want it open for the period it takes to determine the average, or is the opening time independent of the collection of the first 10 values where Pdelta is over 50? If the initial valve period is related to the calibrartion of average, then the code needs to make that happen, and not rely on some time period between 1 and 2 seconds.

I tried to do it with 1 sec again and it did not work for 1 sec too. It just opens initially and then remains closed. Although the peak counter and current max gets updated correctly, the valve doesnt go on. It even stopped working for 1 sec. Why would it work once with the same program and not work for the second time?

 if (peakcounter >= 1)
          {
            digitalWrite(4, HIGH);
            startTime = millis();
            solstate = true;
            peakcounter = 0;
            currentmax++;
            Serial.print(" \tpeakcounter:\t  ");
            Serial.println(peakcounter);
            Serial.print(" \tcurrentmax: \t  ");
            Serial.println(currentmax);
          }

If you are seeing printouts in this section and increasing values of currentmax then there must be a hardware problem if digitalWrite(4,HIGH) is not opening the valve.

I am just trying to print the output of the valve.

Where? I don't see any printouts of "Valve Opened" or "Valve Closed" in the sections of your code where the valve pin 4 is written HIGH or LOW.

The same program either opens initially and stays off or opens initially and after sometime, suddenly switches on and then stays off.

Should I change the components or software? or is there a bug in the code which is doing this. Also, when I previously made corrections of float that you mentioned, it worked correctly and the exact same program doesnt work now. What might be the issue?

I'm not quite sure what your expectations are? Without knowing the values of average, Pdelta, and the count numbers and whether or not the valve is supposed to be open or closed, you don't know if you are looking at correct logic and unexpected physical inputs, or a programming bug. In particular, the expected behaviour of opening and closing of the valve after an increasing number of cycles has never been clear to me.

Your debugging environment is not good. First, I would remove all the SD code, and only work with Serial output.

Second, I would restructure the serial output for debugging and plotting purposes and not have it related to how you would collect data when all is operating well.

It's also unclear to me how the 300 ms period that the valve is open relates to the loop time, sensor reading time, and sd write time. Perhaps this piece of tracking code is missing the transition

 int solenoidvalue = digitalRead(4);

In my opinion, the code is probably working as intended (since it did work correctly) and you are looking at unexpected inputs or conditions.

EDIT: The pressure value printed in the output code you posted is not the same as used in the logic portion of the program. It is a new and different reading. You need to track and debug with variables used in the main body of the code.

Serial.println(getPressure());

An incorrect brace, and faulty sensor was the problem. Thanks to your advice on debugging method , I could figure it out.

Good job sorting this out.

It's also unclear to me how the 300 ms period that the valve is open relates to the loop time, sensor reading time, and sd write time. Perhaps this piece of tracking code is missing the transition.

Could you please explain what you mean by this?

I was searching for reasons why you were possibly missing valve open or closed states.

the valve doesnt open for the interval specified. It opens for 0.1 sec and closes and starts counting for the next cycle.

Does the valve stay open for the complete 10 reading calibration cycle? How long does that take? Are you saying that once the calibration period is complete the valve does not open for the 300 ms when it supposed to open at the increasing peak counts?

This piece of code looks like it will close the valve before the calibration cycle is complete as all these conditions are likely true after the valve opens to collect the 10 readings.

if (solstate == true && millis() - startTime >= interval)
{
  digitalWrite(4, LOW);
  solstate = false;
  peakcounter = 0;

}

Please post complete code.

Yes after the calibration is done, the valve still opens after counting peaks but it does not open for the specified time. It just opens for 100ms or less and then closes and starts counting for the peaks again.

if (calculatedaverage == true)
  {
    
    digitalWrite(4, LOW);
    solstate = false;
  }

This piece of code is being executed every pass through loop() after the calibration routine. It is what is clsing the valve.

I would eliminate that code, and turn the valve off when calculated average is first set = true.

 if (Pdelta >= 50 && calculatedaverage == false)
      {
        float value = Pdelta;
        readings[index] = value;
        index ++;
        if (index >= numReadings)
        {
          index = 10;

          float total = 0;

          for (byte i = 0; i < 10 ; i++)
          {
            total += readings[i];

          }
          float average = total / 10;
          calculatedaverage = true;
          digitalWrite(4, LOW);
          solstate = false;
         

          Serial.print(" \tAVG: \t  ");
          Serial.println(average);
        }
      }