# Arduino C, Led > binary alternative all lights

I'm trying to alternatively have a random int generate a binary number with 4 lights.

This is my current code, However, it seems that all the lights(leds) go on, I don't think the minus option works. As in, it won't take a value from the currently randomized digit. (randombinary = randombinary - x)

I also tried just "randombinary - x"

``````const int button = 2;
const int redone = 13;
const int redtwo = 8;
const int greenone = 12;
const int greentwo = 7;
int randomnumber = 0;
int randombinary = 0;

void setup() {

pinMode(13, OUTPUT);
pinMode(12, OUTPUT);
pinMode(7, OUTPUT);
pinMode(8, OUTPUT);
pinMode(2, INPUT);

}
void loop() {

delay(1000);

int randomnumber = (1, 15);
randombinary = randomnumber;

if (randombinary >=8){
digitalWrite(redone, HIGH);
randombinary = randombinary - 8;
}
else {
digitalWrite(redone, LOW);
}

if (randombinary >=4){
digitalWrite(greenone, HIGH);
randombinary = randombinary - 4;
}
else {
digitalWrite(greenone, LOW);
}

if (randombinary >=2){
digitalWrite(redtwo, HIGH);
randombinary =  randombinary - 2;
}
else  {
digitalWrite(greentwo, LOW);
}

if (randombinary >=1){
digitalWrite(greentwo, HIGH);
randombinary =  randombinary - 1;
}
else  {
digitalWrite(greentwo, LOW);
}

}
``````

Would appreciate any help!

• Please edit your question and state what you want to happen. Eg, "Generate a random number. Turn on light x if ..., turn off light x if ..., turn on light y if ..., turn off light y if ..." and so forth. – James Waldby - jwpat7 Dec 20 '15 at 23:42

Code review (one possible solution can be found below)

• Absolute numbers in the code, like 15 as the limit for the random numbers are generally considered harmful. It would be better to give all absolute numbers a constantsname and use these throughout. The OP code did not use the constants for the pins, for instance.
• Writing similar pieces of code in a row makes maintenance hard. Therefore, group similar items in any kind of list, like an array, and iterate over them.
• Functions should have expressive names clearly stating what is going on inside.
• Functions should be short and concise, ideally having only a few lines.

Try something along these lines:

``````// setting the number of bits to analyze, corresponding pins and corresponding powers of two.
// the latter is here for instructional purposes although it is redundant.
const int numberOfBits = 4;
const int randLimit = 15;   // 2^(1+numberOfBits)-1 actually;
int ledPins[numberOfBits] = {13, 8, 12, 7};
int bitValue[numberOfBits] = {8, 4, 2, 1}; // the powers of two

void turnOffAllLeds ()
{
Serial.println ("turning all off");
for (int i = 0; i < numberOfBits; i++)
{
digitalWrite (ledPins[i], LOW);
}
}

void selectivelyTurnOnLeds(int value)
{
turnOffAllLeds ();

// now for each bit, check if it is set in the value, if yes: turn on corresponding led
for (int i = 0; i < numberOfBits; i++)
{
if ((value & bitValue[i]) > 0)
{
digitalWrite (ledPins[i], HIGH);
}
}
}

void setup()
{
for (int i = 0; i < numberOfBits; i++)
{
pinMode (ledPins[i], OUTPUT);
}
}

void loop()
{
selectivelyTurnOnLeds (random (randLimit));
delay (1000);
}
``````

P.S. When posting a question, please also post what your code is supposed to do. If it were not for a lazy prechristmas workday, I would not have taken the time to reverse engineer your code.

• Thank you !! My apologies for the late reply, I'm working on it now, Yours is very tough, I cannot seem to get it working, yet I myself am not sure how exactly you've done it, this is a bit over my level. I for example, don't know how where the "i" is linked to, nor do I know why you put (int value) after the selectivelyturn... void. I thank you kindly, and I'm sorry for not being clear in my question, it was the first time I've entered a forum for help.. It's always giving the value of 15, but the leds change nevertheless – Texz Jan 4 '16 at 19:33
``````int randomnumber = (1, 15);
``````

Guess you have missed to call random. That statement actually always returns 15. C is a strange language.

``````int randomnumber = random(1, 15);
``````

Cheers!

• Thanks a bunch! You're a savior, Now only I have to make it so that I have the button input match the random binary which results into a victory LED if it's completed. :S It's going to be tough – Texz Dec 20 '15 at 21:43
• I actually just moved it into a new void, as due to the loop, it kept recreating a new binary. Now it only shows the value of 8, each time.. do you happen to know what could possibly be wrong? Nothing in the code changed – Texz Dec 20 '15 at 21:54
• Something must have changed if you moved it into a new void! Post the new code under the old to keep the history of the question. – Transistor Dec 20 '15 at 22:11
• Thank you for your time, I've replied to this inquiry myself with the new code and issue. – Texz Dec 20 '15 at 22:27

So here's the new version, very strange, when I set the number on random, it doesn't work. However, if I enter a number myself, it does.

``````const int button = 2;
const int redone = 13;
const int redtwo = 8;
const int greenone = 12;
const int greentwo = 7;
int randomnumber = 0;
int randombinary = 0;

void setup() {

pinMode(13, OUTPUT);
pinMode(12, OUTPUT);
pinMode(7, OUTPUT);
pinMode(8, OUTPUT);
pinMode(2, INPUT);

maarten();

}
void loop() {

}

void maarten(){

int randomnumber = random(1, 15);
randombinary = randomnumber;

if (randombinary >=8){

digitalWrite(redone, HIGH);
randombinary = randombinary - 8;
}
else {
digitalWrite(redone, LOW);
}

if (randombinary >=4){

digitalWrite(greenone, HIGH);
randombinary = randombinary - 4;
}
else {
digitalWrite(greenone, LOW);
}

if (randombinary >=2){

digitalWrite(redtwo, HIGH);
randombinary =  randombinary - 2;
}
else  {
digitalWrite(greentwo, LOW);
}

if (randombinary >=1){

digitalWrite(greentwo, HIGH);
randombinary =  randombinary - 1;
}
else  {
digitalWrite(greentwo, LOW);
}

}
``````
• But now the setting (maarten) is only done once? – Mikael Patel Dec 20 '15 at 22:42
• Please do the edits in the main question so folks know which code to work with. I'd replace the conditional subtraction math with bitwise operations on an unsigned value "unsigned int randombinary; ... digitalWrite(redone,randombinary && 0b1000 ); ..." – Dave X Dec 21 '15 at 19:09