Strange coding problem

I wrote a small program to display incrementing numbers in binary on a string of LED’s but bits > 3 (ie 2^4, 2^5, 2^6…etc) dont light up or are very dim

I know it’s not a hardware problem because i can swap around LED’s and they work fine, I can also upload some older code i wrote that uses the same setup and all is well there too.

any help would be much appreciated.

int drawdelay = 15; // delay between displaying each bit
int countdelay = 50; // delay between bisplaying digits
int ledPins[]={2,3,4,5,6,7}; //pins LEDS are connected to,lowest bit first
int ledcount=sizeof(ledPins)/sizeof(ledPins[0]); //some magic to get size of array
int num=pow(2,ledcount)-1; //highest number we can display

void setup() {
  Serial.begin(9600);
  for (int i = 0; i < ledcount-1;i++){ // Setup pins as outputs
    pinMode (i,OUTPUT);
  }
}

void loop() {
  for (int i = 0; i <= num+1; i++){ // generate the numbers we will display
    for (int n = ledcount-1; n >= 0; n--){ // iterate through each bit
      Serial.print(bitRead(i,n));
      if (bitRead(i,n) == 0) {
        digitalWrite(ledPins[n],LOW); //set led to bit state
      }
      if (bitRead(i,n) == 1) {
        digitalWrite(ledPins[n],HIGH); //set led to bit state
      }
      delay(drawdelay); // delay between bits
    }
    ///*
    Serial.print(" ");
    Serial.print(i);
    Serial.print(" ");
    Serial.print(ledcount);
    Serial.println();
    //*/
    delay(countdelay); //delay before next number
  }
}

Are you using resistors on the LED's, I'm not sure, but you probably shouldn't drive six LED's off the arduino supply/pins. Use transistors and a battery/PSU to provide some more power.

also you can combine your ifs, No need to check that bitRead(i,n) == 1 as the previous if checks if it is 0

This will replace both ifs, but is only usable as the Arduino core currently has LOW & HIGH defined as 0 & 1 respectively.

digitalWrite(ledPins[n], bitRead(i,n) );

Hi this is my first post here, i'm aware they are old hat but for this i used a darlington array with 4.5v 350ma phone charger and it worked perfectly, I am developing a nix like serial shell to be controlled from and dd-wrt router if anyone has any experience in string handling i can post some code. The problem is occurring when i change the delimiter half way through a sequence of strtok_r calls.

Any help is much appreciated.

Mark

I didn't relize that High and LOW could be expressed as 1 or 0, so thanks muchly for that bit.

As for it being a power problem, i don't think that is the case as my old sketch works just fine.

and yes, all LED's are protected by 330 ohm resistors.

I would put the delay after the inner loop - not inside it - and make it a bit longer so that the LEDs hold one number long enough for you to see it (e.g. 500). And, with the fix from pYro_65, the inner loop would look like this:

    for (int n = ledcount-1; n >= 0; n--){ // iterate through each bit
      Serial.print(bitRead(i,n));
      digitalWrite(ledPins[n], bitRead(i,n) );
    }
    delay(drawdelay); // delay between bits

You also have a problem with using pow. It is a floating point function and you won’t get the result you are expecting. On my Duemilanove, num is not 63 - it is 62. Define it as:

int num = (1<<ledcount) - 1;

Pete

This code:

  for (int i = 0; i < ledcount-1;i++){ // Setup pins as outputs
    pinMode (i,OUTPUT);
  }

sets pins 0,1,2,3,4 as outputs, not pins 2,3,4,5,6,7 as you want. Hence pins 5,6,7 are still inputs so will only light the LED dimly due to the internal pullup resistor.

@stimmer Good catch!

Pete

you guys rock!

totally missed that

should have been

  for (int i = 0; i < ledcount;i++){ // Setup pins as outputs
    pinMode (ledPins[i],OUTPUT);

and if anyone is interested here is the revised code

int drawdelay = 15; // delay between displaying each bit
int countdelay = 200; // delay between bisplaying digits
int ledPins[]={2,3,4,5,6,7}; //pins LEDS are connected to,lowest bit first
int ledcount=sizeof(ledPins)/sizeof(ledPins[0]); //some magic to get size of array
//int num=pow(2,ledcount)-1; //highest number we can display
int num = (1<<ledcount) - 1;

void setup() {
  Serial.begin(9600);
  for (int i = 0; i < ledcount;i++){ // Setup pins as outputs
    pinMode (ledPins[i],OUTPUT);
  }
}

void loop() {
  for (int i = 0; i <= num+1; i++){ // generate the numbers we will display
    for (int n = ledcount-1; n >= 0; n--){ // iterate through each bit
      Serial.print(bitRead(i,n));
      digitalWrite(ledPins[n],(bitRead(i,n))); //set led to bit state
      delay(drawdelay); // delay between bits
    }
    ///*
    Serial.print(" ");
    Serial.print(i);
    Serial.print(" ");
    Serial.print(ledcount);
    Serial.print(" ");
    Serial.print(num);
    Serial.println();
    //*/
    delay(countdelay); //delay before next number
  }
}

Thanks for helping me get my learn on! :slight_smile:

I feel compelled to add that this has been one of my most enjoyable internet experiences to date. I learned a lot and had fun. keep up the good work guys!

pYro_65: Are you using resistors on the LED's, I'm not sure, but you probably shouldn't drive six LED's off the arduino supply/pins. Use transistors and a battery/PSU to provide some more power.

also you can combine your ifs, No need to check that bitRead(i,n) == 1 as the previous if checks if it is 0

This will replace both ifs, but is only usable as the Arduino core currently has LOW & HIGH defined as 0 & 1 respectively.

digitalWrite(ledPins[n], bitRead(i,n) );

6 LEDS off the Arduino pins is no problem at all. If they are run at 10ma each, it would total to 60ma. Well below the 200ma total max and 40ma per pin max.