BWoD Concept working in one function but not the other

Projects Programming
1

Hi, this is my first post and I have been learning to work with this Arduino Duo for exactly 5 days. I have a decent amount of coding experience over my 40+ years of life but am brand spanking new to programming microcontrollers.

My project begins fairly straightforward. I have two fog machines that I want to alternately fire for 6 seconds each with a 2 second cooldown in between them when both fog machines are online (detected by a "preheat" pin going high from the fog machine's thermostat). If only one machine is found to be online I have a different timing scheme of 8 seconds on and 3 seconds off.

My long term vision is to incorporate a LCD screen and rotary encoder with some buttons to allow on the fly manipulation of the timing intervals but for now the code is limited to just demonstrating that the timing works with the default values.

Currently the only hardware connected to the Arduino board are a couple of LEDs to stand in for the relays. The LEDS are current limited to ground with 10k ohm resistors. The preheat pins are manually connected to Vcc to emulate the fog machine(s) coming online but are otherwise pulled down to Ground using 10k ohm resistors. The relay signal pins are likewise pulled down to Ground using 10k ohm resistors just to keep everything at a known state. As the code has evolved the hardware has always performed exactly as expected.

My code is currently littered with several Serial.print lines to aid in debugging and has comments along the way to aid in readability. The code as posted actually works. My issue is more that I don't really understand why. To be more specific, I don't understand why the BWod concept works in the fog() function but not in the same way in the noFog() function.

In fog() the loop remains in the function until if (onLoopStart - loopElapsed >= fogOn && fogState == true) becomes true. This makes sense to me because at this point the program has no means of exiting the function.

Looking at noFog() when if (offLoopStart - loopElapsed >= fogOff) evaluates as false the program will instantly return to fog() without the else statement to call the noFog() function for a repeat pass. Why does the program not remain in this function; what makes this one different from fog()?

The Serial output for this loop reads:

...fogger 1
noFog
running offLoopStart: 183227   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183309   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183390   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183471   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183552   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183632   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183714   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183795   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183876   running loopElapsed: 183217
try again
noFog
running offLoopStart: 183957   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184039   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184120   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184201   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184282   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184363   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184444   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184525   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184606   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184687   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184769   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184850   running loopElapsed: 183217
try again
noFog
running offLoopStart: 184931   running loopElapsed: 183217
try again
noFog
running offLoopStart: 185012   running loopElapsed: 183217
try again
noFog
running offLoopStart: 185093   running loopElapsed: 183217
try again
noFog
running offLoopStart: 185175   running loopElapsed: 183217
try again
noFog
running offLoopStart: 185255   running loopElapsed: 183217
finished offLoopStart: 185255   finished loopElapsed: 185255
fogger 2...

Here's the code:

//assign I/O pin names
const int Relay1 = 11;
const int Relay2 = 12;
const int Preheat1 = 9;
const int Preheat2 = 10;

//create relay on/off durations
long unsigned dualFogOn = 6000;
long unsigned dualFogOff = 2000;
long unsigned singleFogOn = 8000;
long unsigned singleFogOff = 3000;

//create clock watch variables
long unsigned loopElapsed = 0;
bool foggerToggle = true;
bool fogState = true;

//fog state functions
void noFog(long unsigned fogOff)
{
  unsigned long offLoopStart = millis();
  if (fogState == false)
  {
    //turn off the fogger(s)
    Serial.println("noFog");
    digitalWrite(Relay1, LOW);
    digitalWrite(Relay2, LOW);
    Serial.print("running offLoopStart: ");
    Serial.print(offLoopStart);
    Serial.print("   running loopElapsed: ");
    Serial.print(loopElapsed);
    Serial.println();
  }

  if (offLoopStart - loopElapsed >= fogOff)
  {
    loopElapsed = offLoopStart;
    Serial.print("finished offLoopStart: ");
    Serial.print(offLoopStart);
    Serial.print("   finished loopElapsed: ");
    Serial.print(loopElapsed);
    Serial.println();
    //prepare to fire the alternate fogger
    foggerToggle = !foggerToggle;
    //enable fogger mode functions
    fogState = true;
  }
  else
  {
    Serial.println("try again");
    noFog(fogOff);
  }
};


void fog(int fogger, long unsigned fogOn, long unsigned fogOff)
{
  unsigned long onLoopStart = millis();
  //determine which fogger is to be fired
  if (fogger == 1 && fogState == true)
  {
    Serial.print("fogger ");
    Serial.println(fogger);
    digitalWrite(Relay1, HIGH);
    digitalWrite(Relay2, LOW);
  }
  else if (fogger == 2 && fogState == true)
  {
    Serial.print("fogger ");
    Serial.println(fogger);
    digitalWrite(Relay1, LOW);
    digitalWrite(Relay2, HIGH);
  };

  //continue fogging for the specified amount of time
  if (onLoopStart - loopElapsed >= fogOn && fogState == true)
  {
    loopElapsed = onLoopStart;
    fogState = false;
    noFog(fogOff);
  }
};



void setup()
{
  //assign pin modes
  pinMode(Relay1, OUTPUT);
  pinMode(Relay2, OUTPUT);
  pinMode(Preheat1, INPUT);
  pinMode(Preheat2, INPUT);
  //open comms for debug
  Serial.begin(9600);
}



void loop()
{
  //preheat mode
  if (digitalRead(Preheat1) == LOW && digitalRead(Preheat2) == LOW)
  {
    fogState = false;
    noFog(5000);
  };

  //alternating fogger mode 1
  if (digitalRead(Preheat1) == HIGH && digitalRead(Preheat2) == HIGH && (foggerToggle == true))
  {
    fogState = true;
    fog(1, dualFogOn, dualFogOff);
  };

  //alternating fogger mode 2
  if (digitalRead(Preheat1) == HIGH && digitalRead(Preheat2) == HIGH && (foggerToggle == false))
  {
    fogState = true;
    fog(2, dualFogOn, dualFogOff);
  };

  //fogger1 only mode
  if (digitalRead(Preheat1) == HIGH && digitalRead(Preheat2) == LOW)
  {
    fogState = true;
    fog (1, singleFogOn, singleFogOff);
  };

  //fogger2 only mode
  if (digitalRead(Preheat1) == LOW && digitalRead(Preheat2) == HIGH)
  {
    fogState = true;
    fog (2, singleFogOn, singleFogOff);
  };
}
2

your code has a lot of conditionals

consider the following while i go thru your code

struct Fog {
    byte          pin;
    bool          active;
    unsigned long msecPeriodOn;
    unsigned long msecPeriodOff;
    unsigned long msecPeriod;
    unsigned long msecLst;
    const char   *desc;
};

Fog fogs [] = {
    { 10, 1, 600, 200, 0, 0, "fog1" },
    { 11, 1, 600, 200, 0, 0, "fog2" },
};
const size_t N_Fog = sizeof(fogs)/sizeof(Fog);

enum { Off = HIGH, On = LOW };

unsigned long msec;

// -------------------------------------
void
setLed (
    Fog        *p,
    unsigned long  msecPeriodOn,
    unsigned long  msecPeriodOff )
{
    p->active        = true;
    p->msecPeriodOn  = msecPeriodOn;
    p->msecPeriodOff = msecPeriodOff;
}

// -----------------------------------------------------------------------------

void
loop (void)
{
    msec = millis ();

    Fog *p = fogs;
    for (unsigned n = 0; n < N_Fog; n++, p++)  {
        if (! p->active)
            continue;

        if ((msec - p->msecLst) > p->msecPeriod)  {
            if (On == digitalRead (p->pin))  {
                digitalWrite (p->pin, Off);
                p->msecPeriod = p->msecPeriodOff;
            }
            else  {
                digitalWrite (p->pin, On);
                p->msecPeriod = p->msecPeriodOn;
            }

            p->msecLst = msec;

            Serial.println (p->desc;
        }
    }
}

// -----------------------------------------------------------------------------
void
setup (void)
{
    Serial.begin (9600);

    fogs [0].msecLst -= fogs [0].msecPeriodOn / 2;

    Fog *f = fogs;
    for (unsigned n = 0; n < N_Fog; n++, f++)  {
        digitalWrite (f->pin, Off);
        pinMode      (f->pin, OUTPUT);
    }
}
3

This:

  //continue fogging for the specified amount of time
  if (onLoopStart - loopElapsed >= fogOn)
  {
    if (fogState == true)
    {
      loopElapsed = onLoopStart;
      fogState = false;
      noFog(fogOff);
    }
  }

is different in a subtle way, and makes it really match the logic what works.

Not sure where the loopElapsed time shoukd be updated and can't experiment just now, sry.

a7

4

not sure what you're trying to do.

if you want to stay in "nofog()" until fogOff msec has expired, the conventional approach would be

   while (millis() - loopElapsed <= fogOff)
        ;

or maybe just call delay (fogOff)

but you also recursively call "nofog()" in the else condition within "nofog()" which besides being bad practice, seems unnecessary

5

consider

what do the PreHeat input mean?
should they throttle the relays during the ON period?

//assign I/O pin names
const int Relay1 = 11;
const int Relay2 = 12;
const int Preheat1 = 9;
const int Preheat2 = 10;

//create relay on/off durations
long unsigned dualFogOn    = 600;
long unsigned dualFogOff   = 200;
long unsigned singleFogOn  = 800;
long unsigned singleFogOff = 300;

//create clock watch variables
long unsigned loopElapsed = 0;
bool foggerToggle = true;
bool fogState = true;
bool dualMode = true;

char s [80];

// -----------------------------------------------------------------------------
//turn off the fogger(s)
void noFog (long unsigned fogOff)
{
    Serial.println ("noFog:");

    digitalWrite(Relay1, LOW);
    digitalWrite(Relay2, LOW);

    fogState = false;

    delay (fogOff);
};

// -----------------------------------------------------------------------------
//turn off the fogger(s)
void fog (
    int           fogger,
    long unsigned fogOn,
    long unsigned fogOff )
{
    Serial.print   ("  fog: ");
    Serial.println (fogger);

    if (fogger == 1)
    {
        digitalWrite(Relay1, HIGH);
        digitalWrite(Relay2, LOW);
    }
    else
    {
        digitalWrite(Relay1, LOW);
        digitalWrite(Relay2, HIGH);
    };

    delay (fogOn);

    noFog(fogOff);
};

// -----------------------------------------------------------------------------
void setup()
{
    //assign pin modes
    pinMode(Relay1, OUTPUT);
    pinMode(Relay2, OUTPUT);

#if 0
    pinMode(Preheat1, INPUT);
    pinMode(Preheat2, INPUT);
#else
    pinMode(Preheat1, OUTPUT);
    pinMode(Preheat2, OUTPUT);
    digitalWrite(Preheat1, LOW);
    digitalWrite(Preheat2, LOW);
#endif

    //open comms for debug
    Serial.begin(9600);
}

// -----------------------------------------------------------------------------
void loop()
{
    if (dualMode)  {
        fog (foggerToggle, dualFogOn, dualFogOff);
        foggerToggle = ! foggerToggle;
    }
    else
        fog (foggerToggle, singleFogOn, singleFogOff);
}
6

A fog machine has to warm-up and if the fog runs long enough the temperature of the heater falls enough that the thermostat shuts it off until it can come back up to temperature.

Thus, Preheat signals if the fog machine is available to be fired.

My original code was extremely simple and appeared to work extremely well because it used delays prodigiously. The future additions to this project likely would require disuse of delay. At a minimum I do want the program to switch modes when a fogger falls out of Preheat. That functionality does exist in this iteration and did/could not using delay.

7

so you would switch the other fogger on if not already on? what of both preheaters are off?

my post #2 showed an approach not using delays. need to understand the preheater issue better

8

I am trying to set the Relay1/2 pins low for the provided fogOff interval. No fog would be emitted during this period.

Your code makes no sense to me and does not work in practice. When loopStart begins it will always be some microseconds larger than loopElapsed and with each iteration that gap grows and so that statement can never be true. I am already using >= which is the conventional and recommended method.

I am aware the recursion is terrible practice. That's the point of this post; to resolve a method for noFog() that works as well as it does for fog(). As you can see from the serial output that the noFog() function will exit if I don't recurse it which is not true for fog() and I don't understand why.

9

The if condition for both preheats going low is to call noFog() repeatedly with a 5 second interval until at least one of them goes high:

void loop()
{
  //preheat mode
  if (digitalRead(Preheat1) == LOW && digitalRead(Preheat2) == LOW)
  {
    fogState = false;
    noFog(5000);
  };...
10

It sounds to me as though a state machine would be a good way to code this

There seem to be 4 states

fog machine 1 on for periodA
fog machine 1 off for periodB
fog machine 2 on for periodC
fog machine 2 off for periodD

The duration of the periods depends on input from the preheat pins of the 2 fog machines noting that the periods for a fog machine that is off line could be zero

11

I don't disagree. The fogState variable is strictly to keep the fog() initiating conditionals from triggering the fogOn loop while the program is in the fogOff state. It's potentially extraneous. I did reword it to no different effect. You will note that the pin state change takes place ahead of anything else and that is what I was trying to protect; ensuring the fogState had been correctly toggled before taking the pins down which logically should take place before anything else.

12

loopElapsed, or startTime would be set just before the while. but this is really no better than delay(), hence my earlier post

there's no need to repeatedly call a function to perform some action that already been performed (i.e. turn of relays). as post #2 demonstrates, a timer test in loop() can determine when to do such things

see above. do you need to take any action or just recognize that the thermostat has turned the machine off (??).

so as soon as the preheater clears you want to turn the machine back on for some limited amount of time? presumably you want to recognize that you're in such a state.

13

Your code has a lot of surplus semicolons. I am a little surprised that the compiler doesn't object to the couple that you have right after a function ends.

14

I don't disagree with sentiment that repeating a pin change achieves anything. It's my current solution for being able to immediately leave state and do something different based on preheat conditionals.

To be fair, the likelihood of a fogger going offline (Preheatn=low) due to overfogging is unlikely. Ideally one would use appropriate fogOn times such as to never overrun the heater's capacity to stay up to temperature. I did want to make the on/off time user configurable and adaptable to future fog machines with different capabilities. Thus enters the possibility for overrunning a machine based on experimentation or mistake. Really, it's more important that a second fogger be immediately used if added to an already single fogger state of running (which is currently achieved).

It's a valid argument that nothing is technically ever hurt by trying to fire a fog machine that is not preheated as the thermostat is literally the master switch for pump operation. That said, I wanted my code to behave as if that were not necessarily true.

15

I have a wide ranging hodgepodge of coding language exposure and C is towards the bottom of that list. I don't doubt what you say to be true but the compiler never complains about them. i think I just have a policy of ending any statements that I am finished stating.

16

Fundamentally I just want to know:

What makes noFog() exit on the first iteration unless recursed while fog() suffers no such constraint? Or vice versa, why can fog() stay put until the "timer" expires and noFog() can't.

I'm not convinced both questions have the same answer but I would like it if they did.

17

I like that it could be that simple. I would just need assurances that there could never be any overlap such as both fog machines on simultaneously or a fog machine being on strictly because another was off (i.e. the cooldown period is about both machines being off).

18

They aren't errors. Just not usual.

# include <stdio.h>

// with -Wall -Wextra.

int main()
{
    printf("Hello World");

;
;
;

    return 0;
}

;
;

No error, no warning.

a7

19

I admit I can read this only because I know what the program is supposed to be doing and even then I had to take long pauses and think it through. I do like the use of arrays (a path I was considering going down once this pile worked) and how the only for loop necessarily can't block itself which is doing all the things that need doing and is only changing pin states once as needed. This code exhibits a clear level of language and concept mastery well ahead of my current abilities to port and write with good syntax and strong understanding of what I am doing. For example, if there is something wrong in there I wouldn't be able to point it out to you.

While I am not opposed to following through on the discovery using that example as the roadmap I feel like right now I may be better served to just understand why my own code only works the way I wrote it which we both acknowledge is less than desirable. I am currently really just looking for enlightenment on why I have to recurse one function to stay inside it to intended completion while I don't have to for the other despite them being structured effectively the same.

20

That is the beauty of a state machine. Only the code for the current state is executed. When a change of state is triggered, in your case based on a time period ending, you execute any code necessary to end the code cleanly, such as turning off the current fog machine. Then you set up the entry conditions for the target state, such as saving the current time and finally change state

Using switch/case is a convenient way to implement a state machine, particularly if you names for the states, perhaps by using an enum