0

I'm not sure if i should post this here or on StackOverflow but I am coding for an LED cube controlled with 4 shift registers. I have a function in a .h file that is supposed to take an array of 1s and 0s (to represent each LED) and then use the shiftOut function to shift the data into the shift registers. This is complicated that the shiftOut function has to take a int which it shifts out in binary and only goes 8 bits at a time. This means I have to separate my array into 4 groups of 8 and then convert each group into an int. I have tried to make my function do that but it is compiling fine and then not lighting up any LEDs. I believe the problem has to do with the way I'm separating the array into 4 smaller arrays althoughI can not figure out exactly what is going wrong, here is my code.

Main program:

#include <LEDcube.h>

int data = 2; 
int clock = 3;
int latch = 4;

void setup() 
{
  pinMode(data, OUTPUT);
  pinMode(clock, OUTPUT);  
  pinMode(latch, OUTPUT);  
  Serial.begin(9600);
}

void loop() 
{
  int layer[32] = {0,

                  1,1,1,1,1,
                  0,0,0,0,0,
                  1,1,1,1,1,
                  0,0,0,0,0,
                  1,1,1,1,1,

                  0,0,0,0,0,0};
  write(data, clock, latch, layer);
}

LEDcube.h:

#include "Arduino.h"

int binToNum(int bin[])
{
  int j = 1;
  int num = 0;
  for(int i = 0; i < 8; i++, j = j * 2)
  {
    if(bin[i] == 1)
    {
      num = num + j;
    }
  }
  return num;
}
void write(int data, int clock, int latch, int layer[])
{
  int reg1[8];
  int reg2[8];
  int reg3[8];
  int reg4[8];
  for(int i = 0; 1 < 8; i++)
  {
    reg1[i] = layer[i];
  }
    for(int i = 0; 1 < 8; i++)
  {
    int q = i + 8;
    reg2[i] = layer[q];
  }
  for(int i = 0; 1 < 8; i++)
  {
    int q = i + 16;
    reg3[i] = layer[q];
  }
  for(int i = 0; 1 < 8; i++)
  {
    int q = i + 24;
    reg4[i] = layer[q];
  }  
  int regnum1 = binToNum(reg1);
  int regnum2 = binToNum(reg2);
  int regnum3 = binToNum(reg3);
  int regnum4 = binToNum(reg4);
  digitalWrite(latch, LOW);
  shiftOut(data, clock, MSBFIRST, regnum4);
  shiftOut(data, clock, MSBFIRST, regnum3);
  shiftOut(data, clock, MSBFIRST, regnum2);  
  shiftOut(data, clock, MSBFIRST, regnum1);
  digitalWrite(latch, HIGH);
}
5
  • for(int i = 0; 1 < 8; i++) is this correct? should it be for(int i = 0; i < 8; i++)?
    – PhillyNJ
    Oct 21 '15 at 0:40
  • I'm not sure what you mean @PhillyNJ
    – Stu
    Oct 21 '15 at 1:45
  • What are you using for a shift register?
    – Jake C
    Oct 21 '15 at 4:36
  • @PhillyNJ means 1<8 instead of i<8 (notice the 1 (one) instead of the i (letter I))
    – Gerben
    Oct 21 '15 at 9:56
  • ohhhhhhhhhhhhhhh, that typo might have been the problem
    – Stu
    Oct 21 '15 at 17:31
1

As noted in Stu's answer, the question's code does not pack bits properly into the bytes that shiftOut() will unpack them from.

If you merely want to make things work, as opposed to packing stuff up one place so it can be unpacked another, just write the bits to your SR yourself. For example:

for (int i=0; i<32; ++i) {
  digitalWrite(data, layer[i]); // Put a bit on the data pin
  digitalWrite(clock, HIGH); // Pulse the clock
  digitalWrite(clock, LOW);
}
digitalWrite(latch, HIGH); // Pulse the latch
digitalWrite(latch, LOW);

If the clock or latch need low-going instead of high-going pulses, reverse the order of writing HIGH and LOW.

1

The problem is you need to write your data with all of your bits consolidated into a single integer instead of as an array of integers. For example, if you were to shiftOut(data, clock, MSBFIRST, 1) that would send 00000001, not 1. So for example, if you want to send out 1,0,1,0,1,0,1,0 you need to send shiftOut(data, clock, MSBFIRST, 170), because 170 in binary is 10101010. So you will need to figure out how to represent your data as an integer rather than array of integers of which you are only using the first bit.

EDIT:

I see that you are attempting to do a binary conversion, but your for loop is a little off. Try this instead:

for(int i = 0; i < 8; i++)
{
  if(bin[i] == 1)
  {
    num = num + j;
  }
  j = j * 2;
}

Even if that fixes it, you will probably still need to some major refactoring to get decent speed, currently you are using 128 bytes of memory to represent 4 bytes worth of data. and there are plenty of other little inefficiencies here and there.

EDIT 2:

The simplest version of the above functionality is this:

int data = 2; 
int clock = 3;
int latch = 4;

void setup() 
{
  pinMode(data, OUTPUT);
  pinMode(clock, OUTPUT);
  pinMode(latch, OUTPUT);
}

void loop() 
{
  int data1 = 0b1111100000111110; //Set a variable using a binary constant
  int data2 = 0b0000111110000000;
  digitalWrite(latch, LOW);
  shiftOut(data, clock, MSBFIRST, data1);
  shiftOut(data, clock, MSBFIRST, data2);
  digitalWrite(latch, HIGH);
}

If you are going to be doing more bit level manipulation, you should probably learn various bit wise operators such as <<, >> &, ^, |, ~, &=, and |=. The Arduino documentation goes over them and what they do (scroll down, on the left under Bitwise Operators).

2
  • Thats what I was trying to do by converting my array of ones and zeros into an integer ( int regnum1 = binToNum(reg1); ) @Jake C
    – Stu
    Oct 21 '15 at 1:43
  • Thanks for the help although I don't think that the problem is in the binary to integer conversation function as it was works fine with out when called out side of the write function. I think the problem has more to do with separating the 32-bit array into 4 8-bit arrays, I should have made that clearer in the question. You do make a good point with the efficiency though and if you have any suggestions as to how I could improve that would be great.
    – Stu
    Oct 21 '15 at 3:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.