Go Down

Topic: unexpected result with millis (Read 1 time) previous topic - next topic

Hello, we are using an Arduino Mega 2560 for controlling old analog telephone relais witch are connected on the digital pins .
When we tested the sketch below , with 1  output, the status high seemed accurate  but the low status  was each 5 seconds not on the right moment. When using the full sketch, the whole thing seemed to act randomly. I think because of the fact that the loop cycle is interfering with the given times. Is there a way to solve this?
A rough solution could be; finding out how long it takes to read the loop, so we can calculate at what times the different steps are read. But i think there must be a better one.
Code: [Select]
unsigned long time;

void setup() {
pinMode(1,OUTPUT);
pinMode(2,OUTPUT);
pinMode(3,OUTPUT);
pinMode(4,OUTPUT);
pinMode(5,OUTPUT);
pinMode(6,OUTPUT);
pinMode(7,OUTPUT);
pinMode(8,OUTPUT);
pinMode(9,OUTPUT);
pinMode(10,OUTPUT);
pinMode(11,OUTPUT);
pinMode(13,OUTPUT);
pinMode(14,OUTPUT);
pinMode(15,OUTPUT);
pinMode(16,OUTPUT);
pinMode(17,OUTPUT);
pinMode(18,OUTPUT);
pinMode(19,OUTPUT);
pinMode(20,OUTPUT);
pinMode(21,OUTPUT);
pinMode(22,OUTPUT);
pinMode(23,OUTPUT);
pinMode(24,OUTPUT);
pinMode(25,OUTPUT);
pinMode(26,OUTPUT);
pinMode(27,OUTPUT);
pinMode(28,OUTPUT);
pinMode(29,OUTPUT);
pinMode(30,OUTPUT);
pinMode(31,OUTPUT);
pinMode(32,OUTPUT);
pinMode(33,OUTPUT);
pinMode(34,OUTPUT);
pinMode(35,OUTPUT);
pinMode(36,OUTPUT);
pinMode(37,OUTPUT);
pinMode(38,OUTPUT);
pinMode(39,OUTPUT);
pinMode(40,OUTPUT);

}

void eena(){
digitalWrite(1,HIGH);
}
void eenb(){
digitalWrite(1,LOW);
}
void tweea(){
digitalWrite(2,HIGH);
}
void tweeb(){
digitalWrite(2,LOW);
}
void driea(){
digitalWrite(3,HIGH);
}
void drieb(){
digitalWrite(3,LOW);
}
void viera(){
digitalWrite(4,HIGH);
}
void vierb(){
digitalWrite(4,LOW);
}
void vijfa(){
digitalWrite(5,HIGH);
}
void vijfb(){
digitalWrite(5,LOW);
}
void zesa(){
digitalWrite(6,HIGH);
}
void zesb(){
digitalWrite(6,LOW);
}
void zevena(){
digitalWrite(7,HIGH);
}
void zevenb(){
digitalWrite(7,LOW);
}
void achta(){
digitalWrite(8,HIGH);
}
void achtb(){
digitalWrite(8,LOW);
}
void negena(){
digitalWrite(9,HIGH);
}
void negenb(){
digitalWrite(9,LOW);
}
void tiena(){
digitalWrite(10,HIGH);
}
void tienb(){
digitalWrite(10,LOW);
}
void elfa(){
digitalWrite(11,HIGH);
}
void elfb(){
digitalWrite(11,LOW);
}
void dertiena(){
digitalWrite(13,HIGH);
}
void dertienb(){
digitalWrite(13,LOW);
}
void veertiena(){
digitalWrite(14,HIGH);
}
void veertienb(){
digitalWrite(14,LOW);
}
void vijftiena(){
digitalWrite(15,HIGH);
}
void vijftienb(){
digitalWrite(15,LOW);
}
void zestiena(){
digitalWrite(16,HIGH);
}
void zestienb(){
digitalWrite(16,LOW);
}
void zeventiena(){
digitalWrite(17,HIGH);
}
void zeventienb(){
digitalWrite(17,LOW);
}
void achttiena(){
digitalWrite(18,HIGH);
}
void achttienb(){
digitalWrite(18,LOW);
}
void negentiena(){
digitalWrite(19,HIGH);
}
void negentienb(){
digitalWrite(19,LOW);
}
void twintiga(){
digitalWrite(20,HIGH);
}
void twintigb(){
digitalWrite(20,LOW);
}
void eenentwintiga(){
digitalWrite(21,HIGH);
}
void eenentwintigb(){
digitalWrite(21,LOW);
}
void tweeentwintiga(){
digitalWrite(22,HIGH);
}
void tweeentwintigb(){
digitalWrite(22,LOW);
}
void drieentwintiga(){
digitalWrite(23,HIGH);
}
void drieentwintigb(){
digitalWrite(23,LOW);
}
void vierentwintiga(){
digitalWrite(24,HIGH);
}
void vierentwintigb(){
digitalWrite(24,LOW);
}
void vijfentwintiga(){
digitalWrite(25,HIGH);
}
void vijfentwintigb(){
digitalWrite(25,LOW);
}
void zesentwintiga(){
digitalWrite(26,HIGH);
}
void zesentwintigb(){
digitalWrite(26,LOW);
}
void zevenentwintiga(){
digitalWrite(27,HIGH);
}
void zevenentwintigb(){
digitalWrite(27,LOW);
}
void achtentwintiga(){
digitalWrite(28,HIGH);
}
void achtentwintigb(){
digitalWrite(28,LOW);
}
void negenentwintiga(){
digitalWrite(29,HIGH);
}
void negenentwintigb(){
digitalWrite(29,LOW);
}
void dertiga(){
digitalWrite(30,HIGH);
}
void dertigb(){
digitalWrite(30,LOW);
}
void eenendertiga(){
digitalWrite(31,HIGH);
}
void eenendertigb(){
digitalWrite(31,LOW);
}
void tweeendertiga(){
digitalWrite(32,HIGH);
}
void tweeendertigb(){
digitalWrite(32,LOW);
}
void drieendertiga(){
digitalWrite(33,HIGH);
}
void drieendertigb(){
digitalWrite(33,LOW);
}
void vierendertiga(){
digitalWrite(34,HIGH);
}
void vierendertigb(){
digitalWrite(34,LOW);
}
void vijfendertiga(){
digitalWrite(35,HIGH);
}
void vijfendertigb(){
digitalWrite(35,LOW);
}
void zesendertiga(){
digitalWrite(36,HIGH);
}
void zesendertigb(){
digitalWrite(36,LOW);
}
void zevenendertiga(){
digitalWrite(37,HIGH);
}
void zevenendertigb(){
digitalWrite(37,LOW);
}
void achtendertiga(){
digitalWrite(38,HIGH);
}
void achtendertigb(){
digitalWrite(38,LOW);
}
void negenendertiga(){
digitalWrite(39,HIGH);
}
void negenendertigb(){
digitalWrite(39,LOW);
}
void veertiga(){
digitalWrite(40,HIGH);
}
void veertigb(){
digitalWrite(40,LOW);
}


void loop()
{
time = millis();

if (time % 1000 == 0){
eena();
}
if (time % 1001 == 0){
eenb();
}
if (time % 1100 == 0){
tweea();
}
if (time % 1101 == 0){
tweeb();
}
if (time % 900 == 0){
driea();
}
if (time % 901 == 0){
drieb();
}
if (time % 1200 == 0){
viera();
}
if (time % 1201 == 0){
vierb();
}
if (time % 2000 == 0){
vijfa();
}
if (time % 2001 == 0){
vijfb();
}
if (time % 1500 == 0){
zesa();
}
if (time % 1501 == 0){
zesb();
}
if (time % 990 == 0){
zevena();
}
if (time % 991 == 0){
zevenb();
}
if (time % 550 == 0){
achta();
}
if (time % 551 == 0){
achtb();
}
if (time % 600 == 0){
negena();
}
if (time % 601 == 0){
negenb();
}
if (time % 500 == 0){
tiena();
}
if (time % 501 == 0){
tienb();
}
if (time % 600 == 0){
elfa();
}
if (time % 601 == 0){
elfb();
}
if (time % 950 == 0){
dertiena();
}
if (time % 951 == 0){
dertienb();
}
if (time % 800 == 0){
veertiena();
}
if (time % 801 == 0){
veertienb();
}
if (time % 700 == 0){
vijftiena();
}
if (time % 701 == 0){
vijftienb();
}
if (time % 850 == 0){
zestiena();
}
if (time % 851 == 0){
zestienb();
}
if (time % 550 == 0){
zeventiena();
}
if (time % 551 == 0){
zeventienb();
}
if (time % 300 == 0){
achttiena();
}
if (time % 301 == 0){
achttienb();
}
if (time % 400 == 0){
negentiena();
}
if (time % 401 == 0){
negentienb();
}
if (time % 300 == 0){
twintiga();
}
if (time % 301 == 0){
twintigb();
}
if (time % 350 == 0){
eenentwintiga();
}
if (time % 351 == 0){
eenentwintigb();
}
if (time % 200 == 0){
tweeentwintiga();
}
if (time % 201 == 0){
tweeentwintigb();
}
if (time % 250 == 0){
drieentwintiga();
}
if (time % 251 == 0){
drieentwintigb();
}
if (time % 320 == 0){
vierentwintiga();
}
if (time % 321 == 0){
vierentwintigb();
}
if (time % 120 == 0){
vijfentwintiga();
}
if (time % 121 == 0){
vijfentwintigb();
}
if (time % 500 == 0){
zesentwintiga();
}
if (time % 501 == 0){
zesentwintigb();
}
if (time % 500 == 0){
zevenentwintiga();
}
if (time % 501 == 0){
zevenentwintigb();
}
if (time % 500 == 0){
achtentwintiga();
}
if (time % 501 == 0){
achtentwintigb();
}
if (time % 500 == 0){
dertiga();
}
if (time % 501 == 0){
dertigb();
}
if (time % 500 == 0){
eenendertiga();
}
if (time % 501 == 0){
eenendertigb();
}
if (time % 500 == 0){
tweeendertiga();
}
if (time % 501 == 0){
tweeendertigb();
}
if (time % 500 == 0){
drieendertiga();
}
if (time % 501 == 0){
drieendertigb();
}
if (time % 500 == 0){
vierendertiga();
}
if (time % 501 == 0){
vierendertigb();
}
if (time % 500 == 0){
vijfendertiga();
}
if (time % 501 == 0){
vijfendertigb();
}
if (time % 500 == 0){
zesendertiga();
}
if (time % 501 == 0){
zesendertigb();
}
if (time % 500 == 0){
zevenendertiga();
}
if (time % 501 == 0){
zevenendertigb();
}
if (time % 500 == 0){
achtendertiga();
}
if (time % 501 == 0){
achtendertigb();
}
if (time % 150 == 0){
negenendertiga();
}
if (time % 151 == 0){
negenendertigb();
}
if (time % 500 == 0){
veertiga();
}
if (time % 501 == 0){
veertigb();
}

}

Magician

I hope you didn't connected relays directly to the outputs of the board?
You shouldn't, check on :

http://todbot.com/blog/bionicarduino/

class notes 1 to 4.

hobbified

Two things:

1. Are you sure that your conditions using % are really doing what you want them to? They could be, but it seems very strange to have a loop that's running with so many different intervals at once.

2. The whole thing is broken if the loop ever takes more than one millisecond to run, which is very possible with so many conditions and so many port writes. What you need is to take into account all of the time that has passed between the previous loop and the current one, and change any pins that should have had their values changed in the intervening time. Better yet would be to get some help from the hardware counters -- but you're working with too many pins to make that easy. Ideally, you would only have 4 or 5 per Arduino, and you could use the PWM pins directly.

CrossRoads

There is lot of duplication also, some that could be combined for faster operation.
Try setting it up as switch:case also so it can run faster.
Not sure if this would quite do what you want, as at millis = 1000 for example
1000 % 200, 250, 500, 1000 would all == 0 and
600 % 120, 150, 200, 300, 600 would all == 0,
many which all meet the 1200, 1500, and 2000 intervals, so some optimization could be done there.

I see 22 different intervals, many of which will occur repeatedly, so lump them together.

Using blink without delay kind of code:

void loop(){
  currentMillis = millis();  // sample the time
  if (currentMillis - previousMillis >= one_millis_interval) // more than our interval?
  {
    // save the last time we okayed time updates
    previousMillis = currentMillis; // save the current time for next comparison
    time = time+1;
if (time == 2000){
// reset the time?
time = 0;
}

switch (time){
case 120:
vijfentwintiga();
break;

case 121:
vijfentwintigb();
break;

:
:
case 600:
all the things that happen at 120, 200, 300, 600
break;

case 1000:
all the things that happen at 200,250, 500, 1000
break;
:
case 2000:
all the things that happen at 200, 400, 500, 1000, 2000, etc.
vijfa();
delay(1);
// wait 1 mS then do the 2001 event
vijfb();
break;
} // end switch                                                                                                                                   
} // end of time interval test
} //end void loop

your code sorted by occurrences:
Code: [Select]

void loop()
{
time = millis();
if (time % 120 == 0){
vijfentwintiga();
}
if (time % 121 == 0){
vijfentwintigb();
}
if (time % 150 == 0){
negenendertiga();
}
if (time % 151 == 0){
negenendertigb();
}
if (time % 200 == 0){
tweeentwintiga();
}
if (time % 201 == 0){
tweeentwintigb();
}
if (time % 250 == 0){
drieentwintiga();
}
if (time % 251 == 0){
drieentwintigb();
}
if (time % 300 == 0){
twintiga();
achttiena();
}
if (time % 301 == 0){
twintigb();
achttienb();
}
if (time % 320 == 0){
vierentwintiga();
}
if (time % 321 == 0){
vierentwintigb();
}
if (time % 350 == 0){
eenentwintiga();
}
if (time % 351 == 0){
eenentwintigb();
}
if (time % 400 == 0){
negentiena();
}
if (time % 401 == 0){
negentienb();
}
if (time % 500 == 0){
zesentwintiga();
zevenentwintiga();
achtentwintiga();
dertiga();
eenendertiga();
tweeendertiga();
drieendertiga();
vierendertiga();
vijfendertiga();
zesendertiga();
achtendertiga();
zevenendertiga();
veertiga();
tiena();
}

if (time % 501 == 0){
zesentwintigb();
zevenentwintigb();
achtentwintigb();
dertigb();
eenendertigb();
tweeendertigb();
drieendertigb();
vijfendertigb();
zevenendertigb();
achtendertigb();
tienb();
}

if (time % 550 == 0){
achta();
zeventiena();
}

if (time % 551 == 0){
achtb();
zeventienb();
}
if (time % 600 == 0){
negena();
elfa();
}
if (time % 601 == 0){
negenb();
elfb();
}
if (time % 700 == 0){
vijftiena();
}
if (time % 701 == 0){
vijftienb();
}
if (time % 800 == 0){
veertiena();
}
if (time % 801 == 0){
veertienb();
}
if (time % 850 == 0){
zestiena();
}
if (time % 851 == 0){
zestienb();
}
if (time % 900 == 0){
driea();
}
if (time % 901 == 0){
drieb();
}
if (time % 950 == 0){
dertiena();
}
if (time % 951 == 0){
dertienb();
}
if (time % 990 == 0){
zevena();
}
if (time % 991 == 0){
zevenb();
}
if (time % 1000 == 0){
eena();
}
if (time % 1001 == 0){
eenb();
}
if (time % 1100 == 0){
tweea();
if (time % 1101 == 0){
tweeb();
}
if (time % 1200 == 0){
viera();
}
if (time % 1201 == 0){
vierb();
}
if (time % 1500 == 0){
zesa();
}
if (time % 1501 == 0){
zesb();
}
if (time % 2000 == 0){
vijfa();
}
if (time % 2001 == 0){
vijfb();
}
Designing & building electrical circuits for over 25 years.  Screw Shield for Mega/Due/Uno,  Bobuino with ATMega1284P, & other '328P & '1284P creations & offerings at  my website.


I hope you didn't connected relays directly to the outputs of the board?

No, that isn't the problem, we putted some resistors and such in between

bperrybap

Are you using real electro-mechanical relays? i.e. coil activated?
If so, it seems unlikely that they can do much in 1ms.

The current code turns on the pin and off again 1ms later.

If the relay is supposed to energize turn on when the pin is high,
it would seem unlikely that it would be able to fully energize fully in 1ms.

But the biggest thing to check is to see if the loop is taking longer than 1ms.
Wouldn't surprise if it was. Theres a lot of 32 bit math going on in that loop.
If it is longer than 1ms whacky things will happen.
You'll need to time it with a analyzer or scope using a spare i/o pin.

Since your numbers are all small, you might try a quick test using 16 bit values instead.
Try declaring time as a unsigned int instead
and then use:

time = (uint16_t) millis();

to use only the lower 16 bits.

--- bill

westfw

The timer interrupt occurs every 1024 microseconds, and the interrupt service routine increments the millis() count by one each time, AND increments it again whenever enough 24s have passed to total another millisecond.  This means that sometimes, the value returned by millis() will be two greater than the previous call, and it also means that certain values of millis() will NEVER OCCUR.

I suspect that some of the values you are using match up with some of the values that never occur.

Checking for an exact value of time rather than "past a certain time" is generally a pad idea.  In addition to the problem you're seeing here on Arduino, other environments may increment their clock value by more than one tick at a time, or if there is other code running (say, in a multiprocessing environment) you can simply miss the exact tick you were waiting for.

Your code is sufficiently complex and obscure that I don't have an immediate thought on how to fix it, short of adding a "next time" variable for each pin (which seems like it would be painful.)

Nick Gammon

Your code is, er, rather complex. For a start, try replacing setup with:

Code: [Select]
void setup()
{
for (byte i = 1; i <= 40; i++)
  pinMode(i, OUTPUT);
}


Then you might look at whether you can make similar improvements to the rest.

Quote

No, that isn't the problem, we putted some resistors and such in between


Circuit please?

You realize, I suppose, that the "%" is the modulus operator. That is, the remainder after a division. Now a division is one of the slowest bits of arithmetic you can do (try it in your head if you don't believe me).  And you are asking it to do 76 divisions every 1/1000 of a second.

A quick test shows that an Arduino Mega 2560 takes 3.34 milliseconds to do a single batch of 76 modulus operations on an unsigned long. Test:

Code: [Select]
void setup () {
pinMode (13, OUTPUT);
}

unsigned long time;

void loop ()
{

digitalWrite (13, HIGH);

time = millis();
for (byte i = 0; i < 76; i++)
  if (time % 1000)
    digitalWrite (8, OUTPUT);
digitalWrite (13, LOW);
   
delay (1000);
}


In this example, pin 13 was held high for 3.34 ms.

So it is completely impossible for your sketch to work, if you are relying on the loop being executed once every millisecond. You need to find a more efficient way of doing it. For example, don't do a modulus, but work on intervals.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

AWOL

Code: [Select]
  digitalWrite (8, OUTPUT);

best to be consistent, dontcha think?  ;-)
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

Nick Gammon

Oh yes, good point. Although OUTPUT and HIGH are both defined to be 1.

I wrote that in a hurry and it seemed to work. ;)
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

dafid

#10
Mar 12, 2011, 09:26 pm Last Edit: Mar 13, 2011, 04:34 pm by dafid Reason: 1
This is a good place for data driven programming...

This is your code expressed in that style...
Code: [Select]
// defines the intervals for each pin
// on is % the interval, of is % interval+1
// the pin is not changed if the interval is 0.
unsigned int interval[41] = {
0, // index 0 not used, just to align pin numbers to array indexes.
//1
1000, 1100, 900, 1200, 2000, 1500, 990, 550, 600, 500,
// 11 (12 not used)
600, 0, 950, 800, 700, 850, 550, 300, 400, 300,
// 21 (29 not used)
350, 200, 250, 320, 120, 500, 500, 500, 0, 500,
// 31
500, 500, 500, 500, 500, 500, 500, 500, 150, 500
} ;

void setup(void)
{
int i;

for (i=1; i <= 40; i++)
pinMode(i, OUTPUT) ;
}

void
loop(void)
{
unsigned long int time = millis() ;
byte i ;

// for (i=1; i<= 41; i++) // ** MISTAKE**
for (i=1; i <= 40; i++)
{
if (interval[i])
{
if ((time % interval[i]) == 0)
{
digitalWrite(i, HIGH) ;
}
if ((time % (interval[i]+1)) == 0)
{
digitalWrite(i, LOW) ;
}
}
}
}

dafid

#11
Mar 12, 2011, 09:35 pm Last Edit: Mar 13, 2011, 04:40 pm by dafid Reason: 1
Once it is data driven..
it is easy to change the logic..


You realize, I suppose, that the "%" is the modulus operator. That is, the remainder after a division. Now a division is one of the slowest bits of arithmetic you can do (try it in your head if you don't believe me).  And you are asking it to do 76 divisions every 1/1000 of a second.
....
A quick test shows that an Arduino Mega 2560 takes 3.34 milliseconds to do a single batch of 76 modulus operations on an unsigned long. Test:
So it is completely impossible for your sketch to work, if you are relying on the loop being executed once every millisecond. You need to find a more efficient way of doing it. For example, don't do a modulus, but work on intervals.


To address Nicks comment about % being slow, the % is replaced in loop() with additions and comparisons..
This is what was meant by using 'intervals' in some of the earlier posts.

Code: [Select]
// defines the intervals for each pin
// on is % the interval, of is % interval+1
// the pin is not changed if the interval is 0.
unsigned int interval[41] = {
0, // index 0 not used, just to align pin numbers to array indexes.
//1
1000, 1100, 900, 1200, 2000, 1500, 990, 550, 600, 500,
// 11 (12 not used)
600, 0, 950, 800, 700, 850, 550, 300, 400, 300,
// 21 (29 not used)
350, 200, 250, 320, 120, 500, 500, 500, 0, 500,
// 31
500, 500, 500, 500, 500, 500, 500, 500, 150, 500
} ;
// records the next time the pin should be turned on
unsigned long int nextOn[41] ;
// records the next time the pin should be turned off
unsigned long int nextOff[41] ;

void setup(void)
{
int i;

for (i=1; i <= 40; i++)
pinMode(i, OUTPUT) ;

unsigned long int time = millis() ;
// for (i=1; i < 41; i++)  // This is not a mistake, but is inconsistent to the line above.
//
for (i=1; i <= 40; i++)
{
if (interval[i] != 0)
{
nextOn[i] = time + interval[i] - (time % interval[i]) ;
nextOff[i] = time + interval[i]+1 - (time % (interval[i]+1)) ;
}
}
}

void
loop(void)
{
unsigned long int time = millis() ;
byte i ;

// for (i=1; i<= 41; i++) // ** MISTAKE**
for (i=1; i <= 40; i++)
{
if (interval[i])
{
if (nextOn[i] <= time)
{
nextOn[i] += interval[i] ;
digitalWrite(i, HIGH) ;
}
if (nextOff[i] <= time)
{
nextOff[i] += interval[i]+1 ;
digitalWrite(i, LOW) ;
}
}
}
}


Of course there is still the question of whether the implemented algorithm is what you intended.
As the the periods the switches remain on will over time vary between 0 ms and interval [ i ]+1 ms.

CrossRoads

@gregorvanmulders,

Is any of this looking helpful to you?
Designing & building electrical circuits for over 25 years.  Screw Shield for Mega/Due/Uno,  Bobuino with ATMega1284P, & other '328P & '1284P creations & offerings at  my website.

floresta



I hope you didn't connected relays directly to the outputs of the board?

No, that isn't the problem, we putted some resistors and such in between
I hope the "and such" includes some diodes across the relay coils.

Don

dafid

#14
Mar 13, 2011, 04:32 pm Last Edit: Mar 13, 2011, 04:41 pm by dafid Reason: 1
The maximum current the ATMEGA2560 can use is is 200 milliAmp, so even though the per output current limit is 40 milliAmps, in your case where there could be most of the 40 pins active at once, you should be drawing 5 milliAmps (or less) per pin.

Also - in the code i posted earlier there are errors -- I will fix them in place and leave the bad code as comments so you can see clearly the mistakes.




Go Up