Embedded C# bitpacked arrays to low-level STM32F4 driver for GE G35 RGB LED Christmas tree light

Question

I'm going to be opensourcing some code I'm working on. I don't need help with the code, I just want to make sure my code is readable and my comments make sense. I have a knack for the esoteric. This code is to control GE G35 Christmas lights using a Netduino controller. Due to the .NET overhead, I've written a custom lower-level driver compiled into the firmware.

Can you follow my code? This is on an embedded processor which is why I'm doing a lot of bit-shifting. It's easier to write this way.

   public Int32[,] getData()
    {
        int maxBulbs = getMaxBulbs();

        // Using an abdnormal form of bitpacking here to make the C loop, very tight and efficient.
        // The first address of the array is the bulb position on all strings.
        // The second address of the array is actually the corresponding bulb information for that bit.
        // Within the array, we store a 32-bit int. Each bit in this int, represents a G35 Strand/string
        // So at data[0, 0] we have a 32 bit int, this int represents the first bulb on all strings, first data information bit for up to 32 strands.
        Int32[,] data = new Int32[getMaxBulbs(), 26]; // number bulbs on a strand, 26 bit bulb info

        foreach (G35String gstring in Strings)
        {
            for (short c_bulb = 1; c_bulb < maxBulbs; c_bulb++)
            {
                // This is a bit of a shortcut. because we know that G35's just pass the information down
                // the pipe and due to the way we are sending data in parallel, if one string has
                // more bulbs than another, we just send fake data to the non existent bulb on that string
                G35Bulb gbulb = (c_bulb < gstring.bulbs.Length ? gstring.bulbs[c_bulb] : new G35Bulb(0, 0));

                // bulb address
                data[c_bulb, 0] = (c_bulb & 0x20) << gstring.StringPinAddress;
                data[c_bulb, 1] = (c_bulb & 0x10) << gstring.StringPinAddress;
                data[c_bulb, 2] = (c_bulb & 0x08) << gstring.StringPinAddress;
                data[c_bulb, 3] = (c_bulb & 0x04) << gstring.StringPinAddress;
                data[c_bulb, 4] = (c_bulb & 0x02) << gstring.StringPinAddress;
                data[c_bulb, 5] = (c_bulb & 0x01) << gstring.StringPinAddress;

                // bulb brightness
                data[c_bulb, 6] = (gbulb.BulbBrightness & 0x80) << gstring.StringPinAddress;
                data[c_bulb, 7] = (gbulb.BulbBrightness & 0x40) << gstring.StringPinAddress;
                data[c_bulb, 8] = (gbulb.BulbBrightness & 0x20) << gstring.StringPinAddress;
                data[c_bulb, 9] = (gbulb.BulbBrightness & 0x10) << gstring.StringPinAddress;
                data[c_bulb, 10] = (gbulb.BulbBrightness & 0x08) << gstring.StringPinAddress;
                data[c_bulb, 11] = (gbulb.BulbBrightness & 0x04) << gstring.StringPinAddress;
                data[c_bulb, 12] = (gbulb.BulbBrightness & 0x02) << gstring.StringPinAddress;
                data[c_bulb, 13] = (gbulb.BulbBrightness & 0x01) << gstring.StringPinAddress;

                // Blue
                data[c_bulb, 14] = (gbulb.BulbColor >> 8 & 0x8) << gstring.StringPinAddress;
                data[c_bulb, 15] = (gbulb.BulbColor >> 8 & 0x4) << gstring.StringPinAddress;
                data[c_bulb, 16] = (gbulb.BulbColor >> 8 & 0x2) << gstring.StringPinAddress;
                data[c_bulb, 17] = (gbulb.BulbColor >> 8 & 0x1) << gstring.StringPinAddress;

                // Green
                data[c_bulb, 18] = (gbulb.BulbColor >> 4 & 0x8) << gstring.StringPinAddress;
                data[c_bulb, 19] = (gbulb.BulbColor >> 4 & 0x4) << gstring.StringPinAddress;
                data[c_bulb, 20] = (gbulb.BulbColor >> 4 & 0x2) << gstring.StringPinAddress;
                data[c_bulb, 21] = (gbulb.BulbColor >> 4 & 0x1) << gstring.StringPinAddress;

                // Red
                data[c_bulb, 22] = (gbulb.BulbColor & 0x8) << gstring.StringPinAddress;
                data[c_bulb, 23] = (gbulb.BulbColor & 0x4) << gstring.StringPinAddress;
                data[c_bulb, 24] = (gbulb.BulbColor & 0x2) << gstring.StringPinAddress;
                data[c_bulb, 25] = (gbulb.BulbColor & 0x1) << gstring.StringPinAddress;
            }
        }

        return data;
    }

In the C++ driver, I can blast all the registers I need like this in parallel:

// LED Address
sendBits(data[i][0]);
sendBits(data[i][1]);
sendBits(data[i][2]);
sendBits(data[i][3]);
sendBits(data[i][4]);
sendBits(data[i][5]);

void sendBits(uint16_t gpioPins, uint32_t data)
{
    *_BSRRL = gpioPins;
    delayMicroseconds(DELAYSHORT); // 10us
    *_BSRRH = gpioPins;
    delayMicroseconds(DELAYSHORT); // 10us
    *_BSRRL = ~data;
    delayMicroseconds(DELAYSHORT); // 10us
    *_BSRRL = gpioPins;
}

Answer 1

1. Standard C# naming convention for methods is PascalCase. Following standard naming conventions makes the code look more familiar to other C# developers (which might be important as you plan to open source it)

2. You are potentially wasting some cycles here by calling getMaxBulbs() again even though you just did it and have the result stored in maxBulbs:

   Int32[,] data = new Int32[getMaxBulbs(), 26];
   

3. c_bulb is not a very nice name. bulbIndex would have been better.

4. gbulb should probably just be bulb as it represents a bulb object.

5. Rather than using the somewhat generic names of Strings for a string of bulbs maybe BulbString would be better.

6. I might be missing something but your outer loop which loops over Strings constantly overwrites the content of data and only the values from the last iteration will actually be in there due to the assignment operator. Did you mean to write:

   data[c_bulb, 0] |= (c_bulb & 0x20) << gstring.StringPinAddress;
   

   instead (OR-ing the bit masks)?

Answer 2

Here's the new code with @ChrisWue's suggestions.

using System;
using Microsoft.SPOT;

namespace Netduino.G35.Main
{
public class G35Cluster
{
    public G35String[] BulbStrings;

    public int getMaxBulbs()
    {
        int maxBulbs = 0;

        foreach (G35String bulbString in BulbStrings)
        {
            maxBulbs = (maxBulbs > bulbString.Bulbs.Length ? bulbString.Bulbs.Length : maxBulbs) + 1; // we add 1 to keep the bulb addressing below simple
        }

        return maxBulbs;
    }

    public void Render()
    {
        UInt32 gpioPins = 0;
        foreach (G35String gstring in this.BulbStrings)
        {
            // This uses the OR equals operator to flip the pin address bit.
            // This will tell the low-level driver which pins it can flip to
            // high or low for pin state management
            // So if for some reason, you had G35 strings on GPIO pins 1, 4, 8
            // Then the binary of this would look like, B00000000000000000000000010001001
            gpioPins |= (UInt32)(1 << gstring.StringPinAddress);
        }
    }

    public Int32[,] getData()
    {
        int maxBulbs = getMaxBulbs();

        // Using an abdnormal form of bitpacking here to make the C loop, very tight and efficient.
        // The first address of the array is the bulb position on all strings.
        // The second address of the array is actually the corresponding bulb information for that bit.
        // Within the array, we store a 32-bit int. Each bit in this int, represents a G35 Strand/string
        // So at data[0, 0] we have a 32 bit int, this int represents the first bulb on all strings, first data information bit for up to 32 strands.
        Int32[,] data = new Int32[maxBulbs, 26]; // number bulbs on a strand, 26 bit bulb info

        foreach (G35String bulbString in BulbStrings)
        {
            for (short bulbIndex = 1; bulbIndex < maxBulbs; bulbIndex++)
            {
                // This is a bit of a shortcut. because we know that G35's just pass the information down
                // the pipe and due to the way we are sending data in parallel, if one string has
                // more bulbs than another, we just send fake data to the non existent bulb on that string
                G35Bulb bulb = (bulbIndex < bulbString.Bulbs.Length ? bulbString.Bulbs[bulbIndex] : new G35Bulb(0, 0));

                // bulb address
                data[bulbIndex, 0] |= (bulbIndex & 0x20) << bulbString.StringPinAddress;
                data[bulbIndex, 1] |= (bulbIndex & 0x10) << bulbString.StringPinAddress;
                data[bulbIndex, 2] |= (bulbIndex & 0x08) << bulbString.StringPinAddress;
                data[bulbIndex, 3] |= (bulbIndex & 0x04) << bulbString.StringPinAddress;
                data[bulbIndex, 4] |= (bulbIndex & 0x02) << bulbString.StringPinAddress;
                data[bulbIndex, 5] |= (bulbIndex & 0x01) << bulbString.StringPinAddress;

                // bulb brightness
                data[bulbIndex, 6] |= (bulb.BulbBrightness & 0x80) << bulbString.StringPinAddress;
                data[bulbIndex, 7] |= (bulb.BulbBrightness & 0x40) << bulbString.StringPinAddress;
                data[bulbIndex, 8] |= (bulb.BulbBrightness & 0x20) << bulbString.StringPinAddress;
                data[bulbIndex, 9] |= (bulb.BulbBrightness & 0x10) << bulbString.StringPinAddress;
                data[bulbIndex, 10] |= (bulb.BulbBrightness & 0x08) << bulbString.StringPinAddress;
                data[bulbIndex, 11] |= (bulb.BulbBrightness & 0x04) << bulbString.StringPinAddress;
                data[bulbIndex, 12] |= (bulb.BulbBrightness & 0x02) << bulbString.StringPinAddress;
                data[bulbIndex, 13] |= (bulb.BulbBrightness & 0x01) << bulbString.StringPinAddress;

                // Blue
                data[bulbIndex, 14] |= (bulb.BulbColor >> 8 & 0x8) << bulbString.StringPinAddress;
                data[bulbIndex, 15] |= (bulb.BulbColor >> 8 & 0x4) << bulbString.StringPinAddress;
                data[bulbIndex, 16] |= (bulb.BulbColor >> 8 & 0x2) << bulbString.StringPinAddress;
                data[bulbIndex, 17] |= (bulb.BulbColor >> 8 & 0x1) << bulbString.StringPinAddress;

                // Green
                data[bulbIndex, 18] |= (bulb.BulbColor >> 4 & 0x8) << bulbString.StringPinAddress;
                data[bulbIndex, 19] |= (bulb.BulbColor >> 4 & 0x4) << bulbString.StringPinAddress;
                data[bulbIndex, 20] |= (bulb.BulbColor >> 4 & 0x2) << bulbString.StringPinAddress;
                data[bulbIndex, 21] |= (bulb.BulbColor >> 4 & 0x1) << bulbString.StringPinAddress;

                // Red
                data[bulbIndex, 22] |= (bulb.BulbColor & 0x8) << bulbString.StringPinAddress;
                data[bulbIndex, 23] |= (bulb.BulbColor & 0x4) << bulbString.StringPinAddress;
                data[bulbIndex, 24] |= (bulb.BulbColor & 0x2) << bulbString.StringPinAddress;
                data[bulbIndex, 25] |= (bulb.BulbColor & 0x1) << bulbString.StringPinAddress;
            }
        }

        return data;
    }
}
}

In the C++ driver, I can blast all the registers I need like this in parallel:

// LED Address
sendBits(data[i][0]);
sendBits(data[i][1]);
sendBits(data[i][2]);
sendBits(data[i][3]);
sendBits(data[i][4]);
sendBits(data[i][5]);

void sendBits(uint16_t gpioPins, uint32_t data)
{
    *_BSRRL = gpioPins;
    delayMicroseconds(DELAYSHORT); // 10us
    *_BSRRH = gpioPins;
    delayMicroseconds(DELAYSHORT); // 10us
    *_BSRRL = ~data;
    delayMicroseconds(DELAYSHORT); // 10us
    *_BSRRL = gpioPins;
}