Program flow question

Using Arduino Programming Questions
1

Hey folks, I've got a little sketch here wherein I'm controlling the speed of three motors with three pots. Under normal operation, it is just that. But I wanted to put an alternate routine in where I just feed constant speed values to all three motors when I flip a switch (Sort of a program mode). As long as the switch is flipped, the program cycles between 4 or 5 "scenes", separated by 3 second intervals. The problem is- the first "if" in the loop is being ignored and the switch doesn't work. No matter if I flip the switch or not, I can't get the sketch to goto the runPGM function. I'm no doubt doing this wrong, but I'm not getting it. Anyone have a suggestion?

Here is the code:

int speedSet1 = A0;
int speedSet2 = A1;
int speedSet3 = A2;
int motor1 = 9;
int motor2 = 10;
int motor3 = 11;
int speedVal1 = 0;
int speedVal2 = 0;
int speedVal3 = 0;
int motor1Speed = 0;
int motor2Speed = 0;
int motor3Speed = 0;
int motor1Limit = 35;
int motor2Limit = 35;
int motor3Limit = 35;
int LED1 = 5;
int LED2 = 6;
int LED3 = 7;
int PGM_SW = 4;
int val = 0;

void setup() {
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(PGM_SW, INPUT);
  Serial.begin(9600);
}

void loop() {
//Check to see if the PGM switch has been switched
  val = digitalRead(PGM_SW);

  if (val != 0) { 
     runPGM;
  }
    
  //Motor 1
  speedVal1 = analogRead(speedSet1);
  speedVal1 = analogRead(speedSet1);
  motor1Speed = map(speedVal1, 0, 1023, 30, 255);
    if (motor1Speed <= motor1Limit)               //If not at lower speed limit..
  {
    digitalWrite(motor1, LOW);                    //Turn off LED and send 0 to motor (Stop)
    digitalWrite(LED1, LOW);
  }
  else                                            //Otherwise...
  {
    digitalWrite(LED1, HIGH);                      //Turn on LED and send PWM to motor
    analogWrite(motor1, motor1Speed);              //according to value from pot
  }
  //Motor 2
  speedVal2 = analogRead(speedSet2);
  speedVal2 = analogRead(speedSet2);
  motor2Speed = map(speedVal2, 0, 1023, 30, 255);
    if (motor2Speed <= motor2Limit)
  {
    digitalWrite(motor2, LOW);
    digitalWrite(LED2, LOW);
  }
  else
  {
    digitalWrite(LED2, HIGH);
    analogWrite(motor2, motor2Speed);
  }
  //Motor 3
  speedVal3 = analogRead(speedSet3);
  speedVal3 = analogRead(speedSet3);
  motor3Speed = map(speedVal3, 0, 1023, 30, 255);
    if (motor3Speed <= motor3Limit)
  {
    digitalWrite(motor3, LOW);
    digitalWrite(LED3, LOW);
  }
  else
  {
    digitalWrite(LED3, HIGH);
    analogWrite(motor3, motor3Speed);
  }
  //Serial.print("motor1Speed = " );
  //Serial.print(motor1Speed);
  //Serial.print(" Switch = ");
  //Serial.println(val);
  //delay(5);
}

void runPGM() {
  do {

  analogWrite(motor1, 90);
  analogWrite(motor2, 50);
  analogWrite(motor3, 140);
  delay (3000);

  analogWrite(motor1, 30);
  analogWrite(motor2, 70);
  analogWrite(motor3, 125);
  delay (3000);

  analogWrite(motor1, 130);
  analogWrite(motor2, 190);
  analogWrite(motor3, 200);
  delay (3000);

  analogWrite(motor1, 170);
  analogWrite(motor2, 100);
  analogWrite(motor3, 80);
  delay (3000);

  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print("motor2Speed = " );
  Serial.print(motor2Speed);
  Serial.print("motor3Speed = " );
  Serial.println(motor3Speed);
  delay(5);

  }

  while (val == 1);
}
2

are you using arduino board or mcu?

edit
if you are using arduino board this is not a fix

i had same problem once, i had "if" at then beginning of loop and it was just ignored.
to fix it i had to set fuses again from my mcu and make sure i choose same settings in arduino ide, not sure why it behaved like that but it fixed it.

3

buttons are typically connected between the pin and ground, the pin configured as INPUT_PULLUP to use the internal pullup resistor which pulls the pin HIGH and when pressed, the button pulls the pin LOW.

a button press can be recognized by detecting a change in state and becoming LOW

how will the value of val ever change without re-reading the pin

why not put the code using the pots in a function and the code not using the pots in a different function and call one or the other function depending on the switch state?

another approach is to just have separate functions to determine the moto values and a common block with loop() that applies those values to the motors

4

Right now, I'm using an Uno board. Ultimately, I'll use just an ATMega328p with bootloader.

5

You defined the switch as

  pinMode(PGM_SW, INPUT);

and I guess you have wired the switch between IO-pin and +5V
With Input-mode INPUT the resistance of the "input" is very high.
So high that even a small piece of wire like 10 cm works as an antenna that catches up so much electromagnetic noise that you have no reliable function.

Best way to avoid this is to use mode

  pinMode(PGM_SW, INPUT_PULLUP);

This enables an microcontroller internal resistor whch connects the IO-pin to +5V
Then you connect your switch between IO-pin and GND

opened switch means you read "HIGH"
closed switch means you read "LOW"

Which is inverted to the logic you use now.

  1. your do-while loop does not read in the switch new.
    There is no chance to leave the do-while-loop because your variable "val" is not updated inside the loop
  do {
    // read in switch INSIDE the do-while-loop
    val = digitalRead(PGM_SW);

    analogWrite(motor1, 90);
    analogWrite(motor2, 50);
  1. your code is blocking through the use of delay
  do {
    // read in switch INSIDE the do-while-loop
    val = digitalRead(PGM_SW);

    analogWrite(motor1, 90);    
    // freeze microcontroller fro 3 seconds react on nothing 
    delay (3000); 

    analogWrite(motor1, 30);
    // freeze microcontroller fro 3 seconds react on nothing 
    delay (3000);

    analogWrite(motor1, 130);
    // freeze microcontroller fro 3 seconds react on nothing 
    delay (3000);

    analogWrite(motor3, 80);
    // freeze microcontroller fro 3 seconds react on nothing 
    delay (3000);

    Serial.print("motor1Speed = " );
    delay(5);
  }

This means it will take up to 12 seconds until flipping the switch is recognised by the code.

If you want instant reaction on flipping the switch your code must be modified to be non-blocking.
This means you have to change from linear code-execution with INNER loops
to
circular code-execution with ONE single OUTER-loop()

To earn nonblocking coding would be very good. To understand the basic concept wil take 1 or 2 hours. (not only minutes). Applying the non-blocking principle will take 2 to 4 hours. So it is not a 5 to 10 minutes quick-shot.

  1. you should use self-explaining names for each and everything a variable named "val" says almost nothing. What does the value represent? What is it used for?

A name like "constantRpmState" would point to what the variable is used for
"const_or_variable_Speed" to say both

best regards Stefan

6

This must have been a random coincidence. I don't believe that changing fuses would make an if-condition work. Except your if-condition uses things that depend on that fuse-bit.

7

was kinda suprised my self that it started working after that, i had "if" to check button for enabling debugging, and spent like 3hours looking at my code not understanding whats wrong and then just tried to set fuses again as i was outof ideas whats wrong, and after that it worked.

8

Still not convinced that this was the real cause
Did you change something in the wiring?
A simple wire is 3cm more away from a another wire sometimes can be enough to make the difference

What microcontroller? What fuse-bits?
What does the code look like?

9

i had "atmega168" soldered on prototyping circuit board so wiring was all times exactly same.
i set fuses with calculator to 8mhz internal and brown out to off,

the code then was different tho because i used at first "IRsmallDecoder.h" library instead of "IRremote.hpp"

and the if-condition for "IR" had inside "if(debug == 1)" then serial.print(IR DATA)
didn't get any serial output until i set fuses again

also the code still isn't ready, im kinda lazy with this stuff so i do it like every other week

#include <Adafruit_NeoPixel.h>


#define NUMPIXELS 300

//#define led1 PB6
#define led2 PD3
#define debug PD4

#define Dpin1 PB1
#define Dpin2 PB2
#define Dpin3 PB3
#define Dpin4 PB4

#define vcc PC0
#define ldr PC2

#define DECODE_NEC

#include <IRremote.hpp>



Adafruit_NeoPixel strip1(NUMPIXELS, Dpin1, NEO_GRB + NEO_KHZ800);
Adafruit_NeoPixel strip2(NUMPIXELS, Dpin2, NEO_GRB + NEO_KHZ800);
Adafruit_NeoPixel strip3(NUMPIXELS, Dpin3, NEO_GRB + NEO_KHZ800);
Adafruit_NeoPixel strip4(NUMPIXELS, Dpin4, NEO_GRB + NEO_KHZ800);



uint8_t command[28] = {0x50, 0x8, 0xB, 0xF0, 0xF20, 0xA, 0x9, 0x11, 0x12, 0x13,
                       0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x53, 0x10, 0x1A,
                       0x43, 0x40, 0x5B, 0x7, 0x44, 0x6, 0xAA, 0x41, 0xAB
                      };

void setup() {
  //pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(debug, INPUT);

  pinMode(Dpin1, OUTPUT);
  pinMode(Dpin2, OUTPUT);
  pinMode(Dpin3, OUTPUT);
  pinMode(Dpin4, OUTPUT);

  pinMode(vcc, INPUT);
  pinMode(ldr, INPUT);



IrReceiver.begin(2);


  Serial.begin(9600);
  
    if (digitalRead(debug) == HIGH) {
      digitalWrite(led1, HIGH);
      digitalWrite(led2, HIGH);

      //Serial.begin(9600);
      delay(3000);
      Serial.print("DEBUGGING ON!");
    } else if (digitalRead(debug) == LOW and firstloop == 0) {
      digitalWrite(led1, HIGH);
    }
  
  /*
  strip1.begin();
  strip2.begin();
  strip3.begin();
  strip4.begin();

  strip1.clear();
  strip2.clear();
  strip3.clear();
  strip4.clear();
*/

}

void loop() {

  /*
    strip1.clear();

    for (int i = 0; i < NUMPIXELS; i++) {


      strip1.setPixelColor(i, strip1.Color(0, 150, 0));

      strip1.show();

      delay(500);
    }
  */


////////////////////////////////// if-condition for IR
if (IrReceiver.decode()) {

        /*
         * Print a short summary of received data
         */
        IrReceiver.printIRResultShort(&Serial);
        IrReceiver.printIRSendUsage(&Serial);
        if (IrReceiver.decodedIRData.protocol == UNKNOWN) {
            Serial.println(F("Received noise or an unknown (or not yet enabled) protocol"));
            // We have an unknown protocol here, print more info
            IrReceiver.printIRResultRawFormatted(&Serial, true);
        }
        Serial.println();

        /*
         * !!!Important!!! Enable receiving of the next value,
         * since receiving has stopped after the end of the current received data packet.
         */
        IrReceiver.resume(); // Enable receiving of the next value

        /*
         * Finally, check the received data and perform actions according to the received command
         */
        if (IrReceiver.decodedIRData.command == 0x10) {
            // do something
        } else if (IrReceiver.decodedIRData.command == 0x11) {
            // do something else
        }
    }


  
}
10

Thanks, guys! The sketch works a little better after applying your suggestions. I can switch easily between the two modes (only having to wait 3 seconds after the switch is flipped instead of 12) Except now, when I flip the switch and go into "demo" mode, the program is not sending the values I've specified. Instead, all three values are near max (250 and above). I was wondering if I have to map any values like I do in variable mode, but figured no because I'm just trying to send constant integers through analogWrite. It should work, but it's not...
Here is the revised code:

int speedSet1 = A0;
int speedSet2 = A1;
int speedSet3 = A2;
int motor1 = 9;
int motor2 = 10;
int motor3 = 11;
int speedVal1 = 0;
int speedVal2 = 0;
int speedVal3 = 0;
int motor1Speed = 0;
int motor2Speed = 0;
int motor3Speed = 0;
int motor1Limit = 35;
int motor2Limit = 35;
int motor3Limit = 35;
int LED1 = 5;
int LED2 = 6;
int LED3 = 7;
int PGM_SW = 4;
int val = 0;

void setup() {
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(PGM_SW, INPUT_PULLUP);
  Serial.begin(9600);
}
void varPots() {   //varable control of the motors    
  //Motor 1
  speedVal1 = analogRead(speedSet1);
  speedVal1 = analogRead(speedSet1);
  motor1Speed = map(speedVal1, 0, 1023, 30, 255);
    if (motor1Speed <= motor1Limit)               //If not at lower speed limit..
  {
    digitalWrite(motor1, LOW);                    //Turn off LED and send 0 to motor (Stop)
    digitalWrite(LED1, LOW);
  }
  else                                            //Otherwise...
  {
    digitalWrite(LED1, HIGH);                      //Turn on LED and send PWM to motor
    analogWrite(motor1, motor1Speed);              //according to value from pot
  }
  //Motor 2
  speedVal2 = analogRead(speedSet2);
  speedVal2 = analogRead(speedSet2);
  motor2Speed = map(speedVal2, 0, 1023, 30, 255);
    if (motor2Speed <= motor2Limit)
  {
    digitalWrite(motor2, LOW);
    digitalWrite(LED2, LOW);
  }
  else
  {
    digitalWrite(LED2, HIGH);
    analogWrite(motor2, motor2Speed);
  }
  //Motor 3
  speedVal3 = analogRead(speedSet3);
  speedVal3 = analogRead(speedSet3);
  motor3Speed = map(speedVal3, 0, 1023, 30, 255);
    if (motor3Speed <= motor3Limit)
  {
    digitalWrite(motor3, LOW);
    digitalWrite(LED3, LOW);
  }
  else
  {
    digitalWrite(LED3, HIGH);
    analogWrite(motor3, motor3Speed);
  }
  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print(" Switch = ");
  Serial.println(val);
  delay(5);
}

void runPGM() {   //Program domo mode
  do {

  val = digitalRead(PGM_SW);
  if (val == 1) {
    varPots();
  }
  analogWrite(motor1, 90);
  analogWrite(motor2, 50);
  analogWrite(motor3, 140);
  delay (3000);

  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print("motor2Speed = " );
  Serial.print(motor2Speed);
  Serial.print("motor3Speed = " );
  Serial.println(motor3Speed);
  delay(5);
  
  val = digitalRead(PGM_SW);
  if (val == 1) {
    varPots();
  }
  
  analogWrite(motor1, 30);
  analogWrite(motor2, 70);
  analogWrite(motor3, 125);
  delay (3000);

  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print("motor2Speed = " );
  Serial.print(motor2Speed);
  Serial.print("motor3Speed = " );
  Serial.println(motor3Speed);
  delay(5);
  
  val = digitalRead(PGM_SW);
  if (val == 1) {
    varPots();
  }
  
  analogWrite(motor1, 130);
  analogWrite(motor2, 190);
  analogWrite(motor3, 200);
  delay (3000);

  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print("motor2Speed = " );
  Serial.print(motor2Speed);
  Serial.print("motor3Speed = " );
  Serial.println(motor3Speed);
  delay(5);
  
  val = digitalRead(PGM_SW);
  if (val == 1) {
    varPots();
  }
  
  analogWrite(motor1, 170);
  analogWrite(motor2, 100);
  analogWrite(motor3, 80);
  delay (3000);

  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print("motor2Speed = " );
  Serial.print(motor2Speed);
  Serial.print("motor3Speed = " );
  Serial.println(motor3Speed);
  delay(5);
  
  val = digitalRead(PGM_SW);
  if (val == 1) {
    varPots();
  }
  }

  while (val == 0);
}



void loop() {
//Check to see if the PGM switch has been switched
  val = digitalRead(PGM_SW);

  if (val != 1) { 
     runPGM();
  }
     else
     varPots();
  
}
11

Hello

Yes, I´v made a small code review.

In general - Arrays and structs are your friends.
Don't duplicate code in your sketch. Write code once - use it multiple times.

Do you have experience with programming in C++?

The task can easily be realised with an object.
A structured array contains all information, such as pin addresses for the I/O devices, as well as the information for the timing.
A single service takes care of this information and initiates the intended action.
The structured array makes the sketch scalable until all I/O pins are used up without having to adapt the code for the service.
It is cool stuff, isn´t it?

Have a nice day and enjoy coding in C++.

12

What Serial output do you get?

13

Well, when the switch is ON (logic 0), the serial monitor shows the numbers being sent to all three motors. When the switch is OFF (logic 1), it shows the number being sent to motor #1 and the state of the switch (1 or 0).

14

Can you post that output (in code tags please) and indicate how you know it isn't working?

16 
motor1Speed = 81 Switch = 1   //switch off
motor1Speed = 122 Switch = 1
motor1Speed = 130 Switch = 1   
motor1Speed = 130motor2Speed = 132motor3Speed = 123  //switch on
motor1Speed = 130motor2Speed = 132motor3Speed = 123
motor1Speed = 130motor2Speed = 132motor3Speed = 123
motor1Speed = 130motor2Speed = 132motor3Speed = 123
motor1Speed = 130motor2Speed = 132motor3Speed = 123
motor1Speed = 130motor2Speed = 132motor3Speed = 123
motor1Speed = 130motor2Speed = 132motor3Speed = 123
motor1Speed = 130motor2Speed = 132motor3Speed = 123
motor1Speed = 117 Switch = 1     //switch off
motor1Speed = 121 Switch = 1
motor1Speed = 114 Switch = 1
17

Your code does exactly what you have written the code to do
Your function varPots only prints this

void varPots() {   //varable control of the motors
  //Motor 1
  speedVal1 = analogRead(speedSet1);
  //... 
  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print(" Switch = ");
  Serial.println(val);
  delay(5);
}

calling the function varPots from inside function runPGM()
is possible but then you won't need to call it from function loop()

Without re-writing your code to be non-blocking you can't go byond what you have now
a delay() should better have the name

freeze_microcontroller_completely_react_on_nothing_until_freezing_time_is_over()

Here is a first introduction into non-blocking code

It needs more to make your code non-blocking.
But it is very important to understand the fundamental difference between
linear executed blocking code
and
circular executed non-blocking code

best regards Stefan

18

The problem with your misleading serial output is that it prints the value of your motor speed variables. In runPGM, they don't get set so although the analogWrite is being changed, what you print is what came from varPots.

19

Ah, I see it now. So, in order to monitor the numbers I send, I need to somehow objectify the numbers in each analogWrite command and specify those objects in the serial.print commands. Is that even possible?

20

Yes. Set motor1Speed and its siblings and use them in the analogWrite instead of constants. Then the prints will show the number you actually used for the write.

21

The issue you're facing is due to the fact that you're not updating the motor1Speed, motor2Speed, and motor3Speed variables in your runPGM() function. These variables are used to control the speed of the motors, but since you're not updating them, they remain at their initial values (0).

To fix this, you need to update these variables with the values you're sending to the motors. Here's how you can modify your runPGM() function:

void runPGM() {   //Program domo mode
  do {

  val = digitalRead(PGM_SW);
  if (val == 1) {
    varPots();
  }
  motor1Speed = map(90, 0, 1023, 30, 255);
  motor2Speed = map(50, 0, 1023, 30, 255);
  motor3Speed = map(140, 0, 1023, 30, 255);
  analogWrite(motor1, motor1Speed);
  analogWrite(motor2, motor2Speed);
  analogWrite(motor3, motor3Speed);
  delay (3000);

  // ...

  motor1Speed = map(170, 0, 1023, 30, 255);
  motor2Speed = map(100, 0, 1023, 30, 255);
  motor3Speed = map(80, 0, 1023, 30, 255);
  analogWrite(motor1, motor1Speed);
  analogWrite(motor2, motor2Speed);
  analogWrite(motor3, motor3Speed);
  delay (3000);

  // ...

  }

  while (val == 0);
}

In this modified version, I've added the map() function to convert the constant values (90, 50, 140, etc.) to the range of values that analogWrite() expects (30-255). The result is then stored in the motor1Speed, motor2Speed, and motor3Speed variables. These variables are then used to control the speed of the motors.

here is the complete code:

int speedSet1 = A0;
int speedSet2 = A1;
int speedSet3 = A2;
int motor1 = 9;
int motor2 = 10;
int motor3 = 11;
int speedVal1 = 0;
int speedVal2 = 0;
int speedVal3 = 0;
int motor1Speed = 0;
int motor2Speed = 0;
int motor3Speed = 0;
int motor1Limit = 35;
int motor2Limit = 35;
int motor3Limit = 35;
int LED1 = 5;
int LED2 = 6;
int LED3 = 7;
int PGM_SW = 4;
int val = 0;

void setup() {
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(PGM_SW, INPUT_PULLUP);
  Serial.begin(9600);
}

void varPots() {   //varable control of the motors    
  //Motor 1
  speedVal1 = analogRead(speedSet1);
  speedVal1 = analogRead(speedSet1);
  motor1Speed = map(speedVal1, 0, 1023, 30, 255);
    if (motor1Speed <= motor1Limit)               //If not at lower speed limit..
  {
    digitalWrite(motor1, LOW);                    //Turn off LED and send 0 to motor (Stop)
    digitalWrite(LED1, LOW);
  }
  else                                            //Otherwise...
  {
    digitalWrite(LED1, HIGH);                      //Turn on LED and send PWM to motor
    analogWrite(motor1, motor1Speed);              //according to value from pot
  }
  //Motor 2
  speedVal2 = analogRead(speedSet2);
  speedVal2 = analogRead(speedSet2);
  motor2Speed = map(speedVal2, 0, 1023, 30, 255);
    if (motor2Speed <= motor2Limit)
  {
    digitalWrite(motor2, LOW);
    digitalWrite(LED2, LOW);
  }
  else
  {
    digitalWrite(LED2, HIGH);
    analogWrite(motor2, motor2Speed);
  }
  //Motor 3
  speedVal3 = analogRead(speedSet3);
  speedVal3 = analogRead(speedSet3);
  motor3Speed = map(speedVal3, 0, 1023, 30, 255);
    if (motor3Speed <= motor3Limit)
  {
    digitalWrite(motor3, LOW);
    digitalWrite(LED3, LOW);
  }
  else
  {
    digitalWrite(LED3, HIGH);
    analogWrite(motor3, motor3Speed);
  }
  Serial.print("motor1Speed = " );
  Serial.print(motor1Speed);
  Serial.print(" Switch = ");
  Serial.println(val);
  delay(5);
}

void runPGM() {   //Program domo mode
  do {

  val = digitalRead(PGM_SW);
  if (val == 1) {
    varPots();
  }
  motor1Speed = map(90, 0, 1023, 30, 255);
  motor2Speed = map(50, 0, 1023, 30, 255);
  motor3Speed = map(140, 0, 1023, 30, 255);
  analogWrite(motor1, motor1Speed);
  analogWrite(motor2, motor2Speed);
  analogWrite(motor3, motor3Speed);
  delay (3000);

  // ...

  motor1Speed = map(170, 0, 1023, 30, 255);
  motor2Speed = map(100, 0, 1023, 30, 255);
  motor3Speed = map(80, 0, 1023, 30, 255);
  analogWrite(motor1, motor1Speed);
  analogWrite(motor2, motor2Speed);
  analogWrite(motor3, motor3Speed);
  delay (3000);

  // ...

  }

  while (val == 0);
}

void loop() {
//Check to see if the PGM switch has been switched
  val = digitalRead(PGM_SW);

  if (val != 1) { 
     runPGM();
  }
     else
     varPots();
  
}

This code should now correctly send the values you've specified in the runPGM() function to the motors, and update the motor1Speed, motor2Speed, and motor3Speed variables accordingly.