NEWB Help If/Else Statement

dereckbc · Nov 6, 2016

OK I have a short piece of code I wrote and not working like it should. or the way I think it should work and I cannot figure out what I did wrong. It compiles, just does not do much of anything except Serial Print two integers that never change value.

The code is pretty simple. I want to use Digital Pins 20, 21, and 22 as Inputs with Pullup to set them as High or 1. I am using 2 switches. One on/off switch on Pin 20 to set var AI_GEAR to either 800 or 2200. The second switch is a SPDT-Center Off, and is used to change var AI_AUX to 800, 1500, or 2200. Both switches have Ground connected to the common contact to togle the digital pins low or 0. Then all the program does is Print the vars AI_GEAR and AI_AUX all day long. Unfortunately all it does is Print:

AI_GEAR = 800
AI_AUX = 2200

All day long. The values do not change as I ground pins 20, 21, and 22. Where did I go wrong?

Code:

// Initialize digital pins 20, 21, and 22 as integers

int D1_GEAR = 20; //
int D1_AUX = 21; //
int D2_AUX = 22; // 

// Initialize vars

int AI_GEAR = 800; 
int AI_AUX = 800; 



void setup() {

// set digital pins 20, 21, and 22 as Inputs and Pullup.

pinMode(D1_GEAR, INPUT_PULLUP);
pinMode(D1_AUX, INPUT_PULLUP);
pinMode(D2_AUX, INPUT_PULLUP);

  
Serial.begin(9600); // initialize serial communications:
}

void loop() {

//Set Gear channel to 800 or 2200

if (D1_GEAR == 0)
{AI_GEAR = 800;}
  else {AI_GEAR = 2200;}

//Set Aux channel to either 800, 1500, or 2200

if (D1_AUX == LOW && D2_AUX == LOW) 
{AI_AUX = 1500;}
else if (D1_AUX == LOW && D2_AUX == HIGH)
{AI_AUX = 800;}
else if (D1_AUX == HIGH && D2_AUX == LOW)
{AI_AUX = 2200;}
else if (D1_AUX == HIGH && D2_AUX == HIGH)
{AI_AUX = 800;}
  
//Print Gear and Aux channel values 

  Serial.print("AI_GEAR =");
  Serial.print(AI_GEAR);
  Serial.print("AI_AUX =");
  Serial.print(AI_AUX);  
  delay(1000);  // delay in between reads for stability
  }

Frank B · Nov 6, 2016

I guess, using digitalRead( D1_AUX ) can be helpful

oops..edited.. lol..

Cosford · Nov 6, 2016

Fleshing out Frank's reply a little, I think you may have some confusion with the way the pins work.

What you've done initially is define variables for D1_GEAR, D1_AUX and D2_AUX, containing the pin numbers of those pins (20, 21 and 22 respectively).
You've then initialised those physical pins to be inputs. All good, that's all fine.

In the start of your loop, you call this:

Code:

if (D1_GEAR == 0)
{AI_GEAR = 800;}
  else {AI_GEAR = 2200;}

This is where the confusion becomes apparent; D1_GEAR is storing the pin number of the D1_GEAR pin, not the digital value read by the pin itself. In order to access the value of the pin, you need to call something like:

Code:

int d1_reading = digitalRead(D1_GEAR);

Now, d1_reading will store the value of either 0 or 1 (which is equivalent to LOW and HIGH respectively), depending on the state of the D1_GEAR pin.

If you've followed me up to here, you should be able to work out how to fix the rest of your code, I'll leave that to you as a learning exercise though.

dereckbc · Nov 6, 2016

Cosford said:
Fleshing out Frank's reply a little, I think you may have some confusion with the way the pins work.

Thank you. You can say that again.

Cosford said:
This is where the confusion becomes apparent; D1_GEAR is storing the pin number of the D1_GEAR pin, not the digital value read by the pin itself. In order to access the value of the pin, you need to call something like:

Code:

int d1_reading = digitalRead(D1_GEAR);

Now, d1_reading will store the value of either 0 or 1 (which is equivalent to LOW and HIGH respectively), depending on the state of the D1_GEAR pin.

If you've followed me up to here, you should be able to work out how to fix the rest of your code, I'll leave that to you as a learning exercise though.

OK this made perfect sense at first, but I must be having a dumb arse attack. Here is what I came up with:

Code:

//Initialize Digital Pins 20, 21, and 22 as variables

int D1_GEAR = 20; //
int D1_AUX = 21; //
int D2_AUX = 22; // 

//Initialize Gear and Aux channel vars
int AI_GEAR = 800; // Ana In var - 0->2047 compensated
int AI_AUX = 800; // Ana In var - 0->2047 compensated



void setup() {

//Set Digital Pins 20, 21, and 22 as INPUT and PULLUP

pinMode(D1_GEAR, INPUT_PULLUP);
pinMode(D1_AUX, INPUT_PULLUP);
pinMode(D2_AUX, INPUT_PULLUP);

//TEST 

Serial.begin(9600); // initialize serial communications:
}

void loop() {

// Read Digital Pins 20, 21, and 22

int D1_GEAR = digitalRead(D1_GEAR); //Read Pin 20 Input
int D1_AUX = digitalRead(D1_AUX); //Read Pin 21 Input
int D2_AUX = digitalRead(D2_AUX); //Read Pin 22 Input

// Set Gear Channel to 800 or 2200

if (D1_GEAR == 0)
{AI_GEAR = 800;}
else {AI_GEAR = 2200;}

//Set Aux channel to either 800, 1500, or 2200

if (D1_AUX == LOW && D2_AUX == LOW) 
{AI_AUX = 1500;}
else if (D1_AUX == LOW && D2_AUX == HIGH)
{AI_AUX = 800;}
else if (D1_AUX == HIGH && D2_AUX == LOW)
{AI_AUX = 2200;}
else if (D1_AUX == HIGH && D2_AUX == HIGH)
{AI_AUX = 800;}

//Test, print values of Gear and Aux channels

Serial.print("AI_GEAR =");
Serial.print(AI_GEAR);
Serial.print("AI_AUX =");
Serial.print(AI_AUX);  
delay(1000);        // delay in between reads for stability
} // End Loop

Unfortunately still not right. It prints:

A1_Gear = 1500
AI_Aux = 2200

Does not change when I ground any of the Digital Pins 20, 21, ans 22. Sigh!

Frank B · Nov 6, 2016

Try something like this:

Code:

const int PIN_D1_GEAR = 20; //
const int PIN_D1_AUX = 21; //
const int PIN_D2_AUX = 22; // 

//Initialize Gear and Aux channel vars
int AI_GEAR = 800; // Ana In var - 0->2047 compensated
int AI_AUX = 800; // Ana In var - 0->2047 compensated



void setup() {

//Set Digital Pins 20, 21, and 22 as INPUT and PULLUP

pinMode(PIN_D1_GEAR, INPUT_PULLUP);
pinMode(PIN_D1_AUX, INPUT_PULLUP);
pinMode(PIN_D2_AUX, INPUT_PULLUP);

//TEST 

Serial.begin(9600); // initialize serial communications:
}

void loop() {

// Read Digital Pins 20, 21, and 22

int D1_GEAR = digitalRead(PIN_D1_GEAR); //Read Pin 20 Input
int D1_AUX = digitalRead(PIN_D1_AUX); //Read Pin 21 Input
int D2_AUX = digitalRead(PIN_D2_AUX); //Read Pin 22 Input

// Set Gear Channel to 800 or 2200

if (D1_GEAR == 0)
{AI_GEAR = 800;}
else {AI_GEAR = 2200;}

//Set Aux channel to either 800, 1500, or 2200

if (D1_AUX == LOW && D2_AUX == LOW) 
{AI_AUX = 1500;}
else if (D1_AUX == LOW && D2_AUX == HIGH)
{AI_AUX = 800;}
else if (D1_AUX == HIGH && D2_AUX == LOW)
{AI_AUX = 2200;}
else if (D1_AUX == HIGH && D2_AUX == HIGH)
{AI_AUX = 800;}

//Test, print values of Gear and Aux channels

Serial.print("AI_GEAR =");
Serial.print(AI_GEAR);
Serial.print("AI_AUX =");
Serial.print(AI_AUX);  
delay(1000);        // de
}

(Attention, not tested, there might be a bug or two)

dereckbc · Nov 6, 2016

Frank B said:
Try something like this:

DOH!

Thank you Frank, I did not expect anyone to just spell it out for me. I appreciate it. Took me a minute to figure out what you did different. So allow me say what I think each block of code does. First you changed the names or pins 20, 21, and 22, set them as Constants, and gave each pin a Name? I good with that and understand. I assume you renamed them to be more descriptive with the word PIN? If that is accurate, I got it.

Code:

const int PIN_D1_GEAR = 20; //
const int PIN_D1_AUX = 21; //
const int PIN_D2_AUX = 22; //

Next you left this alone, but I do have a question. Is setting the default to 800 problematic say vs 0? I used 800 because in the event of missing reading it writes 800 which is Gear Down. So I have wheels down to land on if the channel fails. Pretty much the same reason I set Aux to 800 default.

Code:

//Initialize Gear and Aux channel vars
int AI_GEAR = 800; // Ana In var - 0->2047 compensated
int AI_AUX = 800; // Ana In var - 0->2047 compensated

Next is the Pin Mode. Got it you just changed the names to reflect the changes in the Constant statements.

Code:

pinMode(PIN_D1_GEAR, INPUT_PULLUP);
pinMode(PIN_D1_AUX, INPUT_PULLUP);
pinMode(PIN_D2_AUX, INPUT_PULLUP);

OK this is where I went off track. You declared 3 new Variables, read the digital Pins to set the variable value.

Code:

int D1_GEAR = digitalRead(PIN_D1_GEAR); //Read Pin 20 Input
int D1_AUX = digitalRead(PIN_D1_AUX); //Read Pin 21 Input
int D2_AUX = digitalRead(PIN_D2_AUX); //Read Pin 22 Input

Thank you Frank.

dereckbc · Nov 7, 2016

Frank FWIW if you should check back, I shortened and cleaned up the code based on what I learned from you. I took out the variables you had in the main loop, they are not needed. In fact about 1/4 of the code I had is gone. Still looking for other routes to make it even more simple. Here is how it looks now. Made it easier to follow.

Thanks again Frank, and if you or anyone sees room for improvement speak up please.

Code:

 #include <PulsePosition.h>

  PulsePositionOutput PPMOut(FALLING); //PPM signal output Falling Leading Edge

//Initialize and Name Analog Input Pins A0 to A4, and Digital Pins 20 to 22, 
  const int inPin_A0_Ael = A0; // Ana Input - Aeleron potentiometer 
  const int inPin_A1_Ele = A1; // Ana Input - Elevator potentiometer 
  const int inPin_A2_Thr = A2; // Ana Input - Throttle potentiometer
  const int inPin_A3_Rud = A3; // Ana Input - Rudder potentiometer
  const int inPin_D20_Gear_Sw = 20; // Gear Input Switch
  const int inPin_D21_Aux_Sw1 = 21; // Aux Input Switch 1
  const int inPin_D22_Aux_Sw2 = 22; // Aux Input Switch 2

  int led = 13; // initiate digital pin 13 LED "Power On Indicator"

//Declare MAP varibles in Us. Used to "Write" to PPM Output channels in Us (micro-seconds)
  int Ael_Us = 0; // 800 to 2200 Us
  int Ele_Us = 0; // 800 to 2200 Us
  int Thr_Us = 0; // 800 to 2200 Us
  int Rud_Us = 0; // 800 to 2200 Us
  int Gear_Us = 800; // Switch, 800 Us Gear Down, 2200 Us Gear Up
  int Aux_Us = 1500; // 3-way Switch, Cneter-Off. 800 Us, 1500 Us, 2200Us


void setup() {
  
  PPMOut.begin(10);  // Starts PPM Output, Pin 10, TX to Trainer Port Radio Input

  analogReadResolution(11);// Sets ADC range to 11 bits from 0 to 2043. Teensy 3.2 range is 10 to 13 bits
  analogReadAveraging(8);//Averagge 8 readings, smooths stick input and jitter. Options are 4, 8, 16, and 32 samples, no delay

  pinMode(inPin_D20_Gear_Sw, INPUT_PULLUP); //Set Pin D20 to Input and logic 1
  pinMode(inPin_D21_Aux_Sw1, INPUT_PULLUP); //Set Pin D21 to Input and logic 1
  pinMode(inPin_D22_Aux_Sw2, INPUT_PULLUP); //Set Pin D22 to Input and logic 1
  pinMode(led, OUTPUT); //Set up LED for Power On indicator
  digitalWrite(led, HIGH); //Turn On power indicator LED Pin 13

void loop() {

// Convert Analog Input Voltage 11-bit input to 800 to 2200 uS. Change to match your Radio timing requirements. Standard = 1000 to 2000 Us
  Ael_Us = map(analogRead(inPin_A0_Ael), 0, 2047, 800, 2200); // Invert Aeleron pot and slight centre offset
  Ele_Us = map(analogRead(inPin_A1_Ele), 0, 2047, 800, 2200); // Invert Elevator pot and slight centre offset
  Thr_Us = map(analogRead(inPin_A2_Thr), 0, 2047, 800, 2200); // Throttle
  Rud_Us = map(analogRead(inPin_A3_Rud), 0, 2047, 800, 2200); // Rudder

//Set Gear Us
  if (digitalRead(inPin_D20_Gear_Sw) == 0)
    {Gear_Us = 800;}
    else {Gear_Us = 2200;}

//Set Aux Us
  if (digitalRead(inPin_D21_Aux_Sw1) == LOW && digitalRead(inPin_D22_Aux_Sw2) == LOW) 
    {Aux_Us = 1500;}
      else if (digitalRead(inPin_D21_Aux_Sw1) == LOW && digitalRead(inPin_D22_Aux_Sw2) == HIGH)
      {Aux_Us = 800;}
        else if (digitalRead(inPin_D21_Aux_Sw1) == HIGH && digitalRead(inPin_D22_Aux_Sw2) == LOW)
        {Aux_Us = 2200;}
          else if (digitalRead(inPin_D21_Aux_Sw1) == HIGH && digitalRead(inPin_D22_Aux_Sw2) == HIGH)
          {Aux_Us = 1500;}
 
//PPM Output, pin 10, to TX Radio Trainer Port. 1 to 16 channels max as needed. 
  PPMOut.write(1, Ael_Us); //Channel 1
  PPMOut.write(2, Ele_Us); //Channel 2
  PPMOut.write(3, Thr_Us); //Channel 3
  PPMOut.write(4, Rud_Us); //Chanel 4
  PPMOut.write(5, Gear_Us); //Channel 5
  PPMOut.write(6, Aux_Us); //Channel 6

  } //End of Proggram

Frank B · Nov 7, 2016

Great

For now, that's good!

Later, you might want to use variables again (instead of all the digitalReads) - but that comes with more experience.
The issue is - there might be cases where a pins changes its state between two reads.

But for now, leave it as it is. The simpler (and the better readable for YOU!) the better.
Simpler code -> less bugs. That's a lesson, most "programmers" learn late....

dereckbc · Nov 7, 2016

Frank B said:
The issue is - there might be cases where a pins changes its state between two reads.

Not sure how that would be an issue, nature of the beast. This is a Flight Stick (joystick) with constant movement of the controls. The PPM Encoder sends the variables 50 times a second. RC Plane radios use 2-Thumb joysticks, and I really dislike them. A Flight Stick is just more intuitive and natural for me. So I expect the variables to change between reads. Now to take care of Noise and minimize read errors I am using analogReadAverage to Smooth the variables. At least I think I am if I set the code correctly and understand how it works.

Am I over looking something? I am a complete newb, first project.

NEWB Help If/Else Statement

dereckbc

Well-known member

Frank B

Senior Member

Cosford

Well-known member

dereckbc

Well-known member

Frank B

Senior Member

dereckbc

Well-known member

dereckbc

Well-known member

Frank B

Senior Member

dereckbc

Well-known member