Strange issue with function using arrays

Hello,

I am having a problem I can't figure out. I have a bunch of arrays that I am using to set neopixels into letters.

I have all 26 letters as arrays, which I am trying to pass into a function that makes the letters.

What happens here is that if I pass one array as an argument into the function, it works fine. If I have one array followed by another, the program locks up and nothing happens. I've had a few people look at this

I am doing this on a Gemma, but I'm not getting any errors regarding memory usage.

The code below should form an 'A', wait two seconds, then form a 'B' - but it doesn't. It works fine if I comment out the second letter. The code compiles fine in both cases. Any ideas?

#include <Adafruit_NeoPixel.h>
#include <avr/power.h>

#define PIN 1

#define NUMPIXELS 63

Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_GRB + NEO_KHZ800);
const int buttonPin = 0; // the number of the pushbutton pin
int buttonState = 0;

int delayval = 1; // delay for half a second
//letters
int let_a[] = {0, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1};

int let_b[] = {1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0};

void setup() {
// This is for Trinket 5V 16MHz, you can remove these three lines if you are not using a Trinket
#if defined (AVR_ATtiny85)
if (F_CPU == 16000000) clock_prescale_set(clock_div_1);
#endif
// End of trinket special code
pixels.begin(); // This initializes the NeoPixel library.
pinMode(buttonPin, INPUT);
}

void loop() {
buttonState = digitalRead(buttonPin);
if (buttonState == HIGH) {
letter(let_a);
delay(2000);
letter(let_b);// if you comment this line out, it works.
}
else {
shutOff();
}

}

void letter(int array[])
{
for (int i = 0; i < NUMPIXELS; i++) {
if (array == 1) {

  • pixels.setPixelColor(i, pixels.Color(255, 0, 0));*
  • pixels.show();*
  • delay(delayval);*
  • }*
  • else {*
  • pixels.setPixelColor(i, pixels.Color(0, 0, 0)); // off*
  • pixels.show();*
  • }*
  • }*
    }
    void shutOff()
    {
  • for (int i = NUMPIXELS; i >= 0; i--) {*
  • // pixels.Color takes RGB values, from 0,0,0 up to 255,255,255*
  • pixels.setPixelColor(i, pixels.Color(0, 0, 0));*
  • pixels.show();*
  • //delay(10);*
  • }*
    }

You're out of memory - do the math!

int let_a[] = {0, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 1};

Why are you using an int to store a bit? Your array could be a lot smaller - 8 bytes instead of 166 bytes.

You posted your code incorrectly, so the forum kindly mangled it for you. Read the stickies at the top of the forum - the ones you were supposed to read first - and modify your post.

"but it doesn't" doesn't give us anything to go on. "but is blows smoke out the pixels and makes naked girls appear" gives us a clue what is actually happening, and gives us some idea what the problem might be (aside from wasted memory).

Your leta[] and letb[] waste 15 bits for every one bit that you actually use.

int delayval = 1; // delay for half a second

Yeah, right.

UnUnUnium:
#define NUMPIXELS 63

lucky for you, you can store the entire led array in one uint64_t var:

const uint64_t let_a = 0b011111110100000001100000001111111111100000001100000001100000001;

and traverse the bit mask something like this:

void letter(uint64_t bitMask)
{
  for (int i = 0; i < NUMPIXELS; i++)
  {
    pixels.setPixelColor(i, bitRead(bitMask, 0 - i)? (255, 0, 0) : (0, 0, 0)); // watch for the endianness of the array versus the bit mask!
    pixels.show();
    delay(delayval);
  }
}

not tested, not compiled

I don't have the hardware to test but I'm pretty sure BulldogLowell's code WILL have endian related issues.

The following SHOULD work and has the advantage of the ALPHABET array having a visual layout that allows for eyeballing the bitmaps and making adjustments.

Either of which should be modified to place the lookup tables into PROGMEM.

#include <limits.h>

#define ENTRIES_COUNT(ARRAY)	(sizeof(ARRAY) / sizeof(ARRAY[0]))

uint8_t ALPHABET[][8] =
{
    // A
    {
          0b00000000
        , 0b00011000
        , 0b00111100
        , 0b01100110
        , 0b01100110
        , 0b01111110
        , 0b01100110
        , 0b00000000
    }
    ,
    // B
    {
          0b00000000
        , 0b01111100
        , 0b01100110
        , 0b01111100
        , 0b01100110
        , 0b01100110
        , 0b01111100
        , 0b00000000
    }
    
    // ... OTHER CHARACTERS
};


void character(uint8_t entry)
{
    for ( size_t n = 0; n < ENTRIES_COUNT(ALPHABET[entry]); n++ )
    {
        size_t mask = 1 << ((CHAR_BIT * sizeof(ALPHABET[entry][n])) - 1);

        for ( size_t i = 0; mask; mask >>= 1, i++)
        {
            uint32_t color = (ALPHABET[entry][n] & mask) ? pixels.Color(255, 0, 0) : pixels.Color(0, 0, 0);

            pixels.setPixelColor(i, color);
            pixels.show();
            delay(delayval);
        }
    }
}

lloyddean:
I don't have the hardware to test but I'm pretty sure BulldogLowell's code WILL have endian related issues.

it MAY have an endian issue (not having taken the time to re-order his LED array) that CAN be overcome, but it WILL work! :wink:

I was sure that ordered bytes would work, as I don't know how OP wired them (sequence).

Since the device in question is little-endian it WILL indeed work! I just couldn't remember and wasn't certain.

lloyddean:
I don't have the hardware to test but I'm pretty sure BulldogLowell's code WILL have endian related issues.

it won't do. The operations are defined in terms of numeric values (expressed in binary), and it's the compiler's job to straighten this out.

if I go

int a = 0xAA55;
int b = a & (1<<16);

the compiler must generate code that results in b being set to zero, or else the compiler is simply wrong.

You get endian issues when you start fooling around with addressing the bytes in an int using byte pointers. That is, this:

int a = 0xAA55;
int b = *(byte*)&a;

Might put AA or 55 into b, depending on endian-ness. Likewise, using unions to split ints into bytes. This:

union Splitter {
  int x;
  struct {
    byte y1;
    byte y2;
  } y;
} foo;

foo.x = 0xAA55;
int b = foo.y.y1;

Again will give you either AA or 55 in b, or potentially something even weirder depending on how the compiler cooses to lay out structures and unions in memory (eg: back in the day, some x86 compilers would try to lay out things on 4-byte boundaries).

It just isn't necessary to be this clever. Hopefully, when a compiler sees this

x = (y>>8);

It's clever enough to go "shoot! I can do that by just grabbing the high byte of y! On this machine, that would be the second byte."

PaulMurrayCbr:
if I go

int a = 0xAA55;

int b = a & (1<<16);



the compiler **must** generate code that results in b being set to zero, or else the compiler is simply wrong.

That's not what the standard says.

6.5.7 Bitwise shift operators
...
If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined.

> avr-gcc -fsyntax-only /tmp/shift.c
/tmp/shift.c:2:1: warning: left shift count >= width of type [enabled by default]
 int b = a & (1<<16);
 ^