Move variables from void setup to loop (timer)

I have this snip of code that I need to use variable withs. But the code gets its parameters from before the void setup. When I try to move them inside the loop I get compiler errors.

unsigned long phase1 =10;
unsigned long phase2 = 1;
const int phasecount = 2;
unsigned long durations[phasecount] = { phase1, phase2 };
void loop(){
 static int phase;
       static unsigned long phasestart;

    if (now() - phasestart >= durations[phase]) {
    phase = (phase + 1) % phasecount;
    phasestart = now();
    switch (phase) {
	  case 0: // turn on
	Serial.println("im on");	// Turn things on
		break;
	  case 1: // turn of
	Serial.println("im off");	// Turn things off
		break;
     }
}
}

My Problem is that
unsigned long phase1 =10;
unsigned long phase2 = 1;

need to be variables created inside the loop.

unsigned long phase1 = timetobeon;
unsigned long phase2 = timetobeoff;

any ideas?

GroPi5000v0_3:335: error: jump to case label GroPi5000v0_3:314: error: crosses initialization of 'long unsigned int durations [2]' GroPi5000v0_3:313: error: crosses initialization of 'const int phasecount' GroPi5000v0_3:312: error: crosses initialization of 'long unsigned int phase2' GroPi5000v0_3:311: error: crosses initialization of 'long unsigned int phase1' GroPi5000v0_3:339: error: jump to case label GroPi5000v0_3:314: error: crosses initialization of 'long unsigned int durations [2]' GroPi5000v0_3:313: error: crosses initialization of 'const int phasecount' GroPi5000v0_3:312: error: crosses initialization of 'long unsigned int phase2' GroPi5000v0_3:311: error: crosses initialization of 'long unsigned int phase1'

Is the compiler error I get if I do this inside the function.

unsigned long phase1 = 10;
unsigned long phase2 = 1;
const int phasecount = 2;
unsigned long durations[phasecount] = { phase1, phase2 };
static int phase;
static unsigned long phasestart;

    if (now() - phasestart >= durations[phase]) {
    phase = (phase + 1) % phasecount;
    phasestart = now();
    switch (phase) {
      case 0: // turn on
            Serial.println("im on");    // Turn things on
        break;
      case 1: // turn of
            Serial.println("im off");   // Turn things off
        break;
     }

My Problem is that unsigned long phase1 =10; unsigned long phase2 = 1;

need to be variables created inside the loop.

unsigned long phase1 = timetobeon; unsigned long phase2 = timetobeoff;

any ideas?

The whole purpose of using a variable is so that you can assign a new value to it at any time. You seem to be under the impression that declaration of a variable and assigning it a value are permanently linked.

Make the variables global (or static, local). Then, assign them new values whenever you need to,

unsigned long phase1; unsigned long phase2;

phase1 = 10; phase2 = someOtherValue;

One of the reasons that we discourage snippet posting here is because it takes the code out of context. The error message in the 2nd post make it appear as though you are defining variables in one case of a switch statement and then trying to reference them in another case.

unsigned long phase1 = 10;
unsigned long phase2 = 1;
const int phasecount = 2;
unsigned long durations[phasecount] = { phase1, phase2 };

durations' values will not change when phase1 and/or phase2 change. You understand that, right?

Post all of your code (use comments to describe where you want to move stuff to), and we'll help you get it right.

unsigned long phase1;
unsigned long phase2;

phase1 = 10;
phase2 = someOtherValue;

this works perfectly. Well it compiles, ill test if its actually doing what I think it will tonight. I obviously wasnt thinking last night. Looked at it to long and am so close to the end (of the beginning) of this device I cant wait. Thanks!

durations’ values will not change when phase1 and/or phase2 change. You understand that, right?

Not sure I understand you there.
I posted the whole code. One thing I dont understand is I got no compiler errors but
phase1 = PumpOnLight;
phase2 = PumpOffLight;
PumpOnLight is an int and phase1 is an unsigned long. After all the gnashing of teeth converting between types I thought I was going to have to add a line to convert those as well?

uh oh, my code is now to long to post in the forum. its attached.
And my auto-formatter is throwing java exceptions. compiles fine though. sorry for the mess. its pretty well commented though.

GroPi5000v0_3.pde (11.3 KB)

jointtech: One thing I dont understand is I got no compiler errors but phase1 = PumpOnLight; phase2 = PumpOffLight; PumpOnLight is an int and phase1 is an unsigned long. After all the gnashing of teeth converting between types I thought I was going to have to add a line to convert those as well?

int is smaller than long. There is no problem assigning a small thing to a larger thing - it fits.

And my auto-formatter is throwing java exceptions.

If you get rid of the line starting:

 //comes back from php as echo  ...

... then the auto-formatter works. I think that rather lengthy line was too much for it.

What are you doing here? ...

        record = string;

        //Get MaxPH and ready the variable
        p = strtok(record,",");
        MaxPH = atoi(p);

        //Get MinCO2Level and do something.
        p = strtok(NULL,",");
        MinCO2Level = atoi(p);

        //Get MaxPHVariance and ready the variable
        p = strtok(record,",");
        MaxPHVariance = atoi(p);       

        //Get PHDoseAmount and ready the variable
        p = strtok(record,",");
        PHDoseAmount = atoi(p);

Only the first strtok is supposed to have "record" in it, the others should have NULL, in order to step through the line. You only used NULL once.

doh. bad copy pasting. I would have gotten that in the testing tonight. I was in get it to compile mode... thanks. that did fix the auto formatter. thanks.

ok I made some progress. Paul your comment finally clicked after some more searching around. As long as you init the variable above the setup its global. period. I just had to go move all my inits up there. Now I am able to seperate the code into 3 seperate functions like I wanted. Cleaned it up some as well. Its starting to click now.
Now to plug a lightbulb into this thing and see if the delay works as expected.

GroPi5000v0_4.pde (12 KB)

As long as you init the variable above the setup its global.

Well, you can't initialize a variable before you declare it, so if you declare a variable AND initialize it before setup(), then it will be global. It is not necessary to initialize the variable at the time it is declared. So, to be technically accurate, a variable [u]declared[/u] before setup() is global.

So my timer isnt timing. It just loops on and off no matter what info it gets from serial read.

curl http://localhost:81/getmaxlevels.phpexit im on a a a root dudevin curl http://localhost:81/getmaxlevels.phpexit a a a root dudevin curl http://localhost:81/getlevelsfromdongles.php?theresult=@drydingledongle,0,83.17,47.57,1362,4,-1.9750385284,864,78z exit im off a a a root dudevin curl http://localhost:81/getmaxlevels.phpexit im on a a a root dudevin curl http://localhost:81/getmaxlevels.phpexit a a a root dudevin curl http://localhost:81/getlevelsfromdongles.php?theresult=@drydingledongle,0,83.12,45.68,799,3,-1.9750385284,535,78z exit im off a a a root dudevin

 if (lightsensorValue > 10)  //If it is light use the light pump settings.
  {
    static int phase;
    static unsigned long phasestart;
    phase1 = PumpOnLight;
    phase2 = PumpOffLight;
    if (now() - phasestart >= durations[phase]) {
      phase = (phase + 1) % phasecount;
      phasestart = now();
      switch (phase) {
      case 0: // turn on
        Serial.println("im on");    // Turn things on
        digitalWrite(waterpumprelay, HIGH);   // turn the water pump on
        break;
      case 1: // turn of
        Serial.println("im off");   // Turn things off
         digitalWrite(waterpumprelay, LOW);  //turn the water pump off
        break;
      }
    }
  }
  else {  //If it is dark use the Dark pump settings
    static int phase;
    static unsigned long phasestart;
    phase1 = PumpOnDark;
    phase2 = PumpOffDark;
    if (now() - phasestart >= durations[phase]) {
      phase = (phase + 1) % phasecount;
      phasestart = now();
      switch (phase) {
      case 0: // turn on
        Serial.println("im on");    // Turn things on
        digitalWrite(waterpumprelay, HIGH);   // turn the water pump on
        break;
      case 1: // turn of
        Serial.println("im off");   // Turn things off
         digitalWrite(waterpumprelay, LOW);  //turn the water pump off
        break;
      }
    }
  }

If I send it 1200,1200 it should stay on 20 min and stay off 20min. I cant see how it would just loop on and off with the case settings? If it was "broken" I would expect it to not do anything. It does switch the relay through everyloop though.. strange.

unsigned long phase1;
unsigned long phase2;
const int phasecount = 2;
unsigned long durations[phasecount] = { 
  phase1, phase2 };

You may as well have simply written:

unsigned long phase1;
unsigned long phase2;
const int phasecount = 2;
unsigned long durations[phasecount];

the two are functionally equivalent.

ok. Can you elaborate?
What am I doing wrong in my code since this code works:

#include <Time.h>
unsigned long phase1 =10;
unsigned long phase2 = 1;
const int phasecount = 2;
unsigned long durations[phasecount] = { phase1, phase2 };

void setup()
{
  Serial.begin(9600); //This is the setup function where the serial port is initialised,
 }

void loop() {
static int phase;
static unsigned long phasestart;

if (now() - phasestart >= durations[phase]) {
    phase = (phase + 1) % phasecount;
    phasestart = now();
    switch (phase) {
	  case 0: // turn on
	Serial.println("im on");	// Turn things on
		break;
	  case 1: // turn on
	Serial.println("im off");	// Turn things off
		break;
     }
}
}
unsigned long phase1;
unsigned long phase2;
const int phasecount = 2;
unsigned long durations[phasecount] = { 
  phase1, phase2 };

is exactly the same as writing:

unsigned long phase1 = 0;
unsigned long phase2 = 0;
const int phasecount = 2;
unsigned long durations[phasecount] = { 0, 0};

Is that clearer?

much clearer, thanks. ah.. ok so how about

unsigned long phase1 = 0;
unsigned long phase2 = 0;
const int phasecount = 2;
unsigned long durations[phasecount];

and then

static int phase;
static unsigned long phasestart;
phase1 = PumpOnLight;
phase2 = PumpOffLight;
durations[phasecount] = { phase1, phase2 };

well that didnt compile but this does

static int phase;
    static unsigned long phasestart;
    phase1 = PumpOnLight;
    phase2 = PumpOffLight;
    unsigned long durations[phasecount] = { phase1, phase2 };

durations[phasecount] = { phase1, phase2 };

That cannot work - you can't re-initialise arrays like that.

You could do it with pointers:

unsigned long phase1;
unsigned long phase2;
const int phasecount = 2;
unsigned long* durations[phasecount] = { 
  &phase1, &phase2 };

then if (now() - phasestart >= *durations[phase]) {

but I'm fairly sure if you refactored your code, you'd see a simpler way.

The above snip compiles but does the same thing, just loops on and off. So I'm off to refactor some code. wheres a good place to learn about defining and initializing variables and the different types. for example unsigned long* durations[phasecount] = { &phase1, &phase2 }; whats the * and & represent? One of the biggest problems im having is stuff like that. I've done some PHP and .NET but never had some much grief over variables until arduino. I just wanted to play with blinky lights and sensors ;)

unsigned long* durations[phasecount] = { &phase1, &phase2 };

The "*" says "pointer" and the "&" says "address-of".

So, I defined an array of two elements, each element being a pointer to an unsigned long. I then initialised the two pointers with the addresses of "phase1" and "phase2". Now, if I write

phase1 = 1200;
//...
Serial.print (*durations [0]);

I should get the value 1200 printed, because the "*" dereferences the pointer and fetches the value stored at that address.

ok well its not working because of a different bug in the code :wink: phase1 and phase2 were returning 0… so it WAS working with a timer setting of 0 on and 0 off and a 40-something second loop time.
so Ill debug that in the morning and i’m sure it will come to life… thanks for your help this evening.

ok couldnt sleep with it in my head. My problem was: char string[32]; string looks like this $6,1500,.1,1,1000,1200,70,100,1,95,45,10,120,10,1200# and I guess blew it up.

6 150 0 1 1000 1200 70 100 1 9 0 0 0 0 0

I changed it to char string[64]; and its returning the right stuff.

6 100 0 1 1000 1200 70 100 1 95 45 10 120 10 1200

see, damn variable declaration and init got me again. Now I can sleep in peace and work on the timer I started on today... thanks again.