Code Problems - need some advice

ok half the battle with this code is its copied from a website relating to multiplexing LED's and im not 100% sure on how or what its doing especially realting to the bitwise operations.

what i am trying to get this code to-do is multiplex 9 leds, that are setup as the bottom row of what will be eventually a 3 x 3 x 3 cube. however the code does not do anything except light up the first led for a split second then switch it off.

my led grid is arranged like this.

1-2-3
4-5-6
7-8-9

they all share a common ground connection and each Positive (anode ??) pin is connected to a sepeate pin on the arduino, here is a list of led (anode?) pins to arduino pin mappings, each arduino pin needs to be brough HIGH to bring the LED on.

Led = Arduino
1 = 2
2 = 3
3 = 4
4 = 5
5 = 6
6 = 7
7 = 8
8 = 9
9 = 10

here is the sketch i have so far, any help would be appreaciated on what is going wrong or a better way to multiplex the led's..

void setup() {                
  pinMode(2, OUTPUT); 
  pinMode(3, OUTPUT); 
  pinMode(4, OUTPUT); 
  pinMode(5, OUTPUT); 
  pinMode(6, OUTPUT); 
  pinMode(7, OUTPUT); 
  pinMode(8, OUTPUT); 
  pinMode(9, OUTPUT); 
  pinMode(10, OUTPUT); 
  Serial.begin(9600);
};

void loop() {
  byte PATTERN[1][3] = { 
  {B010,B101,B010},
  };
  int PATTERN_SIZE = 1;
  long PATTERN_DELAY_MS = 0.5;
  for (int pos = 0; pos < PATTERN_SIZE; pos++) {
    long start = millis();
    while (millis() - start < PATTERN_DELAY_MS) {
      ShowPattern(PATTERN[pos]);
    }
  }
  delay(3000);
}

void SetColumn(byte pattern) {
  int COL_PINS[] = {2, 3, 4, 5, 6, 7, 8, 9, 10};
  int COL_COUNT = 3;
  for (int i = COL_COUNT-1; i >= 0; i--, pattern >>= 1) {
    digitalWrite(COL_PINS[i], pattern & 1 ? LOW : HIGH);
    Serial.print(COL_PINS[i]);
  }
}

void ShowPattern(byte pattern[]) {
  Serial.println('here');
  long MULTIPLEX_DELAY_MS = 0.5;
  int ROW_COUNT = 3;
  int last_row = ROW_COUNT - 1;
  for (int row = 1; row < ROW_COUNT ; row++) {
    SetColumn(pattern[row]);
    Serial.print(row);
    delay(MULTIPLEX_DELAY_MS);
  }
}

thanks
kris

int COL_PINS[] = {2, 3, 4, 5, 6, 7, 8, 9, 10};

You've got a useful array there - why not share it?

pinMode(2, OUTPUT); 
  pinMode(3, OUTPUT); 
  pinMode(4, OUTPUT); 
  pinMode(5, OUTPUT); 
  pinMode(6, OUTPUT); 
  pinMode(7, OUTPUT); 
  pinMode(8, OUTPUT); 
  pinMode(9, OUTPUT); 
  pinMode(10, OUTPUT);

AWOL:

int COL_PINS[] = {2, 3, 4, 5, 6, 7, 8, 9, 10};

You've got a useful array there - why not share it?

pinMode(2, OUTPUT); 

pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);

this is me sounding completely new now :smiley: when you say share it do you mean add it into the setup function at the top? i appologise for not understanding your reply but im still very new to the arduinos language.

Do you notice how the numbers in the pinMode calls and the numbers in the array look very similar?
We software types are quite lazy and don't like repeating ourselves.
Put the array at global scope, make it a constant, and use it everywhere you reference those pin numbers.

AWOL:
Do you notice how the numbers in the pinMode calls and the numbers in the array look very similar?
We software types are quite lazy and don't like repeating ourselves.
Put the array at global scope, make it a constant, and use it everywhere you reference those pin numbers.

ok i get it now :), so instead of what i was doing do this

const int COL_PINS[] = {2, 3, 4, 5, 6, 7, 8, 9, 10};

void setup() {                
  for (int pinO = 0; pinO < sizeof(COL_PINS); pos++) {
    pinMode(COL_PINS[pinO],OUTPUT);
  }
  Serial.begin(9600);
};
for (int pinO = 0; pinO < sizeof(COL_PINS); pos++) {

Almost:for (int pinO = 0; pinO < sizeof(COL_PINS)/sizeof (COL_PINS[0]); pos++) {

AWOL:

for (int pinO = 0; pinO < sizeof(COL_PINS); pos++) {

Almost:for (int pinO = 0; pinO < sizeof(COL_PINS)/sizeof (COL_PINS[0]); pos++) {

im getting there, ive looked at the example for sizeof and wasn't sure what the divide is for? could you maybe explain what the reason is for it?

i noticed it wasnt needed for string arrays but could not get my head around the explanation of why it was needed for ints?

"sizeof" returns the number of bytes needed to store the given structure.
An "int" on the Arduino occupies two bytes, so the "sizeof" an "int" array is twice as big as the number of array elements.
sizeof (array[0]) is a convenient way of returning the number of bytes needed to represent just one array element.

AWOL:
"sizeof" returns the number of bytes needed to store the given structure.
An "int" on the Arduino occupies two bytes, so the "sizeof" an "int" array is twice as big as the number of array elements.
sizeof (array[0]) is a convenient way of returning the number of bytes needed to represent just one array element.

ahh right i think i get it now, so for instance

const int COL_PINS[] = {1, 2};

sizeof(COL_PINS);  //this would return 4 bytes

//however 

sizeof(COL_PINS) / sizeof(COL_PINS[0])  

//this divides the total amount of bytes returned by the first item in the array which is 2 bytes,
// this would then be the same as 4/2 , am i right in thinking then whilst not ideal you could use
// any item in the array for the division?
// i.e.

sizeof(COL_PINS) / sizeof(COL_PINS[2])

on a seperate note regarding the code, i have two serial.writeln calls in either of the functions to see when it goes through these, but there is nothing showing up in the serial monitor, if i put a serial.writeln in the main loop function it prints it fine.

im guessing its my code again, and maybe the fact that somehow the while loop never evaluates so it never gets in there?

long PATTERN_DELAY_MS = 0.5;

Storing a non-integer in an integer is rarely a good idea.

AWOL:

long PATTERN_DELAY_MS = 0.5;

Storing a non-integer in an integer is rarely a good idea.

ok thats a good point , the original code made no ref to what the variables type should be, so i knew that millis returned a long value and made the jump that this should also be a long, but then proceeded to use 0.5

ive set it to 5 and im now getting some lights :smiley: not working correctly yet but there atleast flashing now haha