1

I have seen several explanations about this topic. But i can't seem to fit any solution for my code. I have read: Passing a 2D array to a C++ function https://www.techiedelight.com/pass-2d-array-function-parameter/

and so on.

But i don't know how to make any of the solutions work for me. My problem is following:

First of all im working with LED driver LP5030. Im trying to write some drivers like giving it an array of colors and then blink etc.

I have an array of colors like this:

static uint8_t colors[][3] = {
    { 0x00, 0xFF, 0x00 }, /* Red    */
    { 0xFF, 0x00, 0x00 }, /* Green  */
    { 0x00, 0x00, 0xFF }, /* Blue   */
    { 0xFF, 0xFF, 0xFF }, /* White  */
    { 0xFF, 0xFF, 0x00 }, /* Yellow */
    { 0x00, 0xFF, 0xFF }, /* Purple */
    { 0xFF, 0x00, 0xFF }, /* Cyan   */
    { 0x79, 0xF4, 0x20 }, /* Orange */
};

And i have a following code which just changes colors in this array after every 1000ms. It works.

for(int i = 0; i < colors_size; i++)
{
    set_LP5030_bank_colors(&I2C1Handle, SLAVE_ADDR, colors[i]);
    DelayMs(1000);
}

I have defined set_LP5030_bank_colors as:

void set_LP5030_bank_colors(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr, uint8_t *colors)

Where *colors should be 1 dimensional array like this: {0x00, 0xFF, 0x00} (one row of two dimensional array)

Now i would like to use all of this in a function because i would like to make fade function for example. My fade function should take whole array in instead of one line because i need to use it further.

I would like to use function inside a function like this (i left fade code out to make it simpler):

void fade_LP5030(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr, uint8_t **colors)
{
    uint32_t colors_size = 8;

    for(int i = 0; i < colors_size; i++)
    {   
        set_LP5030_bank_colors(pI2CHandle, SlaveAddr, (uint8_t*)colors[i]);
        DelayMs(10);
    }
}

And i use it like this in main function:

fade_LP5030(&I2C1Handle, SLAVE_ADDR, colors);

It gives me error:

warning: passing argument 3 of 'fade_LP5030' from incompatible pointer type [-Wincompatible-pointer-types]

  210 |   fade_LP5030(&I2C1Handle, SLAVE_ADDR, colors);
      |                                        ^~~~~~
      |                                        |
      |                                        uint8_t (*)[3] {aka unsigned char (*)[3]}

And when i write my main function like this:

fade_LP5030(&I2C1Handle, SLAVE_ADDR, (uint8_t**)colors);

I get no errors but it seems im violating some memory location. Can you give me advice why this way it doesn't work?

Mini Tamm
  • 25
  • 5

3 Answers3

3

Try this:

void fade_LP5030(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr, uint8_t colors[][3])
{
    uint32_t colors_size = 8;

    for(int i = 0; i < colors_size; i++)
    {   
        set_LP5030_bank_colors(pI2CHandle, SlaveAddr, colors[i]);
        DelayMs(10);
    }
}

Note that, in a formal argument list, colors[][3] is equivalent to, and is automatically adjusted to, (*colors)[3].

Make sure there are no pointer casts in your code. In situations like this, any use of a pointer cast is almost certainly a bug.

Tom Karzes
  • 22,815
  • 2
  • 22
  • 41
  • It is working. I tried so many combinations for many hours... and it was that simple. Thank you. I would upvote if i could. – Mini Tamm Mar 21 '23 at 18:39
  • I suggest declaring the parameter `colors` as `uint8_t colors[static 8][3]` to indicate that at least 8 entries in `colors` are valid. A modern C99 compilers will likely raise a warning if such a constrain is violated at compilation time. – tstanisl Mar 21 '23 at 22:06
2

@Tom Karzes is good, yet I would recommend to pass in the array size and use size_t array indexing.

void fade_LP5030(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr, 
    size_t colors_size, uint8_t colors[][3]) { 
  for(size_t i = 0; i < colors_size; i++) {
    set_LP5030_bank_colors(&I2C1Handle, SLAVE_ADDR, colors[i]);
  DelayMs(1000);
  }
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • Yes, i did like that excatly. I just took colors_size argument off because i was having problems with pointers and wanted to make things simpler. I put it back now. I was wondering can i calculate array size inside the function as well so i could simply pass an array, but sizeof function was giving problems when used inside this function. So im passing size as an argument. – Mini Tamm Mar 21 '23 at 20:37
  • @MiniTamm, code can calculate array size inside the function when VLA support exist, something like `void fade_LP5030(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr, size_t colors_size, size_t primary_n, uint8_t (*colors)[colors_size][primary_n]) { size_t array_size = sizeof *colors.` Yet I suggest we do not attempt this approach. – chux - Reinstate Monica Mar 21 '23 at 21:08
  • Yes, it seems to get too complicated for me. I will just pass the size as an argument. – Mini Tamm Mar 22 '23 at 19:05
1

your colors object is a flat 2D array, ie: an array of arrays of 3 uint8_t. You cannot convert this to a pointer to pointers to uint8_t that the function fade_LP5030 expects as a uint8_t **colors argument. The array colors when passed to a function decays to a pointer to its first element, which is an array of 3 uint8_t. The type for this is uint8_t (*colors)[3] but for convenience, function arguments can be defined using the array syntax so you can use uint8_t colors[][3] too.

The function should therefore be defined as

void fade_LP5030(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr,
                 uint8_t colors[][3]) {
    int colors_size = 8;
    for (int i = 0; i < colors_size; i++) {   
        set_LP5030_bank_colors(pI2CHandle, SlaveAddr, colors[i]);
        DelayMs(10);
    }
}

Or interchangeably as:

void fade_LP5030(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr,
                 uint8_t (*colors)[3]) { ...

You can even specify the length of the array, which will be ignored by the compiler, but may help the reader of your code. Note however that only the first dimension can be omitted, the second dimension must be specified in all cases as it determines the size of the outer array element, which is needed to compute colors[i]:

void fade_LP5030(I2C_Handle_t *pI2CHandle, uint8_t SlaveAddr,
                 uint8_t colors[8][3]) { ...
chqrlie
  • 131,814
  • 10
  • 121
  • 189