EEVblog Electronics Community Forum

Electronics => Beginners => Topic started by: crispytato on May 14, 2014, 04:05:33 am

Title: help with pretty basic arduino code, what am I doing wrong?!
Post by: crispytato on May 14, 2014, 04:05:33 am
Hi all, I'm working on an Arduino project to control a set of LED tail lights in a car part by part, and I'm having some difficulty getting my sketch to achieve the desired result. I've been reading tutorials on basic Arduino functions of all sorts for months now (not specifically on this project, but I've been trying to learn to code on it for some time) yet I still can't get what is seemingly a very simple piece of code to work! I've managed to get some very basic sketches working, but this problem is alluding me...

Simply put, the sketch is meant to check if pins 7,8,9 are high or low, and set the other pins (optically isolated and driving power fets) to follow what the inputs are doing. Should be simple, just some if, else statements and that's it, but at the moment I'm not getting any of my outputs going high regardless what I set the input pin(s) to. Before anyone asks, I've tested the hardware by just feeding in 5v and gnd to the optos, and they drive the led strips just fine.

My code is at the bottom of the post, it should be possible to directly paste into arduino ide. I haven't commented much of it because it is fairly self-explanatory, I would hope.

I hope that someone can point me in the right direction! Please ask if you need anymore details to help me out :)

Code: [Select]
int brakeLightOne = 2; //init outputs on D2-D6
int brakeLightTwo = 3;
int brakeLightThree = 4;
int turnLight = 5;
int reverseLight = 6;

int brakeInput = 7; //init inputs on D7,D8,D9
int turnInput = 8;
int reverseInput = 9;

void setup()
{
  pinMode(brakeLightOne, OUTPUT); //setting light pins (2-6) as outputs
  pinMode(brakeLightTwo, OUTPUT);
  pinMode(brakeLightThree, OUTPUT);
  pinMode(turnLight, OUTPUT);
  pinMode(reverseLight, OUTPUT);

  pinMode(brakeInput, INPUT); //setting input pins (D7,D8,D9) as inputs
  pinMode(turnInput, INPUT);
  pinMode(reverseInput, INPUT);
}

void loop()
{
  if (brakeInput == HIGH) {
    digitalWrite(brakeLightOne, HIGH);
    //delay(500);
    digitalWrite(brakeLightTwo, HIGH);
    //delay(500);
    digitalWrite(brakeLightThree, HIGH);
    //delay(500);
    } 
   
    else{
      digitalWrite(brakeLightOne, LOW);
      //delay(500);
      digitalWrite(brakeLightTwo, LOW);
      //delay(500);
      digitalWrite(brakeLightThree, LOW);
      //delay(500);
      }
     
  if (turnInput == HIGH) {
    digitalWrite(turnLight, HIGH);
    //delay(500);
    }
   
    else{
      digitalWrite(turnLight, LOW);
      //delay(500);
    }
   
if (reverseInput == HIGH) {
  digitalWrite(reverseLight, HIGH);
  //delay(500);
  }
 
  else{
    digitalWrite(reverseLight, LOW);
    //delay(500);
  }
   
  delay(5);
}
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: TerminalJack505 on May 14, 2014, 04:14:16 am
You need to use digitalRead() to get the states of your inputs. 

So, instead of saying if (brakeInput== HIGH), you'd say if (digitalRead(brakeInput) == HIGH).
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: crispytato on May 14, 2014, 04:45:55 am
Thanks very much for the quick response :) I'll try that now, this place is great!
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: electr_peter on May 14, 2014, 05:49:48 pm
I would to create new variables to keep track of input states.
Add to your code
Code: [Select]
// initialise states of inputs
int brakeInputState = 0, turnInputState = 0, reverseInputState = 0;
void setup(){
[...]
}
void main(){
// update input states
if(digitalRead(brakeInput))
  brakeInputState = 1;
else
  brakeInputState = 0;

if(digitalRead(turnInput))
  turnInputState = 1;
else
  turnInputState = 0;

if(digitalRead(reverseInput))
   reverseInputState = 1;
else
  reverseInputState = 0;

// update outputs
if(brakeInputState==1) // turn on some lights
else // turn off some lights
[...]
}
Keeping states in memoty allows for more readable and capable code.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: senso on May 14, 2014, 07:07:46 pm
Thats a lot of if/else to do what this would do:
myVar = digitalRead(myPin); //Returns either 0 or 1
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: electr_peter on May 14, 2014, 08:54:35 pm
Thats a lot of if/else to do what this would do:
myVar = digitalRead(myPin); //Returns either 0 or 1
I agree that writing so is a bit cleaner and shorter. But this expression could be shorter and quicker still, but I won't go there. Compiler and MCU don't care too much either way. Also if/else allows to construct more difficult conditional statements if needed.

I think that if/else notation may be easier to understand for beginners and may be a bit harder to mess up.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: Sam__ on May 14, 2014, 09:59:12 pm
Isn't it cleaner to use a boolean for reading a digital variable? Removes any possibility for error values.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: electr_peter on May 14, 2014, 10:44:07 pm
Isn't it cleaner to use a boolean for reading a digital variable? Removes any possibility for error values.
I agree that boolean is cleaner and could remove errors. I usualy use such "boolean" coding myself.

However, it is not as simple in this case with Arduino (and many AVR chips). Are you aware that there is no "true" boolean type in Arduino IDE? Boolean is one of 8bit types internally. If you miss this, true/false values may become -2 or +10 after some operations with "boolean". This can lead to interesting debuging of strange errors later on, if missed.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: SL4P on May 15, 2014, 02:50:20 am
I'm curious that you are using optos on the output FETs - rather than the inputs.
Also think about fail-safe mode - monitoring the circuit operation to fail back to a less fancy luminaire.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: MLXXXp on May 15, 2014, 12:49:37 pm
I'm a long time C programmer but new to Arduino.

In the OP's code and many other examples, pins are "named" as follows:
Code: [Select]
int myOutput = 3;
In the usual case, where you're not going to move myOutput to a different pin within the sketch, would it be better to declare myOutput as a constant?
Code: [Select]
const int myOutput = 3;
Will declaring a value as const compile to more efficient code? If not, I'd still think it would help to make the code more readable and less subject to coding errors.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: electr_peter on May 15, 2014, 01:00:30 pm
In expression
"const int myPin = 5;"
"const" means read-only variable. Variable would be initialised as such, but program would not change it (or, if it tried, there would be an error).

"const" is good way to define some constants and variables you knoe you will not change. You could also use "#define" to set constants, but "#define" usage is a more tricky.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: MLXXXp on May 15, 2014, 01:04:16 pm
Will declaring a value as const compile to more efficient code?

As a follow up to my own previous post, I hacked the OP code to the following short test:
Code: [Select]
const int brakeLightOne = 2;
const int brakeLightTwo = 3;
const int brakeLightThree = 4;

void setup()
{
  pinMode(brakeLightOne, OUTPUT);
  pinMode(brakeLightTwo, OUTPUT);
  pinMode(brakeLightThree, OUTPUT);
}

void loop()
{
  digitalWrite(brakeLightOne, HIGH);
  digitalWrite(brakeLightTwo, HIGH);
  digitalWrite(brakeLightThree, HIGH);
  delay(5);
}

It compiles to 5010 bytes. With the const declarations removed from the first three lines, the resulting sketch is 5028 bytes.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: Rigby on May 15, 2014, 01:39:44 pm
don't worry about the code size until the code size becomes an issue.

if code size is an issue, move away from the arduino IDE and use C natively, or use an arduino with more program space.  (or just move away from arduino -- I recommend this board (http://www.ti.com/ww/en/launchpad/launchpads-connected-ek-tm4c1294xl.html#tabs) for $20, including shipping.  There is a free IDE for this board (http://www.energia.nu/) that is an Arduino IDE clone.)

If you can, and if the Arduino in question has support for it, I would recommend using hardware interrupts on the sense pins rather than running a scanning loop.  That's just how I prefer to do it, so don't put too much weight into that suggestion; your preference may be different once you've tried both ways.
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: electrolux on May 15, 2014, 01:45:12 pm
Oh dear some one under the murphy curse ???
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: FreddyVictor on May 15, 2014, 01:49:40 pm
I'm a long time C programmer but new to Arduino.

In the OP's code and many other examples, pins are "named" as follows:
Code: [Select]
int myOutput = 3;
In the usual case, where you're not going to move myOutput to a different pin within the sketch, would it be better to declare myOutput as a constant?
Code: [Select]
const int myOutput = 3;
Will declaring a value as const compile to more efficient code? If not, I'd still think it would help to make the code more readable and less subject to coding errors.

seen lots of 'arduino' code, but I've never quite understood the reason for using a variable to hold the pin #
it's not like you're going to change the pin assignment mid-code - well, not usually!

if you're going to make it a const, then why not:

#define myOutput 3
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: MLXXXp on May 15, 2014, 01:55:46 pm
if you're going to make it a const, then why not:

#define myOutput 3
Because "Constants defined with the const keyword obey the rules of variable scoping that govern other variables" as described here:
http://arduino.cc/en/Reference/Const (http://arduino.cc/en/Reference/Const)
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: SL4P on May 16, 2014, 05:12:32 am
if you're going to make it a const, then why not:

#define myOutput 3
Because "Constants defined with the const keyword obey the rules of variable scoping that govern other variables" as described here:
http://arduino.cc/en/Reference/Const (http://arduino.cc/en/Reference/Const)

Gawd - I'd hope that #defines are expanded and compiled inline - not stored in RAM like other variables... !
(In my Arduino experience that is true - so a CONST or define will conserve RAM over using an int)
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: AG6QR on May 16, 2014, 06:07:50 am
Gawd - I'd hope that #defines are expanded and compiled inline - not stored in RAM like other variables... !
(In my Arduino experience that is true - so a CONST or define will conserve RAM over using an int)

Yes, a define will be expanded in-line, just like a text search and replace.  So a #define may have an advantage over declaring an actual variable.

But a const is NOT a variable.  There may be some syntactic similarities, but the semantics are different, and, at least in C++ and the arduino language, a const doesn't take up RAM at runtime.  In any reasonable compiler, there is no size or efficiency advantage of using 

#define myPin 3

over using

const int myPin = 3;

Compilers and their authors generally aren't stupid, and if you tell the compiler to treat a symbol as a constant integer, it will treat it as a constant integer just as efficiently as if you put the literal integer in the text of the program. 

Declaring something as const int has no efficiency downside compared to using a #define, but it has the upside of allowing the compiler to do proper type checking, as well as allowing the symbol to follow the scope rules.

When something really is a constant, it's not a bad thing to declare that fact to the compiler.


http://www.baldengineer.com/blog/2013/11/14/const-vs-define-when-do-you-them-and-why/ (http://www.baldengineer.com/blog/2013/11/14/const-vs-define-when-do-you-them-and-why/)
Title: Re: help with pretty basic arduino code, what am I doing wrong?!
Post by: crispytato on May 18, 2014, 10:37:57 pm
Thanks for all of the input guys, its been interesting hearing ways to streamline my code. As said before, I'm very new to arduino and coding in general, and overall, I wanted something  that wold work, and make sense to me rather than copy/pasting a piece of working code off the net. That's the reason for using if/else and the lengthyness of the routines. I am also pwming some outputs to control brightness so I wanted to be able to implement more complex stuff myself and actually understand it.

I'm definitely a hardware guy... but this stuff is all new to me, hopefully I can get more efficient as time and experience allows.

In regards to someone's question about making the unit fail safe, I have a number of hardware features in place to default to lights to on state in the case of the mcu signal not getting to the switching elements in some way (so that at least the car is visible) and further ideas I'd be happy to hear them. Optos are on both sides, input and output, just to keep all mcu pins isolated far from the +BAT line in the car which is pretty noisy



Thats a lot of if/else to do what this would do:
myVar = digitalRead(myPin); //Returns either 0 or 1
I think that if/else notation may be easier to understand for beginners and may be a bit harder to mess up.
add to that, easier to write as I'm not experienced at this stuff at all.