for loop reads pots, passes value to PWM pins, can this be in the same for loop?

Question

In the code below I am using a for loop to iterate over analog pins (potentiometers) and pass their value, after being mapped, to PWM enabled analog output pins (currently controlling LED brightness). While this works as is, I am curious about the following: would it be better to split the analogRead and analogWrite lines into separate for loops, or would that simply be redundant from a code standpoint? My concern is accidentally mixing up the pot values due to difficult-to-perceive timing issues.

The code:

static const uint8_t analog_pins[] = {A0, A1, A2, A3};
static const uint8_t digital_pins[] = {3, 5, 6, 9};
int pot_vals[] = {0, 0, 0, 0};
int val[] = {0, 0, 0, 0};
void setup() {
 Serial.begin(9600);
 // setup inputs, outputs
 for ( int i = 0; i < 4; i++ ) {
 pinMode( analog_pins[i], INPUT );
 pinMode( digital_pins[i], OUTPUT );
 }
}
void loop() {
 for (int i = 0; i < 4; i++) {
 val[i] = analogRead(analog_pins[i]);
 analogWrite(digital_pins[i], map( val[i], 0, 1023, 0, 255 ));
 }
 delay(2);
}

• goddland_16: Analogue pins are configured as input by default. It is unnecessary to declare them in setup.

Answer 1

Having both the analogRead() and analogWrite() in the same loop is perfectly fine. That would be be my preferred choice if only for one reason: less RAM consumption. If you split them in two loops, you have to store somewhere the readings of the four channels, either before or after mapping. If you keep them in the same loop, you don't need the pot_vals[] nor val[] arrays. You would use a local variable instead, which consumes no RAM:

void loop() {
 for (int i = 0; i < 4; i++) {
 int val = analogRead(analog_pins[i]);
 analogWrite(digital_pins[i], map(val, 0, 1023, 0, 255));
 }
 delay(2);
}

My concern is accidentally mixing up the pot values due to difficult-to-perceive timing issues.

"Difficult to perceive"? I would call these issues "imaginary" instead. You have no risk of "mixing" any values here. Maybe, if you can elaborate on the kind of issues you imagine you could have, we could help you clear some misunderstanding about how the microcontroller executes your code.

---

Edit: To expand on Andrew's comment, the for loop can be optimized as

for (int i = 0; i < 4; i++) {
 unsigned int val = analogRead(analog_pins[i]);
 analogWrite(digital_pins[i], val / 4);
}

Dividing the value by 4 is the best way to map the input range to the output range. It gives a more uniform mapping than the map() call above, as every output value is mapped to exactly 4 input values. It could be noted, though, that map(val, 0, 1024, 0, 256) gives the same uniform mapping.

The division by 4, however, is optimized by the compiler into a bit shift, which is orders of magnitude faster than map(), and likely to consume less stack space. There is one caveat to be aware of: this optimization is only possible if val is unsigned. If it is signed, then it is compiled into a call to a division routine, which is quite expensive.

One could be tempted to go even further and get rid of the temporary val variable, like:

for (int i = 0; i < 4; i++) {
 analogWrite(digital_pins[i], (unsigned int) analogRead(analog_pins[i]) / 4);
}

However, there is zero benefit from doing so: from the point of view of an optimizing compiler, this form is exactly equivalent to the previous one. I prefer the former which I deem more readable.

• Andrew: If you want to really simplify the code and minimize memory usage then you could even do: for (int i = 0; i < 4; i++) analogWrite(digital_pins[i], analogRead(analog_pins[i])/4);
• Edgar Bonet: @Andrew: Your comment is both right and misleading: a naive reader may wrongly think that your not using the intermediate val variable is what reduces the memory footprint.

Answer 2

Obviously you can. Except that in most cases your loop execution is much faster than your own period so that you end up changing the duty cycles multiple times in one period.

Not an advisable aporoach. If you have to do it think about configuring the pwm center aligned.