Skip to content

Commit 3b9da69

Browse files
committed
Write to LED indices instead of raw pointer
The original implementation violates strict aliasing. It works most of the time but can potentially cause issues. Note that the LED index counters are 32-bit instead of 16-bit because the LED count provided by the checksum has a +1, so it will not fit in a 16-bit variable. Similarly, we need to track both the current LED index and the remaining LEDs to write because the size of the LED strip in the code may not match the size of the LED strip as set by the data protocol.
1 parent 4be60e8 commit 3b9da69

File tree

1 file changed

+31
-14
lines changed

1 file changed

+31
-14
lines changed

Arduino/LEDstream_FastLED/LEDstream_FastLED.ino

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ const uint16_t
5252
#include <FastLED.h>
5353

5454
CRGB leds[Num_Leds];
55-
uint8_t * ledsRaw = (uint8_t *)leds;
5655

5756
// A 'magic word' (along with LED count & checksum) precedes each block
5857
// of LED data; this assists the microcontroller in syncing up with the
@@ -79,8 +78,8 @@ const uint8_t magic[] = {
7978

8079
enum processModes_t {Header, Data} mode = Header;
8180

82-
uint16_t outPos; // current byte index in the LED array
83-
uint32_t bytesRemaining; // count of bytes yet received, set by checksum
81+
uint32_t ledIndex; // current index in the LED array
82+
uint32_t ledsRemaining; // count of LEDs still to write, set by checksum (u16 + 1)
8483

8584
unsigned long lastByteTime; // ms timestamp, last byte received
8685
unsigned long lastAckTime; // ms timestamp, lask acknowledge to the host
@@ -189,10 +188,10 @@ void headerMode(uint8_t c){
189188
chk = c;
190189
if(chk == (hi ^ lo ^ 0x55)) {
191190
// Checksum looks valid. Get 16-bit LED count, add 1
192-
// (# LEDs is always > 0) and multiply by 3 for R,G,B.
191+
// (# of LEDs is always > 0), save and reset data
193192
D_LED(HIGH);
194-
bytesRemaining = 3L * (256L * (long)hi + (long)lo + 1L);
195-
outPos = 0;
193+
ledIndex = 0;
194+
ledsRemaining = (256UL * (uint32_t)hi + (uint32_t)lo + 1UL);
196195
memset(leds, 0, Num_Leds * sizeof(struct CRGB));
197196
mode = Data; // Proceed to latch wait mode
198197
}
@@ -203,16 +202,34 @@ void headerMode(uint8_t c){
203202
}
204203

205204
void dataMode(uint8_t c){
206-
// If LED data is not full
207-
if (outPos < sizeof(leds)){
208-
ledsRaw[outPos++] = c; // Issue next byte
205+
static uint8_t channelIndex = 0;
206+
207+
// if LED data is not full, save the byte
208+
if (ledIndex < Num_Leds) {
209+
leds[ledIndex].raw[channelIndex] = c;
209210
}
210-
bytesRemaining--;
211-
212-
if(bytesRemaining == 0) {
213-
// End of data -- issue latch:
214-
mode = Header; // Begin next header search
211+
channelIndex++; // increment regardless, for oversized data
212+
213+
// if we've filled this LED, move to the next
214+
if (channelIndex >= 3) {
215+
// reset the channel index so we can get ready to write
216+
// the next LED, starting on the first channel (R/G/B)
217+
channelIndex = 0;
218+
219+
// allow this to max out at Num_Leds, so that it represents which
220+
// LEDs have data in them when the strip is ready to write
221+
if (ledIndex < Num_Leds) ledIndex++;
222+
223+
// finished writing one LED, decrement the counter
224+
ledsRemaining--;
225+
}
226+
227+
// if all data has been read, write the output
228+
if (ledsRemaining == 0) {
215229
FastLED.show();
230+
channelIndex = 0; // reset channel tracking
231+
mode = Header; // begin next header search
232+
216233
D_FPS;
217234
D_LED(LOW);
218235
SERIAL_FLUSH;

0 commit comments

Comments
 (0)