Switch-Break LED Difficulty

Projects Programming
1

I am attempting to have multiple types of LED runs that are selectable using numbers in the serial monitor (eventually will be DMX controlled, but this is the first step. LOL).

Case 1 is a 50 light run starting at 1 and going to 50. Case 2 reverses that, going from 50 to 1. The first time I call a case (either 1 or 2), it works properly. Every time after that, regardless of which case I call, the LEDs all turn on at once, rather than in a sequence. Both sets of code work properly when run outside of the switch-break format. What am I missing?`

#include <FastLED.h>

#define LED_PIN     7
#define NUM_LEDS    100


CRGB leds[NUM_LEDS];

void setup() {
  Serial.begin(9600);
  pinMode(7, OUTPUT);
  FastLED.addLeds<WS2812, LED_PIN, GRB>(leds, NUM_LEDS);
  FastLED.clear();
  FastLED.show();
}
int H;
int brightness;
char rx_byte = 0;

void loop() {
  if (Serial.available() > 0) { 
    rx_byte = Serial.read();
          
  
    switch (rx_byte) {
      case '1':
        FastLED.clear();
        FastLED.show(); 
        for (H = 0; H < 49; H++) {
          leds[H] = CRGB ( 255, 255, 255);
          FastLED.show();
          delay (20);}
        for (brightness = 255; brightness >= 0; brightness--){
            FastLED.setBrightness(brightness);
            FastLED.show();
              delay( 1000/256 );}
      break;
      
      case '2':
        FastLED.clear();
        FastLED.show(); 
        for (H = 50; H > 1; H--) {
          leds[H] = CRGB ( 255, 0, 0);
          FastLED.show();
          delay (20);}
        for (brightness = 255; brightness >= 0; brightness--){
            FastLED.setBrightness(brightness);
            FastLED.show();
              delay( 1000/256 );}      
      break;
      
      
    } 
  } 
  }
2

Off the top of my head, may bear no relationship to reality, no fitness for purpose is expressed or implied, your mileage may vary:

Using one or two character upper case names for variables is something best avoided.

0..48

50..2

...seems a bit odd.

As does

But ignoring all of that... once you've gone through either of your cases, your brightness level is set to 0.

So the next time through your loop, setting each LED to a particular colour and doing a show is going to display... anything? Isn't brightness still set to 0?

So the first time you're going to see something is when you send FastLED.setBrightness(255); and all the LEDs suddenly get scaled up to full brightness from full off?

In other words: when you receive a character, first thing, set all LEDs to CRGB(0,0,0), setBrightness(255), then do your cases stuff.

1 Like
3

I know the numbers are all wonky- this is a tester program and I've been going through different iterations, so bits and pieces are not getting changed as they should.

Thank you! The brightness was definitely the piece I was missing.

4

50

You do not have an LED[50] so you will be experiencing out-of-bounds problems. Start this loop at 49 and finish at -1

1 Like
5

I don't know why people are so intent on using these "off-by-one" sort of for loops.
How about a simple:
for (H = 49 H >= 0; H--)
[/quote]

1 Like
6

I don't even run loops backwards.

  for (int ii = 0: ii < 50; ii++) {
    int thePixel = 49 - ii;   // algebra! 49 .. 0

// use thePixel, not ii

  }

A loop is easiest to understand when you can see it do 50 times something as the loop index goes from 0 to 49.

If every loop you write does that.

If you need something that goes backwards, or from a here to a there that isn't 0 .. N - 1, or skips by three or whatever, write a simple expression to calculate the index you want.

Nice too when one loop is making things go forward and backward and double fast at once.

a7

1 Like
7

On all processors but AVR, you can use std::rbegin() and std::rend(). That makes the forward and reverse loops nice and symetric.

#include <FastLED.h>
#include <iterator>

CRGB leds[50];
.
.
.
  // Forward Traversal
  for (auto it = std::begin(leds); it != std::end(leds); it++) {
    *it = CRGB ( 255, 255, 255);
    FastLED.show();
    delay (20);
  }

  // Reverse Traversal
  for (auto it = std::rbegin(leds); it != std::rend(leds); it++) {
    *it = CRGB ( 255, 255, 255);
    FastLED.show();
    delay (20);
  }
8

: should be ;

9

; is missing after 49

H=NUM_LEDS-1;

Would be better...
Even better: use i, j and k for counters.

10

If find this a better solution than subtraction and the "maybe" condition ( maybe > or = )

11

In my opinion for loops should be kept as simple as possible and a backwards loop is quite simple. Why introduce extra complications by doing maths in the loop to derive the value that you really want when it could be directly available ?

As for

This looks more like deliberate obfuscation than a reasonable solution

12

One man's deliberate obfuscation is another man's elegant abstraction. Plus, it's the standard technique used to traverse all C++ container classes, not just simple C-style arrays. Thus, it maintains consistent syntax regardless of the container type.

13

In a subtraction loop, MAX is a counting number (does not include zero), so it needs off-by-one math to address the right device, as the MAX value is actually MAX - 1, which is a whole number (includes zero).

14

I don't see where "off-by-one" maths comes into it.

If you want to access elements 49 to 0 of an array then simply write the for loop to do exactly that

for (int H = 49; H >= 0; H--)
15

In (my) negative counter for() loop, I count to "-1" or start from MAX - 1.