How to initialize an array in c?

Question

I have a function that should initialize an array of length 3 and return it. See the code below. It's working so far but only for certain colors. I recognized a bug in my code when I tried to write violet. I got a trailing orange element when I write violet. Why is that and how can I fix it? I have more color arrays like this defined and some of them seem to "leak" colors. I guess there's something wrong how I initialize the arrays. Hope someone can shed some light into this.

#define LEDS 64

struct array{ unsigned char hex[LEDS * 3]; };

struct array init_array(int r, int g, int b) {
    int i;
    struct array z;
    //GRB data, 3 bits for each color
    for(i = 0; i < sizeof(struct array) / sizeof(char); i++) {
        if(i == 0) z.hex[i] = g | 128;
        else if(i==1) z.hex[i] = r | 128;
        else z.hex[i] = b | 128;
    }
    return z;
}
...
struct array orange;
struct array violet;
...
violet=init_array(173, 141, 171); 
orange=init_array(192, 145, 128); 

while(1) {
    for(i=0; i<=LEDS; i++) {
        transfer(fd, violet.hex, sizeof (violet.hex));
    }
    for(i=0; i<=LEDS; i++) {
        transfer(fd, orange.hex, sizeof (orange.hex));
    }
}

That's the transfer function:

void transfer(int fd, char* tx, uint32_t len) {
    int ret; 
    uint8_t rx[sizeof (&tx)] = {0,}; // RX buffer

    struct spi_ioc_transfer data = {
        .tx_buf = (unsigned long) tx,
        .rx_buf = (unsigned long) rx,
        .len = len,
        .delay_usecs = delay,
        .speed_hz = speed,
        .bits_per_word = bits,
        .cs_change = 0,
    };

    ret = ioctl(fd, SPI_IOC_MESSAGE(1), &data);
    if (ret < 1)
        pabort("can't send spi message");

    usleep(50);
}

Answer 1

In your code, array type is defined as

struct array {unsigned char hex[LED * 3];};

LED is defined as 64 so you are initializing an array of 3 * 64 = 192 unsigned char which is one byte each. Are you sure this is what you want?

Also, from your init_array code, you are giving green and red one byte each. Note that z.hex[0] is an unsigned char and so is z.hex[1]. For the rest of 190 bytes, you are giving b | 128 for all. This is definitely not how RGB works.

Assuming you want 3 64 bit unsigned values to save each one of the RGB value, instead of going with the for loop once, you should go three loops, each one for R, G or B. In each loop, based on the RGB value, populate the 64 bits which is 8 bytes. So your LED should be defined as 8 for 8 unsigned char each color space.

Answer 2

If your array represents 64 leds, with the colors like so:

+-----------------------------------
| g | r | b | g | r | b | g | r | b  ... and so on
+-----------------------------------

Your code is actually initializing them like so:

+-----------------------------------
| g | r | b | b | b | b | b | b | b   ... and so on
+-----------------------------------

You might want to do this instead to set the colors as in the first example

for(i = 0; i < sizeof(z.hex); i += 3) {
      z.hex[i    ] = g | 128;
      z.hex[i + 1] = r | 128;
      z.hex[i + 2] = b | 128;
}

(And note that the |128 always sets the most significant bit of each color, you need to verify that that's what you really want)

If this is not relevant, please update your post and clarify what you want to do, why LEDS is defined as 64, and what each of the LEDS * 3 = 192 bytes is used for, post the code for transfer() and explain what it is supposed to do.

Answer 3

The unusual, but nearly valid initialization needs an adjustment: int j = i % 3

struct array init_array(int r, int g, int b) {
  int i;
  struct array z;
  printf("%u  ", (unsigned) (sizeof(struct array) / sizeof(char)));
  //GRB data, 3 bits for each color
  for (i = 0; i < sizeof(struct array) / sizeof(char); i++) {
    int j = i % 3;
    if (j == 0)
      z.hex[i] = g | 128;
    else if (j == 1)
      z.hex[i] = r | 128;
    else
      z.hex[i] = b | 128;
  }
  return z;
}

Answer 4

As i understand, in your example you want to change all 64 leds to violett, than to orange, violett, and so on, using equal, 3*8 bit RGB colors.

Consider such a struct:

 struct color {
  char green;
  char red;
  char blue;
 };

Create the color structs on the stack (here, not in the frame of the init fkt!):

 struct color violett, orange;

Initialize them

init_array( *col, int r, int g, int b) {

 col->red = r | 128;
 col->green = g | 128;
 col->blue = b | 128;
}

..

init_array(&violet, 173, 141, 171); 
init_array(&orange, 192, 145, 128);

Pass to your transfer fkt:

while(1) {
 for(i=0; i<=LEDS; i++) {
   transfer(fd, &violet, sizeof (struct color));
 }
 for(i=0; i<=LEDS; i++) {
   transfer(fd, &orange, sizeof (struct color));
 }
}

Write to LEDs with your provided transfer fkt. Have fun with it :-)

Answer 5

Your code has many problems..

for(i = 0; i < LEDS; i++) {

    z.hex[i] = g|128;

    z.hex[i + 1] = r|128;

    z.hex[i + 2] = b|128;

}

And others: return value , transfer by ref...

Sorry, c dose not contain reference, take struct array init_array(int r, int g, int b)for example:

  int init_array(array* z, int r, int g, int b){

int i;
//GRB data, 3 bits for each color
 for(i = 0; i < LEDS; i++) {

    z->hex[i] = g|128;

    z->hex[i + 1] = r|128;

    z->hex[i + 2] = b|128;

}
return z;

}

It uses a point to transfer z into init function instead of return a new array.