Why am I so stupid? (Or why is this simple thing perplexing me)

Projects Programming
1

OK, I've tried to put this together several different ways and I just can't get it to act the way I think it should.
I'm just trying to turn on a relay. When it's on, blink an LED and after it's been on for a period of time, turn it off.

So I have to assume that I am thinking the the If statement or the way I am writing my statements is just plain wrong. Also, I think my code is junk, I feel like I'm going around the barn twice to get in the door. There has to be a shorter way to do it.

in a nutshell. wait a while, turn the pump on for a short time, blink while the pump is on, turn off the pump and don't blink. It should be so easy!!!! So, please, where is my error? It just sits there with 13 on, doing nothing. I can get it to turn on and off if I just use blink without delay, it turns off and on just fine.

/* Program to turn on misting pump for plant propogation.
Turn on the pump relay for a few seconds every 8 to 10 minutes.
*/
const int ledPin = 13; // the number of the LED pin
const int pumpRelay = 10; // Number of the PumpRun Pin

// Variables will change:
int ledState = LOW; // ledState used to set the LED
int pumpState = LOW;

unsigned long blinkMillis = 0; // last time we blinked
unsigned long blinkInterval = 500; //Interval between blinks
unsigned long previousBlinkMillis = 0;
unsigned long previousPumpMillis =0;
unsigned long previousMillis =0;
unsigned long currentMillis =0;
unsigned long pumpRunMillis =0;
unsigned long pumpRunTime = 100; // interval to run the pump (10 sec currently)
unsigned long betweenPumpRun = 6000; // interval between pump running (10 min)

void setup() {
pinMode(ledPin, OUTPUT);
pinMode(pumpRelay, OUTPUT);
digitalWrite(ledPin, HIGH);
digitalWrite(pumpRelay, LOW);
}

void loop()
{
currentMillis = millis();
if (currentMillis - previousMillis > betweenPumpRun) { //turn the pump on if enough time has passed
previousMillis = currentMillis; // remember when we last ran the pump
digitalWrite (pumpRelay, LOW);
pumpState = LOW;
}

pumpRunMillis = millis();
if (pumpState == LOW) {
if (pumpRunMillis - previousPumpMillis > pumpRunTime){
previousPumpMillis = pumpRunMillis ;
digitalWrite (pumpRelay, HIGH);
pumpState = HIGH;}}

blinkMillis = millis();
if(previousBlinkMillis - blinkMillis > blinkInterval) {
previousBlinkMillis = blinkMillis;
if (ledState == LOW)
ledState = HIGH;
else
ledState = LOW;
digitalWrite(ledPin, ledState);
}
}

2

By the way, I know that the time delays are set to 6 seconds and shorter for the rest. I am trying to use that for testing, so I want it to go on and off faster then waiting 10 mintues at a time.

Thanks for your assistance!

3

Your use of variables confused me a bit, so I rewrote a little to get rid of some of them...

/* Program to turn on misting pump for plant propogation. 
 Turn on the pump relay for a few seconds every 8 to 10 minutes.
 */
const int ledPin = 13;       // the number of the LED pin
const int pumpRelay = 10;   // Number of the PumpRun Pin

// constants
const unsigned long blinkInterval = 500;          //Interval between blinks               
const unsigned long pumpRunTime = 3000;           // interval to run the pump (10 sec currently)
const unsigned long betweenPumpRun = 6000;        // interval between pump running (10 min)

// variables
unsigned long previousBlinkMillis = 0;    // when last toggled LED
unsigned long previousPumpMillis =0;      // when last toggled pump

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(pumpRelay, OUTPUT);
  digitalWrite(ledPin, HIGH);
  digitalWrite(pumpRelay, LOW);
}  // end of setup

void loop()
{
  unsigned long currentMillis = millis();  // time now

  // if pump on, check if time to turn off
  if (digitalRead (pumpRelay) == LOW) 
    {

    // first blink the LED
    if(currentMillis - previousBlinkMillis > blinkInterval) 
      {
      previousBlinkMillis = currentMillis; 
      if (digitalRead(ledPin) == LOW)
        digitalWrite(ledPin, HIGH);
      else
        digitalWrite(ledPin, LOW);
      }  // end of time up

    if (currentMillis - previousPumpMillis >= pumpRunTime)
      {
      previousPumpMillis = currentMillis;
      digitalWrite (pumpRelay, HIGH);
      }  // end of time up
    }  // end of relay == LOW
  else
    {
    // otherwise, if pump off check if time to turn on
    if (currentMillis - previousPumpMillis >= betweenPumpRun)
      {          //turn the pump on if enough time has passed
      previousPumpMillis = currentMillis;      
      digitalWrite (pumpRelay, LOW);
      }  // end of time up
    }  // end of relay == HIGH

} // end of loop

This turns the pump on and off at different intervals and also blinks the LED on pin 13 every half second if it is on. I presume from the way you worded it that if the relay pin is low, the pump is on?

4

I tried to make it more simple so that it should work.
But now it works the first time. It turns on the relay when I first reset it. Then it does wait for the delay amount but only lets the relay stay on for a tiny fraction of the amount of time it should. The light just blinks, then it waits 10 more seconds and blinks again.
Arrugh

const int pumpRelay = 10; // Number of the PumpRun Pin

// Variables will change:
unsigned long previousPumpMillis = 0;
unsigned long previousMillis = 0;
long pumpRunTime = 5000; // interval to run the pump
long betweenPumpRun = 10000; // interval between pump running

void setup() {
pinMode(pumpRelay, OUTPUT);}

void loop() {
unsigned long currentMillis = millis();
if (currentMillis - previousMillis > betweenPumpRun) { //turn the pump on if enough time has passed
previousMillis = currentMillis; // remember when we last ran the pump
digitalWrite (pumpRelay, LOW);}

unsigned long pumpRunMillis = millis();
if (pumpRunMillis - previousPumpMillis > pumpRunTime){
previousPumpMillis = pumpRunMillis ;
digitalWrite (pumpRelay, HIGH);
}
}

5

Rather than quoting, can you "code" your code? Compare your post to mine above. Change the word "quote" in the post to "code".

6

Thanks Nick! OK that works.

From looking at what you did, I assume that I needed to nest my IF's. I was checking them one after another and was that messing up the timing? In other words, was the current time not what I expected it to be?

This is my first Sketch, well the first one after blink without delay modified to turn on my relay.

Yes, it's on when LOW. So it works perfect now. I have to study it more to figure out what I did wrong.

Have a Blessed Day!

Thanks,
Bill

7

I will "code" my code from now on. I was using the "copy for forum" button in the editor. So it put that in there automatically. Another good tip. Thanks!

8

I dont get it, what was the problem?
Also, when in 'quote' form, the code looks better displayed in my opinion, warmer color background and it highlights ifs and functions and etc... in 'code' it is just plain text. :confused: Why is that?

9

The problem was that my original code didn't work at all.

Nick, can you tell me why the original didn't work?

Or even the minimal testing one I put up later. I know that for some reason the time I was measuring using Millis() must have not been the values I was expecting. So it wasn't going through the loop the way that I expected and just counting. But it looks to me like it should work!!!!! And it does, but only the first time it runs through the loop. Which is why it frustrates me so much. It should have worked.

Thanks for helping out!

10

It should have worked.

No it is doing exactly what you wrote.
Make the variables global if you don't want them reset every time round the loop.

11

Grumpy_Mike:

It should have worked.

No it is doing exactly what you wrote.
Make the variables global if you don't want them reset every time round the loop.

I tried that too, I think. In the code below, it turns on the relay just once. Then it waits the delay amount, but the relay just turns on for a split second over and over. It waits the delay, click, waits the delay, click, waits the delay, click.... Never says on for the 5 seconds like it should. I know it's doing what I told it to, but I don't understand why telling it this way doesn't work. For some reason either Millis or Loop doesn't work the way I think it does.

const int pumpRelay = 10;   // Number of the PumpRun Pin

// Variables will change:
unsigned long previousPumpMillis = 0;
unsigned long previousMillis = 0;
long pumpRunTime = 5000;           // interval to run the pump 
long betweenPumpRun = 10000;       // interval between pump running
unsigned long currentMillis = millis();
unsigned long pumpRunMillis = millis();  

void setup() {
    pinMode(pumpRelay, OUTPUT);}

void loop() {

  currentMillis = millis();
    if (currentMillis - previousMillis > betweenPumpRun) {          //turn the pump on if enough time has passed
        previousMillis = currentMillis;         // remember when we last ran the pump    
        digitalWrite (pumpRelay, LOW);} 
 
  pumpRunMillis = millis();    
    if (pumpRunMillis - previousPumpMillis > pumpRunTime){
          previousPumpMillis = pumpRunMillis ;
          digitalWrite (pumpRelay, HIGH);
          }
}
12

here's my stab at the code
seems to turn a LED on and off for 2 and 5 seconds

const int pumpRelay = 13;   // Number of the PumpRun Pin

// Variables will change:
unsigned long previousMillis = 0;
unsigned long currentMillis = 0;
long pumpRunTime = 2000;          // interval (mS) to run the pump
long betweenPumpRun = 5000;       // interval (mS) between pump running
boolean pumpOn;

void setup() 
{
  Serial.begin(19200);
  pinMode(pumpRelay, OUTPUT);
  digitalWrite(pumpRelay, HIGH);
  pumpOn = false;
  Serial.println("off");
  previousMillis = 0;
}

void loop() 
{
  currentMillis = millis();
  if (pumpOn)
  {
    if (currentMillis - previousMillis > pumpRunTime)
    {
      //turn the pump off
      digitalWrite(pumpRelay, HIGH);
      pumpOn = false;
      Serial.println("off");
      previousMillis = currentMillis;
    }
  }
  else
  {
    if (currentMillis - previousMillis > betweenPumpRun) 
    {
      //turn the pump on
      digitalWrite(pumpRelay, LOW);
      pumpOn = true;
      Serial.println("on");
      previousMillis = currentMillis;
    }
  }
}

you need to check the pump state else you turn it off quicker than you turn it on!

@stgram
you use [ code] and [ /code]
a) so when the code gets big it's in a nice scrolly box
b) you don't get weird italics and so on from some character combinations!

13

Mk II version
flashes a LED as well!

const int pumpRelay = 10;   // Number of the PumpRun Pin
const int flashLED = 13;    // Number of the flashing LED

// Variables will change:
unsigned long previousMillis;
unsigned long flashMillis;
unsigned long currentMillis;
long pumpRunTime = 5000;       // interval (mS) to run the pump
long betweenPumpRun = 10000;    // interval (mS) between pump running
long flashInterval = 250;      // determines LED flash rate          
boolean pumpOn;
boolean ledOn;

void setup() 
{
  // initialise debugging on serial port
  //====================================
  Serial.begin(19200);
  
  // start with pump and LED off
  //============================
  pinMode(pumpRelay, OUTPUT);
  pinMode(flashLED, OUTPUT);
  digitalWrite(pumpRelay, HIGH);
  digitalWrite(flashLED, LOW);
  pumpOn = false;
  ledOn = false;
  Serial.println("off");
  
  // initialise the timers
  //======================
  currentMillis = 0;
  previousMillis = 0;
  flashMillis = 0;
}

void loop() 
{
  // get the current time
  //=====================
  currentMillis = millis();
  if (pumpOn)
  {
    // pump is ON, time to turn it OFF yet?
    //=====================================
    if (currentMillis - previousMillis > pumpRunTime)
    {
      //turn the pump OFF
      //=================
      digitalWrite(pumpRelay, HIGH);
      pumpOn = false;
      Serial.println("pump OFF");
      previousMillis = currentMillis;

      // turn the led OFF
      //=================
      digitalWrite(flashLED, LOW);
      ledOn = false;
      Serial.println("led OFF");
      flashMillis = currentMillis;
    }
    else
    if (currentMillis - flashMillis > flashInterval)
    {
      // flash interval has passed toggle the LED
      //=========================================
      if (ledOn)
      {
        digitalWrite(flashLED, LOW);
        ledOn = false;
        Serial.println("led OFF");
        flashMillis = currentMillis;
      }
      else
      {
        digitalWrite(flashLED, HIGH);
        ledOn = true;
        Serial.println("led ON");
        flashMillis = currentMillis;
      }
    }
  }
  else
  {
    // pump is OFF; time to turn it ON yet?
    //=====================================
    if (currentMillis - previousMillis > betweenPumpRun) 
    {
      //turn the pump ON
      //================
      digitalWrite(pumpRelay, LOW);
      pumpOn = true;
      Serial.println("pump ON");
      previousMillis = currentMillis;
    }
  }
}
14

Bill_of_the_Midwest:
Nick, can you tell me why the original didn't work?

Well for one thing:

    blinkMillis = millis();       
    if(previousBlinkMillis - blinkMillis > blinkInterval) {

That is backwards. I changed it to:

    blinkMillis = millis();       
    if(blinkMillis - previousBlinkMillis > blinkInterval) {

... and the LED blinked.

But then I read that you only want the LED to blink if the pump is on, and your code for blinking was unconditional (apart from the delay). So I moved it to the part where we knew the pump was on.

As for the rest, I couldn't get it to work myself for a while. I just got confused when you had a "time when we turned the pump on" and "time when we turned the pump off" variables. After all, you don't have "time when we turned the LED on" and "time when we turned the LED off" variables do you?

So rather than spend more time trying to work out what all these variables did, and if they were set and tested when they should be, I simplified. Which I do to my own code when necessary.

15

Nick,
Thanks for your help. Everything started making sense when I read through how you did it. Thank you for taking the time to do that.
I also ran across the tutorial on how to do multiple things on your forum here: Gammon Forum : Electronics : Microprocessors : How to do multiple things at once ... like cook bacon and eggs, I am learning a lot from that as well.

When I added serial outs to troubleshoot my original code I discovered that often that the pump was turning off multiple times while waiting to get to time it should run. Then if it had last turned off 1500 millis ago, and then hit 5000 milis to turn on, it would only stay on for a moment and shut off.
Plus I didn't use >= so that also messed up some things.....

Overall, it was just poorly written, but more things make sense now.

I've modified it to include an LED to let me know when it's waiting and I'm going to add a sensor next so the pump will only run during the day when it's light.

I'm on the right track now!