Pages: [1] 2 3   Go Down
Author Topic: millis() blink != 50/50  (Read 2863 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Faraday Member
**
Karma: 66
Posts: 2568
Now, More Than Ever
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

There has been a spate of millis() questions.
Seems like it might be the hardest part of Arduino to grasp.
I'm trying to expand on an example with equal on and off times and have a blink with different on_time and off_time.
Code:
long waitUntil = 0;
boolean LED13state = true;

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

void loop()
{
  if (LED13state = true)
  {
    digitalWrite(13, LED13state);
    if (millis() >= waitUntil)
      {
        LED13state = !(LED13state); // LED13state = false
        waitUntil = millis() + 50;
      }
  }
  if (LED13state = false)
  {
    digitalWrite(13, LED13state);
    if (millis() >= waitUntil)
      {
        LED13state = !(LED13state); // LED13state = true
        waitUntil = millis() + 950;
      }
  }
}

It gets it once, on 50ms, off 950ms, and then back on where it stays.
Logged

"Hello, I must be going..."
"You gotta fight -- for your right -- to party!"
Don't react - Read.
"Who is like unto the beast? who is able to make war with him?"

Seattle, WA USA
Online Online
Brattain Member
*****
Karma: 631
Posts: 50065
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
  if (LED13state = false)
The second hardest concept might be = vs ==.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 495
Posts: 19035
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Code:
  if (LED13state == true)
  {
...
        LED13state = !(LED13state); // LED13state = false

Wouldn't it be easier to write:


Code:
        LED13state = false;

After all, that's what the comment says.

(Note, corrected = to == when quoting). smiley-wink

Logged


Ft. Worth, Texas
Offline Offline
God Member
*****
Karma: 0
Posts: 591
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
  if (LED13state = false)
The second hardest concept might be = vs ==.

I think Paul his the finishing nail square on the head with a sledge hammer.

Time and Time again, I see this.
Logged

KF5RVR

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 495
Posts: 19035
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Trouble is, people "graduate" from VBscript (or Visual Basic), where you do in fact use "=" to test for equality.
Logged


Offline Offline
Faraday Member
**
Karma: 66
Posts: 2568
Now, More Than Ever
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

OK
Thanks for your Replies.
I have the 'double equals' / "equal to" implemented.

Using..
Code:
long waitUntil = 0;
boolean LED13state = true;   // init as True = 1 = on

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

void loop()
{
  if (LED13state == true)
  {
    digitalWrite(13, LED13state);  // D13 = 1 (true)?
    if (millis() >= waitUntil)
      {
        LED13state = !LED13state; // LED13state = true
        waitUntil = millis() + 950;
      }
  }
  if (LED13state == false)
  {
    digitalWrite(13, LED13state);  // D13 = 0 (false)?
    if (millis() >= waitUntil)
      {
        LED13state = !LED13state; // LED13state = false
        waitUntil = millis() + 50;
      }
  }
}
I get 50ms ON time and 950ms OFF time.
Since I start with LED13state = true in the first place, I expected the opposite, that my ON time would be 950 and the OFF 50.
Logged

"Hello, I must be going..."
"You gotta fight -- for your right -- to party!"
Don't react - Read.
"Who is like unto the beast? who is able to make war with him?"

Ft. Worth, Texas
Offline Offline
God Member
*****
Karma: 0
Posts: 591
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

A better way to do this with less code, IMO, is with a while loop.
Logged

KF5RVR

Offline Offline
Faraday Member
**
Karma: 66
Posts: 2568
Now, More Than Ever
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

OK
I can't get why I'm stuffing it with my example, the hole in my perception.
But if you've a better example I welcome it.
Thanks.
Logged

"Hello, I must be going..."
"You gotta fight -- for your right -- to party!"
Don't react - Read.
"Who is like unto the beast? who is able to make war with him?"

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 495
Posts: 19035
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

I get 50ms ON time and 950ms OFF time.
Since I start with LED13state = true in the first place, I expected the opposite, that my ON time would be 950 and the OFF 50.

You have:

Code:
if (LED13state == true)
  {
    digitalWrite(13, LED13state);  // D13 = 1 (true)?
    if (millis() >= waitUntil)
      {
        LED13state = !LED13state; // LED13state = true
        waitUntil = millis() + 950;
      }
  }

So once it is ON, you then set it up to wait until time is up (ie. 50mS) then turn it off and wait for 950 mS.

Maybe toggle the boolean, then set the pin. In fact, who needs variables?

Code:
void loop()
{
  if (digitalRead (13) == HIGH)
  {
    if (millis() >= waitUntil)
      {
        digitalWrite(13, LOW);  
        waitUntil = millis() + 50;  // off for 50 mS
      }
  }
  else
  {
    if (millis() >= waitUntil)
      {
        digitalWrite(13, HIGH);
        waitUntil = millis() + 950;  // on for 950 mS
      }
  }
} // end ofloop

Also, this won't work when millis wraps around. Better to do a subtraction:

Code:
unsigned long waitUntil;
unsigned long startTime;

void loop()
{
  if (millis() - startTime >= waitUntil)
    {
     if (digitalRead (13) == HIGH)
     {
      digitalWrite(13, LOW);  
      startTime = millis ();
      waitUntil = 50;  // off for 50 mS
     }
    else
     {
      digitalWrite(13, HIGH);
      startTime = millis ();
      waitUntil = 950;  // on for 950 mS
     }
  }  // end if time up
} // end ofloop
« Last Edit: December 17, 2011, 01:11:57 am by Nick Gammon » Logged


Pittsburgh, PA, USA
Offline Offline
Faraday Member
**
Karma: 98
Posts: 4813
I learn a bit every time I visit the forum.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I need to learn to think, type, test and debug faster....
I can't think of an easier way to say all this in English... 1st though, now you digital write only on state change.

Tested code:
Code:
unsigned long waitUntil = 0UL; // because millis() returns UL
boolean LED13state = false; // so the light turns on 1st time through loop()   

void setup()
{
   pinMode(13, OUTPUT);
   digitalWrite(13, LED13state);  
}

void loop()
{
    if (waitUntil - millis() > 1000UL)  // welcome to unsigned math
    {
        LED13state = ! LED13state;
        if (LED13state == true)    waitUntil = millis() + 50UL;
       else                                   waitUntil = millis() + 950UL;
       digitalWrite(13, LED13state);  
    }
}


Sketch to show unsigned math across unsigned overflow:
Code:
unsigned long a, b, c;

void setup() {
  Serial.begin( 9600 );
  a = 0xffffff00UL;
  b = 0x10UL;
  Serial.println( "unsigned math" );
  Serial.print( "a = ");
  Serial.print( a, DEC );
  Serial.print( " = 0x");
  Serial.println( a, HEX );
  Serial.print( "b = ");
  Serial.print( b, DEC );
  Serial.print( " = 0x");
  Serial.println( b, HEX );
  if ( b >= a ) Serial.println( "b >= a" );
  else          Serial.println( "a > b" );
  c = a - b;
  Serial.print( "a - b = ");
  Serial.print( c, DEC );
  Serial.print( " = 0x");
  Serial.println( c, HEX );
  c = b - a;
  Serial.print( "b - a = ");
  Serial.print( c, DEC );
  Serial.print( " = 0x");
  Serial.println( c, HEX );
  c = b - (b + 1);
  Serial.print( "b - (b + 1) = ");
  Serial.print( c, DEC );
  Serial.print( " = 0x");
  Serial.println( c, HEX );
}

void loop() {};

How do you copy the Serial Monitor to clipboard?

Logged

I find it harder to express logic in English than in Code.
Sometimes an example says more than many times as many words.

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 495
Posts: 19035
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

How do you copy the Serial Monitor to clipboard?

Just select it and Copy. Doesn't that work for you? What operating system are you on?
Logged


Offline Offline
Faraday Member
**
Karma: 66
Posts: 2568
Now, More Than Ever
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
"Who needs variables?"
Well, I'm a "magic numbers" guy, my tendency is to eschew variables.  In long passages, I can see how they facilitate understanding, esp. when assistance is requested. 
(I didn't want to get gigged for not having used variables.  "When in Rome...")

Anyway, I'm using an example written by a fairly influential forum contributor:
Code:
long waitUntil = 0;
boolean LED13state = true;
const byte led_pin = 13;

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

void loop()
{
  digitalWrite(13, LED13state);
  if (millis() >= waitUntil)
    {
      LED13state = !(LED13state);
      waitUntil = millis() + 500;
    }

So, I thought I could take advantage of his boolean flag for asymmetry (desired).

Why does
Code:
if (LED13state == true)
  {
    digitalWrite(13, LED13state);  // D13 = 1 (true)?
    if (millis() >= waitUntil)
      {
        LED13state = !LED13state; // LED13state = true
        waitUntil = millis() + 950;
      }
  }
only get me 50ms of On time and not 950ms?  [Because I'm a total dork, yes?]
I mean...
here it comes into the IF with LED13state being True, and also by that time millis() is > 0 (waitUntil's init value); so LED13state gets inverted [now = false] and waitUntil becomes millis() + 950
and.. ??
I'm stumped. (Oh, me.) 
What's going on that's evading me, that I ought consider but I'm blind to? 

[ Nick G. - That subtraction example wouldn't compile for me. ]
Logged

"Hello, I must be going..."
"You gotta fight -- for your right -- to party!"
Don't react - Read.
"Who is like unto the beast? who is able to make war with him?"

UK
Offline Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

When you're trying to make something happen at a regular interval, I recommend that you use
Code:
nextTime = nextTime + INTERVAL;
Rather than:
Code:
nextTime = now + INTERVAL;

By the time you make this assignment you are some unknown and variable time past when the event was due to have occurred. To avoid cumulative error, schedule the next event based on when the previous event was actually due, not based on 'now'.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Offline Offline
Faraday Member
**
Karma: 66
Posts: 2568
Now, More Than Ever
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

OK
No progress in nailing that "state" deal.
I went back to the shop, as it were, for what I could hammer out on my own.

Using...
Code:
const byte led_pin = 13;    // using on-board "LED D13"
const long time_on = 100;
const long time_off = 900;
unsigned long change_pt = 0;

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

void loop()
{
  change_pt = (millis() + time_on);
  while (millis() < change_pt)
  {
    digitalWrite(led_pin, HIGH);
  }
 
  change_pt = (millis() + time_off);
  while (millis() < change_pt)
  {
    digitalWrite(led_pin, LOW);
  } 
}
...the blink stays in sync with the klacker on WWV.

It's likely not roll-over proof and if I've minimised the effect of any cumulative error, unlikely, that was purely by accident.
Logged

"Hello, I must be going..."
"You gotta fight -- for your right -- to party!"
Don't react - Read.
"Who is like unto the beast? who is able to make war with him?"

Seattle, WA USA
Online Online
Brattain Member
*****
Karma: 631
Posts: 50065
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

There are still a couple of issues with your code. Well, three.

First, adding time variables is not a good idea. Addition can cause rollover.

Second, there is no reason to have digitalWrite() in the while body. It needs to be called once, not over and over again.

Third, congratulations on re-writing the delay() function. Not a particularly necessary activity, since there is a perfectly good way to waste time already.

The whole reason for using millis() is to do things in a non-blocking way.
while(now - later)
{
   // Do nothing useful
}
is blocking, exactly like delay() is.
Logged

Pages: [1] 2 3   Go Up
Jump to: