Demonstration code for several things at the same time

I debated using switch statements (even just using them in one function to illustrate how they are an alternative to if statements). In the end I decided that it would just add to the learning burden for new users. It would be an unusual program that could avoid if statements but switch statements are always optional - even if convenient when you know them.

I like my bracketing system. The formatting was done manually in Gedit. If I have not been consistent everywhere I apologize.

...R

1 Like
...
    if (currentMillis - previousOnBoardLedMillis >= onBoardLedInterval) {
...
       previousOnBoardLedMillis = currentMillis;
    }

The average interval will be slightly to significantly more than onBoardLedInterval. For most uses (like blinking an LED) the longer interval is insignificant. For some uses the longer interval is a failure.

       previousOnBoardLedMillis += onBoardLedInterval;

...produces a bit more code but keeps the average interval at exactly onBoardLedInterval.

2 Likes

@CodingBadly, I'm interested but I don't understand exactly what you mean by "The average interval will be slightly to significantly more than onBoardLedInterval"

My plan was/is to use a single instant in time (as captured in currentMillis) as the reference point for the flashing of all of the leds so that their timing can't drift apart.

I can't figure out what effect your code would have.

...R

Nice long example (not all examples can be 1<10 lines).

I wondered what the difference was between the attached file in the first and reply#1 with the code inline. My editor only shows comments lines are shorter :grin:

I would do it slightly different (which means neither better nor worse) :slight_smile:

I do not quite get the educational value of having update_LED_state() and switchLEDs() as seperate functions - it means you have to carry LED-desired-state as global variables, when they could be nicely encapsulated. Place most variables as static inside the function where they are exclusivly used - this way you can not inadvertently modify a timer variable elsewhere.

CodingBadly's comment is that if your loop takes - say - 5 millisec to execute (because of some not-yet-written heavy calculation code), then the LEDs will blink at the interval+5ms. By adding "interval" to the "timer" (not the measured millis() value) you get it to run at the "interval" rate. For a "real" project it may be desirable to include the loop overhead/jitter, for some you want a steady rate.

That is my preferred approach too.

When your things are each implemented in separate self-contained functions s in the original example, I also like to define the state data as local static variables rather than globals. Reducing the scope is beneficial in its own right but in particular this makes copy/paste much safer, and that's something that I would expect novice coders to be doing a lot when starting from examples like this. For example:

void blinkLed()
{
	static const int LED_PIN = 13;
	static const unsigned long BLINK_INTERVAL = 500; // duration of each blink phase
	static unsigned long lastBlinkTime = 0;
	
	if(millis() - lastBlinkTime >= BLINK_INTERVAL)
	{
		digitalWrite(LED_PIN, !digitalRead(LED_PIN));
		lastBlinkTime += BLINK_INTERVAL
	}
}
2 Likes

PeterH:
That is my preferred approach too.

I'm hoping you or @GodingBadly will explain why.

I did consider static local variables but personally I prefer the simplicity and visibility of globals when I'm working with such a small memory space. (I wouldn't do that with PC code). Also I was trying to limit the amount of new stuff that a newbie has to take in.

I understand what you say about copy/paste, but I guess I'm old fashioned enough to prefer users to understand what's going on.

There is an opportunity for someone to use my code as a starting point for a separate lesson on the use of static local variables :slight_smile:

And thanks very much for all the comments.

...R

Two hints...

  1. You assume the delta is always 1. It is not.
  2. You assume the time to execute loop is <= 1 millisecond. In your example that may be true but that will not be true for every application.

Assume millis returns values evenly divisible by seven (0, 7, 14, etcetera) and work through the code by hand.

3 Likes

@jiggy-ninja:
I also prefer the switch statement but you need break statements in there to make it equivalent to the original code. I would write it like this:

void updateLed_A_State()
{
  switch( led_A_State )
  {
    case LOW:
      if (currentMillis - previousLed_A_Millis >= led_A_Interval)
      {
        led_A_State = HIGH;
        previousLed_A_Millis = currentMillis;
      }
      break;
      
    case HIGH:
      if (currentMillis - previousLed_A_Millis >= blinkDuration) 
      {
        led_A_State = LOW;
        previousLed_A_Millis = currentMillis;
      }
      break;
      
    default:
      break;
  }
}

Pete

Msquare:
I wondered what the difference was between the attached file in the first and reply#1 with the code inline. My editor only shows comments lines are shorter :grin:

Sorry, I meant to mention this earlier ...

You are quite right, I was just trying to save enough bytes to get within the upload limit. I actually haven't tested the visible version :frowning:

...R

el_supremo:
@jiggy-ninja:
I also prefer the switch statement but you need break statements in there to make it equivalent to the original code. I would write it like this:

Pete

.< Doh!

And that, ladies and gentlemen, is one argument against using switches.

2 Likes

Hey, no, don't give up on switches!
'if' statements can go haywire too if you forget an 'else'! :slight_smile:

Pete

@CodingBadly and @PeterH,

I think I now see the point you are making. I wrote a couple of spreadsheets to experiment with numbers.

I then set out to modify my sketch but I've run into a very strange problem. All of the functions work fine your way ( += interval) EXCEPT the servo function. After a short period it goes haywire, yet it works perfectly using " = currentMillis".

I think the problem may be due to the much shorter interval (or may be showing up sooner because of the shorter interval). And I think the problem arises because at some stage prevMillis exceeds currentMillis so that currentMillis - prevMillis gives a "negative" number or, since it is an unsigned long, a very large positive number which always satisfies the if statement.

I've had enough for today. I will experiment more tomorrow.

I suspect that the error associated with doing it the "wrong" way (i.e. "= currentMillis") depends on the ratio between the interval between successive values of millis() and the size of interval for the blinks (or whatever). If the blink interval is relatively large the error may not matter. I haven't explored this with my spreadsheet yet.

...R

Robin2:
I suspect that the error associated with doing it the "wrong" way (i.e. "= currentMillis") depends on the ratio between the interval between successive values of millis() and the size of interval for the blinks (or whatever). If the blink interval is relatively large the error may not matter. I haven't explored this with my spreadsheet yet.

The basic problem is that the code may not evaluate the condition at the precise instant that the interval has elapsed. Suppose for the sake of argument that you were trying to take some action every 100ms and your loop took 5ms to execute. The original code would take up to 5ms to notice that the interval had elapsed. Since the next interval is calculated based on the value of millis() at the point where we notice the interval elapsed, this means the next interval will be delayed by up to 5ms too. In effect, any latency noticing that the condition is true will cause all subsequent processing to slip.

The preferred approach always schedules the next event based on when the current event became due, not when the code noticed that it had occurred. This avoids slippage when there is any latency noticing the event.

1 Like

I eventually figured out the problem while lying in bed.

The problem arises in the servo function because I change the interval after every sweep. As initially written "prevMillis = currentMillis" is at the bottom of the function after the interval has been changed. And when the revised version "prevMillis += interval" is used at the same location it erroneously adds the revised interval. When there is a switch from a short to a long interval that means the prevMillis is actually greater than the next version of currentMillis which gives a "negative" answer to the subtraction (actually a large +ve number) which always evaluates to true.

The solution to that problem is to move "prevMillis += interval" to a position before the interval is changed.

HOWEVER .... (and I'm delighted that this problem gave me time to look at the bigger picture)

FIRST ... @CodingBadly said in Reply #11 that I was assuming that delta (the difference between successive values of millis()) was 1. Of course, as he suspected, I hadn't thought about that at all. BUT ... millis() does in fact increment by 1.

SECOND ... (though, I had forgotten) The code in "BlinkWithouDelay" uses "previousMillis = currentMillis; "

MY CONCLUSION ... is that this is a discussion about angels on the head of a pin. So, in the absence of evidence of a real problem I will not post an updated version of my demo sketch. The present version works and is consistent with BlinkWithoutDelay.

I am wondering if I might write a short accompanying essay that covers the points raised here (this one, the use of Switch and the maintaining State in static local variables).

As always, comments are welcome.

...R

Robin2:
BUT ... millis() does in fact increment by 1.

There you go again making assumptions. This time you assume I made a mistake (and failed to test your assumption).

What about when the processor is running at 8 MHz? What about when the fractional part reaches a whole number?

Robin2:
MY CONCLUSION ... is that this is a discussion about angels on the head of a pin.

Uh huh. Except, of course, when a drifting clock is undesirable.

I made no assumptions, certainly not about your coding ability. I wrote a short sketch to test millis() on my Uno. I have another 328 which I could (maybe already have) set up for 8MHz but I haven't tested that. I also have some Attiny 45's that I could set up for 1MHz. Most beginners for whom my demo may be useful are likely to be using an Uno at 16MHz.

If you know what different increments of millis() are possible in different circumstances I would appreciate it if you would tell me, rather than teasing with questions.

As regards the drifting clock - I started this to produce a more extensive example of "BlinkWithoutDelay" - nothing more. I expect that someone who needs better performance will also know how to get it, or will start a new Thread about it. I don't see any need to complicate matters for newbies. It would be another thing entirely if my code did not do what it claims (and I believe it does).

None of this means that I have not been interested in your comments - I will certainly bear them in mind for my own projects.

...R

Robin2:
MY CONCLUSION ... is that this is a discussion about angels on the head of a pin. So, in the absence of evidence of a real problem I will not post an updated version of my demo sketch. The present version works and is consistent with BlinkWithoutDelay.

I disagree completely - your conclusion is wrong. The technique used in your example is flawed and is vulnerable to slip. The classic 'blink without delay' makes the same mistake (as well as others). It is quite a common mistake, and the fact that it's taking so long to explain it to you demonstrates that it's quite a subtle problem - but it is still a problem.

The problem is that the timing will be subject to cumulative slip when there is any latency between the timed condition becoming true, and the code to detect that executing. As soon as that latency exceeds 1ms, the time of the next scheduled event will be wrong, and so on for all subsequent events. The timing will have slipped by a small amount. It will keep doing this every time there is any latency. There may be some situations where that doesn't matter, and it's even possible to design a sketch that relies on that behaviour, but as a general approach for making something happen at regular intervals this is a problem.

Fortunately the solution is easy: schedule the next event based on when the current event was due, not when it was detected.

Robin2:
If you know what different increments of millis() are possible in different circumstances I would appreciate it if you would tell me, rather than teasing with questions.

Uno running at 16 MHz...

void setup() 
{
  Serial.begin( 115200 );
  Serial.println( F( "Five seconds to lift off..." ) );
}

static unsigned long Mark;
static unsigned long Previous;
static unsigned short Histogram[256];

void loop() 
{
  unsigned long Current;
  unsigned long Delta;
  
  Current = millis();

  Delta = Current - Previous;

  if ( Delta != 0 )
  {
    Previous = Current;

    if ( Delta < 256 )
    {
      ++Histogram[(unsigned char)Delta];
    }
    else
    {
      ++Histogram[255];
    }
    
    return;
  }

  if ( Current - Mark >= 5000ul )
  {
    for ( int i = 0; i < 256; ++i )
    {
      Serial.print( i );
      Serial.write( '\t' );
      Serial.println( Histogram[i] );
    }
    Serial.println();
    while ( true );
  }
}
Five seconds to lift off...
0	0
1	4766
2	117
3	0
4	0
5	0
6	0
7	0
8	0
9	0
10	0
11	0
12	0
13	0
14	0
15	0
16	0
17	0
18	0
19	0
20	0
21	0
22	0
23	0
24	0
25	0
26	0
27	0
28	0
29	0
30	0
31	0
32	0
33	0
34	0
35	0
36	0
37	0
38	0
39	0
40	0
41	0
42	0
43	0
44	0
45	0
46	0
47	0
48	0
49	0
50	0
51	0
52	0
53	0
54	0
55	0
56	0
57	0
58	0
59	0
60	0
61	0
62	0
63	0
64	0
65	0
66	0
67	0
68	0
69	0
70	0
71	0
72	0
73	0
74	0
75	0
76	0
77	0
78	0
79	0
80	0
81	0
82	0
83	0
84	0
85	0
86	0
87	0
88	0
89	0
90	0
91	0
92	0
93	0
94	0
95	0
96	0
97	0
98	0
99	0
100	0
101	0
102	0
103	0
104	0
105	0
106	0
107	0
108	0
109	0
110	0
111	0
112	0
113	0
114	0
115	0
116	0
117	0
118	0
119	0
120	0
121	0
122	0
123	0
124	0
125	0
126	0
127	0
128	0
129	0
130	0
131	0
132	0
133	0
134	0
135	0
136	0
137	0
138	0
139	0
140	0
141	0
142	0
143	0
144	0
145	0
146	0
147	0
148	0
149	0
150	0
151	0
152	0
153	0
154	0
155	0
156	0
157	0
158	0
159	0
160	0
161	0
162	0
163	0
164	0
165	0
166	0
167	0
168	0
169	0
170	0
171	0
172	0
173	0
174	0
175	0
176	0
177	0
178	0
179	0
180	0
181	0
182	0
183	0
184	0
185	0
186	0
187	0
188	0
189	0
190	0
191	0
192	0
193	0
194	0
195	0
196	0
197	0
198	0
199	0
200	0
201	0
202	0
203	0
204	0
205	0
206	0
207	0
208	0
209	0
210	0
211	0
212	0
213	0
214	0
215	0
216	0
217	0
218	0
219	0
220	0
221	0
222	0
223	0
224	0
225	0
226	0
227	0
228	0
229	0
230	0
231	0
232	0
233	0
234	0
235	0
236	0
237	0
238	0
239	0
240	0
241	0
242	0
243	0
244	0
245	0
246	0
247	0
248	0
249	0
250	0
251	0
252	0
253	0
254	0
255	0

2.4% of the time a two was returned otherwise a one is returned. Which passes a basic stink test: 4766 + 2*117 = 5000.

Robin2:
As regards the drifting clock - I started this to produce a more extensive example of "BlinkWithoutDelay" - nothing more. I expect that someone who needs better performance will also know how to get it, or will start a new Thread about it. I don't see any need to complicate matters for newbies. It would be another thing entirely if my code did not do what it claims (and I believe it does).

Perfect. My only concern was that your audience knows what they are getting and, with that, they do.