HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arrayslowembeddedlevelchristmasdriverbitpackedlightforstm32f4

Problem

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 > 8 & 0x8) > 8 & 0x4) > 8 & 0x2) > 8 & 0x1) > 4 & 0x8) > 4 & 0x4) > 4 & 0x2) > 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(da

Solution

-
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)

-
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];


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

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

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

-
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)?

Code Snippets

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

Context

StackExchange Code Review Q#68999, answer score: 7

Revisions (0)

No revisions yet.