This is my first code with ardiuno.
I currently don’t own an arduino, but I plan to do in a near future.
So, test the capabilities of me programming the arduino, I tried doing this code that took me 2 hours of work.
And I would like someone to test it out and record that testing for me to watch.
I will be really glad If this code works at all.
If your interested you can PM me.
const int ledpins = {
2,3,4,5,6,7};
const int pinCount = 6;
const int btnpin = 1;
const [glow]int[/glow] ledpins[] = {
2,3,4,5,6,7}; [glow] // use byte instead of int (you don't have more than 256 pins, do you?)[/glow]
[glow]const int pinCount = 6; // not needed, use sizeof(ledpins) instead[/glow]
const int btnpin = 1;
int onled = 2;
int counter;
void setup() {
pinMode(btnpin, INPUT);
[glow] int thisPin; // skip this since you declare a new variable in the forloop[/glow]
for (int thisPin = 0; thisPin < pinCount; thisPin++) {
pinMode(ledpins[thisPin], OUTPUT);
}
}
void loop() {
int sequence = 0;
int btnmode = digitalRead(btnpin);
if(btnmode == HIGH) {
int sequence = 1;
if(onled == 7) {
[glow]int[/glow] onled = 2; [glow] // every time you prefix with int you create a new unrelated variable![/glow]
}
else {
int onled = onled + 1;
}
}
if(sequence = 1){
for(int counter = 2; counter == onled; counter++){
digitalWrite(counter, HIGH);
delay(50);
}
}
}
Also, for sequence it's probably better to use type "boolean", which can be either false or true.
Tip for Enddy; if you want people to help you with code please use comments describing what should happen below the comment. A comment is prefixed with //.
For example:
// If button is down (sequence is 1 if button is down)
if(sequence = 1){
// ? -- not sure what you want to do, but a comment could be:
// Switch all LED's with number equal to or lower then onled on
for(int counter = 2; counter == onled; counter++){
digitalWrite(counter, HIGH);
delay(50);
}
}
//Start the loop to activate a sequence.
void loop() {
int sequence = 0;//Turn off the sequence.
delay(500);//So the button press counts only once.
int btnmode = digitalRead(btnpin);//Read the inpput
if(btnmode == HIGH) {//Analyse the input.
int sequence = 1;//Sequence is on.
if(onled == 7) {//Restart the pin selection.
int onled = 2;
}
else {//Move the pin selection.
int onled = onled + 1;
}
}
if(sequence = 1){//Turn on the sequence.
for(int counter = 2; counter == onled; counter++){//Blink every LED before the selected(Selected pin included).
digitalWrite(counter, HIGH);
delay(50);
digitalWrite(counter, LOW);
}
}
else {
digitalWrite(onled, HIGH);
}
}
I don’t use Boolean because 0’s and 1’s are cheap and small.
This is what is supposed to do:
Legend:
LED’s = - off + on
Button = ^ off v on
When turned on.
^ ±----
Sequence:
v ±----
^ ------
^ -±—
^ ------
^ -±— and stay on until next press.
I’m going to skip to the sixth press.
v ±----
^ ------
^ -±—
^ ------
^ -±—
^ ------
^ --±–
^ ------
^ —±-
^ ------
^ ----±
^ ------
^ -----+
^ ------
^ -----+ and stay on until next press.
v ±----
^ ------
^ ±---- and stay on until next press.
This is the one and only time you should have “int” (or even better “byte”) before onled. Remember:
if (onled == 7) {
int onled = 2;
}
The “onled” you set to 2 here will disappear at the “}”, the assignment thus has no effect.
for (int thisPin = 0; thisPin < sizeof(ledpins); thisPin++) {
Sorry, my bad here. sizeof does not return the number of elements, but the number of bytes used by the array.
The proper expression for the number of elements is
sizeof(ledpins)/sizeof(ledpins[0])
If you had changed ledpins to a byte array, then ledpins[0] would have been 1 and so could be omitted.
Meinaart’s comment about the line above still applies. The expression between the semicolons determine if the loop body is to be run. So it could only run when counter equals onled. Since you initialize counter to 2, it only runs if onled is 2. With the current code, onled will always be 2 since the only code that tries to assign another value:
else {//Move the pin selection.
int onled = onled + 1;
}
declares a new temporary variable that will disappear at the “}”.
So later, when you set the value, you had left in the declaration (int) .. which defines a new variable instead of setting the global variable.
so
int onled = onled + 1;
defines a new variable .. also called onled .. which has nothing to do with the global one .. they are in different memory spaces.
this should be
onled = onled + 1;
or better ...
onled++;
Have you programmed in basic? if so, the Dim statement is much like the int declaration.
So the statement
int onled = x
would be the same as
Dim onled as Integer
... which declares a new variable in that function. And when any function ends .. all variables defined in that function die a horrible death.
//Start the loop to activate a sequence.
void loop() {
delay(500);//So the button press counts only once.
int sequence = 0;//Turn off the sequence.
int btnmode = digitalRead(btnpin);//Read the input.
if(btnmode == HIGH) {//Analyse the input.
sequence = 1;//Sequence is on.
if(onled == 7) {//Restart the pin selection.
onled = 2;
}
else {//Move the pin selection.
onled++;
}
}
if(sequence = 1){//Turn on the sequence.
for(int counter = 2; counter <= onled; counter++){//Blink every LED before the selected(Selected pin included).
digitalWrite(counter, HIGH);
delay(50);
digitalWrite(counter, LOW);
}
}
else {
digitalWrite(onled, HIGH);
}
}
Please if you can explain how does sizeof() work I’ll be glad because drhex said something about it but I’m not sure what he meant.
sizeof returns the number of bytes something uses.
An int needs 2 bytes, and there are 6 of them in ledpins, so
sizeof(ledpins) == 12
siezof(ledpins[0]) = 2 // size of only the first int
Thus, the loop in setup will read outside the array. Tis can be fixed in two ways:
const byte ledpins = { 2,3,4,5,6,7 };
so that sizeof(ledpins) == 1*6
(with the for-loop condition thisPin < sizeof(ledpins) )
and/or
for (int thisPin = 0; thisPin < sizeof(ledpins)/sizeof(ledpins[0]); thisPin++) {
You probably need to rethink the ‘sequence’ variable.
Do you intend it to become 1 when the button is pressed and then stay 1 for the rest of the program?
If so, it should be prefixed with “static” or have its declaration moved outside of loop. (now, every time loop starts over sequence is set to 0)
If you actually want sequence to be 1 only when the button is pressed, the variable is not needed. You could simply put the “blink” code inside the “if (btnmode == HIGH)” block.
Oh, and if you have a nice array of pins, why not use it also in the blink code instead of assuming the pin numbers are contiguous?
Another nice improvement might be to use the index of the array instead of the pinnumbers directly. That way it’s easier to change the pin numbers (and they don’t have to be ascending).
For example:
//Set the pins.
const int ledpins[] = {
2,3,4,5,6,7};
const int btnpin = 1;
const int countLedPins; // holds number of elements in ledPins array
//Start the variables.
int currentIndex = 0;
//Setup the pins.
void setup() {
pinMode(btnpin, INPUT);
countLedPins = sizeof(ledpins) / sizeof(ledpins[0]);
for (int thisPin = 0; thisPin < countLedPins; thisPin++) {
pinMode(ledpins[thisPin], OUTPUT);
}
}
//Start the loop to activate a sequence.
void loop() {
delay(500);//So the button press counts only once.
boolean sequence = false;//Turn off the sequence.
int btnmode = digitalRead(btnpin);//Read the input.
if(btnmode == HIGH) {//Analyse the input.
sequence = true; //Sequence is on.
// the - 1 is because of indexes being zero based
if(currentIndex == (countLedPins - 1)) {//Restart the pin selection.
currentIndex = 0;
}
else {//Move the pin selection.
currentIndex++;
}
// Line below is a shorter (but more unreadable) version
// of the if and else statement above
// See this url for the % (modulo) operator:
// http://arduino.cc/en/Reference/Modulo
// currentIndex = (currentIndex + 1) % countLedPins;
}
if(sequence){//Turn on the sequence.
for(int index = 0; index <= currentIndex; index++){//Blink every LED before the selected(Selected pin included).
digitalWrite(ledpins[index], HIGH);
delay(50);
digitalWrite(ledpins[index], LOW);
}
}
else {
// If button is not down just light the last LED
digitalWrite(ledpins[currentIndex], HIGH);
}
}
Btw; just buy a board and start testing. They are not expensive ;). A lot easier then debugging without a board.
//Start the loop to activate a sequence.
void loop() {
delay(500);//So the button press counts only once.
boolean sequence = false;//Turn off the sequence.
byte btnmode = digitalRead(btnpin);//Read the input.
if(btnmode == HIGH) {//Analyse the input.
sequence = true; //Sequence is on.
// the - 1 is because of indexes being zero based
if(currentIndex == (countledpins - 1)) {//Restart the pin selection.
currentIndex = 0;
}
else {//Move the pin selection.
currentIndex++;
}
}
if(sequence){//Turn on the sequence.
for(byte index = 0; index <= currentIndex; index++){//Blink every LED before the selected(Selected pin included).
digitalWrite(ledpins[index], HIGH);
delay(50);
digitalWrite(ledpins[index], LOW);
}
}
else {
// If button is not down just light the last LED
digitalWrite(ledpins[currentIndex], HIGH);
}
}