 # timing way off on simple sketch

The last delay in this sketch I expected to be 30 minutes, but it's closer to 30 seconds. Other than that, the sketch works as planned.

I'm using an UNO.

BTW, I don't need any of the time values to be absolutely accurate, but they do need to be consistent.

unsigned long secondsInMs(int m){ return m*1000; //converts milliseconds to seconds }

// the setup function runs once when you press reset or power the board void setup() { // initialize digital pins 9 and 10 as outputs. pinMode(9, OUTPUT); pinMode(10, OUTPUT); }

// the loop function runs over and over again forever void loop() { digitalWrite(9, HIGH); // turn mixing pump on using a NO relay delay(secondsInMs(20)); // let mixing pump run for 20 seconds digitalWrite(9, LOW); // turn mixing pump off digitalWrite(10, HIGH); // turn dosing pump on using a NO relay delay(secondsInMs(5)); // let dosing pump run for 5 seconds digitalWrite(10, LOW); // turn dosing pump off delay(secondsInMs(1800)); // wait 30 minutes to turn mixing pump back on to start over }

``````unsigned long secondsInMs(int m){
return m*1000;  //converts milliseconds to seconds
}
``````

The comment is completely wrong. Multiplying by 1000 converts seconds to milliseconds.

What is that function trying to do?

`````` delay(secondsInMs(1800)); // wait 30 minutes to turn mixing pump back on to start
``````

Grab your calculator. Multiply 1800 by 1000. Tell me how that number will fit in an int register. 1800 is input to the function as an int. 1000 is an int. An int times an int is an int. That you return the value as an unsigned long is irrelevant.

1000UL is not an int, if you are looking for a solution.

PaulS: ``` unsigned long secondsInMs(int m){   return m*1000;  //converts milliseconds to seconds } ```

The comment is completely wrong. Multiplying by 1000 converts seconds to milliseconds.

What is that function trying to do?

That function is trying to do what the comment says-convert milliseconds to seconds.

It's not completely wrong because it works. The problem was something else.

Wouldn't you divide by 1000 to convert seconds to milliseconds?

PaulS: ``` delay(secondsInMs(1800)); // wait 30 minutes to turn mixing pump back on to start ```

Grab your calculator. Multiply 1800 by 1000. Tell me how that number will fit in an int register. 1800 is input to the function as an int. 1000 is an int. An int times an int is an int. That you return the value as an unsigned long is irrelevant.

1000UL is not an int, if you are looking for a solution.

Of course, I was looking for a solution. 1000UL must be some kind of clue...I googled it and found a lot of pipettes and a panic bar. No help there.

I didn't realize that you could convert milliseconds to both seconds and minutes in the same sketch. Here's the fix;

unsigned long minutesInMs(int m){ return m*60000; //converts milliseconds to minutes

}

unsigned long secondsInMs(int s){ return s*1000; //converts milliseconds to seconds }

I admit, it took me quite awhile to get to the bottom of it. Thanks for your reply, have some karma.

s * 1000L If you want a long result, use long constants. That documents the code as well as getting it right.

Do you mean placing 'L' after '1000' is the same as designating it long?

As in these examples are equal?

long secondsInMs(int s){ return s*1000; //converts milliseconds to seconds

secondsInMs(int s){ return s*1000L; //converts milliseconds to seconds

saltyjoe: Wouldn't you divide by 1000 to convert seconds to milliseconds?

No. Let's do it your way. I give you 5 seconds. You divide by 1000. The answer is 0.005 ms. Oops! That is 5 microseconds.

saltyjoe: As in these examples are equal?

long secondsInMs(int s){ return s*1000; //converts milliseconds to seconds

secondsInMs(int s){ return s*1000L; //converts milliseconds to seconds

No. You still need the return value type. What both Mark and Paul were getting at was the fact that the compiler does int maths by default, which allows for a maximum positive value of 32767. So if you had this:-

``````return s*1000
``````

and s was 60, you'd get an unexpected outcome because the result is 60000, (too large to fit into an 'int' variable). The fact that your return value type is 'long' would make no difference. If, instead, you write the function as:-

``````long secondsInMs(int s)
{
return s*1000L;  //converts seconds to milliseconds (This is what it actually does.)
``````

} the 'L' tells the compiler to do 'long' mathematics, so the correct result would be returned. Similarly, "UL" would tell the compiler to do 'unsigned long' math.

Now I'm really confused....a millisecond is 1/1000 of a second, right? And a microsecond is 1/1000000000 right? And 0.005 seconds is 5/1000 seconds, or 5 milliseconds. Isn't that right?

And a microsecond is 1/1000000000 right?

Wrong

Good catch-I meant to put 1 over a million. 1/1000000

You are a feet to inches converter. I give you 2. You return 24. Did you multiply or divide by 12? You multiplied.

Then you claim that you are an inches to feet converter.

aarg: You are a feet to inches converter. I give you 2. You return 24. Did you multiply or divide by 12? You multiplied.

Then you claim that you are an inches to feet converter.

I am even more confused. Lets please stick to seconds.

By aarg: 'Let's do it your way. I give you 5 seconds. You divide by 1000. The answer is 0.005 ms. Oops! That is 5 microseconds.'

Let's say I have 5 seconds. I divide it by 1000. That would give me 5 milliseconds, not 5 microseconds.

If I'm missing something obvious, and I feel like I must be, please point it out.

saltyjoe: I am even more confused. Lets please stick to seconds.

By aarg: 'Let's do it your way. I give you 5 seconds. You divide by 1000. The answer is 0.005 ms. Oops! That is 5 microseconds.'

Let's say I have 5 seconds. I divide it by 1000. That would give me 5 milliseconds, not 5 microseconds.

If I'm missing something obvious, and I feel like I must be, please point it out.

We've been trying to point it out. What you are proposing in this post, is actually a seconds to milliseconds function. But in the function in your posted code you multiply by 1000, not divide.

If I'm missing something obvious, and I feel like I must be, please point it out.

``````unsigned long secondsInMs(int m){
return m*1000;  //converts milliseconds to seconds
``````

You are obviously converting seconds to milliseconds for use in delay() which accepts milliseconds as the argument. The comment was backwards, and using int m instead of int s for the argument makes it more confusing.

This was pointed out in reply #6.

``````long secondsInMs(int s)
{
return s*1000L;  //converts seconds to milliseconds (This is what it actually does.)
``````

Having the function return a long (or better unsigned long), and making sure that the constant is identified as U or UL to tell the compiler what to do is a separate issue and has gotten mixed up in the confusion over the comment for your function.

I lifted this from another sketch. I figured m*60000 meant 1 millisecond x 60000 or 60 seconds or 1 minute. Converts milliseconds to minutes. That's how the sketch works anyway.

unsigned long minutesInMs(int m){ return m*60000; //converts milliseconds to minutes

}

I made these adjustments to end with seconds. Again, I figured s*1000 meant 1 millisecond x 1000 or 1 second. You can't imagine how happy I was when the sketch acted that way.

unsigned long secondsInMs(int s){ return s*1000; //converts milliseconds to seconds }

There's an easier way by using constants.

``````const unsigned long SECONDS = 1000UL;
const unsigned long MINUTES = SECONDS * 60UL;

.
.
.

// Then your half hour delay would look like :

delay(MINUTES * 30UL);
``````

No need for your function. Let the compiler do the work. It's also easier to change and more obvious what's happening.

Even better, use the Time library, because it has all those constants, and a ton of useful time macros and functions as well.

saltyjoe: I made these adjustments to end with seconds. Again, I figured s*1000 meant 1 millisecond x 1000 or 1 second. You can't imagine how happy I was when the sketch acted that way.

unsigned long secondsInMs(int s){ return s*1000; //converts milliseconds to seconds }

You still don't get it what was said in the first reply. The code converts seconds to milliseconds and that is also what the name implies. The problem is that the comment is wrong. It states that it converts milliseconds to seconds (what would require a divide by 1000).

``````unsigned long secondsInMs(int s){
return s*1000;  //converts seconds to milliseconds
}
``````

This sounds silly but documenting your code properly is important. One day in the future somebody else might have to modify the code (or you have to) and somebody else or you don't understand what is going on.

I agree, solid comments are important. You guys are all over me about the comments-you guys are the ones who know programming-what can I do but conclude I have it wrong?

Goofing with Arduino, I found if I simply use a numeric value for a delay, it's gonna be in milliseconds. But a numeric value using the functions in this crude sketch will be in seconds or minutes. That's why I thought it was converting milliseconds to seconds. The end result made me believe there was a conversion of milliseconds to seconds.....

BTW, this is for dosing a slurry of calcium hydroxide (kalkwasser) to my reef tank. I'm looking for a little help with pH. It's hard to imagine getting the needed control without something like the magic of Arduino.

Thanks all.

``````unsigned long minutesInMs(int m){
return m*60000;  //converts minutes to milliseconds

}

unsigned long secondsInMs(int s){
return s*1000;  //converts seconds to milliseconds
}

void setup() {

pinMode(9, OUTPUT);       //pins 9 and 10 on your arduino are what gets connected to the optocoupler relays
pinMode(10, OUTPUT);
}

void loop() {
digitalWrite(9, HIGH);    // turn mixing pump on using a NO relay
delay(secondsInMs(30));   // let mixing pump run for 30 seconds
digitalWrite(9, LOW);     // turn mixing pump off

digitalWrite(10, HIGH);   // turn dosing pump on using another NO relay
delay(secondsInMs(5));  // let dosing pump run only briefly to prevent a precipitation event
digitalWrite(10, LOW);    // turn dosing pump off briefly to allow slurry to dissipate
delay(secondsInMs(5));

digitalWrite(10, HIGH);   // same as above, this just spreads out the kalk slurry.
delay(secondsInMs(5));
digitalWrite(10, LOW);
delay(secondsInMs(5));

digitalWrite(10, HIGH);   // same as above.
delay(secondsInMs(5));
digitalWrite(10, LOW);
delay(secondsInMs(5));

digitalWrite(10, HIGH);   // almost same as above.
delay(secondsInMs(5));
digitalWrite(10, LOW);    // turn dosing pump off

delay(minutesInMs(30));   // in 30 minutes, the mixing pump turns back on and the loop function repeats itself again and again and again...be sure to closely monitor alkalinity and pH for at least a few days.
}
``````

Joe, by now after 57 posts on these forums, you should be well aware that you can't post your code inline and that it must be placed between code tags. Everyone in this thread except you has done so. Why make it harder for people than it needs to be? (It's not too late to edit and fix this. :) )