Go Down

Topic: Switch case variable not updating (Read 606 times) previous topic - next topic

NaCl19

I'm using a Mega to be an LED Strip controller and I'm trying to use the switch case to cycle through various visual outputs for the LED Strip. The problem I'm running into is that the variable used to count the number of button presses, 'visual', updates fine when the 'Visualize()' isn't included in the loop and only the 'CycleVisual();' is included. Once I add the 'Visualize()' the 'visual' only registers one increment and then doesn't update in the Serial Monitor of any more button presses. I want to assume the logic used in 'CycleVisual();' is ill-suited for 'Visualize();', but I'm lost after a couple of days of trying to figure out the problem. A schematic of the board is included above the code. Please disregard the pot and sound module since I want to include the usage of those after I figure out how to cycle through visuals that don't require sound input for now. Thank you for your time!



Code: [Select]
#include <Adafruit_NeoPixel.h> 
#ifdef __AVR__
#include <avr/power.h>   
#endif                   

#define LED_PIN   A5  //Pin for the pixel strand. Can be analog or digital.
#define LED_TOTAL 300  //Change this to the number of LEDs in your strand.
#define LED_HALF  LED_TOTAL/2
#define VISUALS   2   //Change this accordingly if you add/remove a visual in the switch-case in Visualize()
#define BUTTON_1  6 

Adafruit_NeoPixel strand = Adafruit_NeoPixel(LED_TOTAL, LED_PIN, NEO_GRB + NEO_KHZ800);  //LED strand objetcs

uint8_t visual = 0; 

void setup() {
  Serial.begin(9600);
  pinMode(BUTTON_1, INPUT);
  digitalWrite(BUTTON_1, HIGH);
  strand.begin();
  strand.show();
 }

void loop() {
  Serial.println(visual);
  CycleVisual();
  delay(10);
}

void CycleVisual() {
   if (digitalRead( BUTTON_1 ) == LOW ) {
    visual++;
    if (visual > VISUALS) visual = 0;
    delay(350);
  }
}

void Visualize(){
  switch (visual) {
    case 0: return Sparkle(0xff, 0xff, 0xff, 50);
    break;
    case 1: return BouncingBalls(0xff, 0xff, 0xff, 5);
    break;
    case 2: return RunningLights(23, 43, 0, 50);
    break;
    default: Sparkle(0xff, 0xff, 0xff, 50);

  }
}

void showstrand() {
 #ifdef ADAFRUIT_NEOPIXEL_H
   // NeoPixel
   strand.show();
 #endif
 #ifndef ADAFRUIT_NEOPIXEL_H
   // FastLED
   FastLED.show();
 #endif
}

void setPixel(int Pixel, byte red, byte green, byte blue) {
 #ifdef ADAFRUIT_NEOPIXEL_H
   // NeoPixel
   strand.setPixelColor(Pixel, strand.Color(red, green, blue));
 #endif
 #ifndef ADAFRUIT_NEOPIXEL_H
   // FastLED
   leds[Pixel].r = red;
   leds[Pixel].g = green;
   leds[Pixel].b = blue;
 #endif
}

void setAll(byte red, byte green, byte blue) {
  for(int i = 0; i < LED_TOTAL; i++ ) {
    setPixel(i, red, green, blue);
  }
  strand.show();
}

void Sparkle(byte red, byte green, byte blue, int SpeedDelay) {
  int Pixel = random(LED_TOTAL);
  setPixel(Pixel,red,green,blue);
  strand.show();
  delay(SpeedDelay);
  setPixel(Pixel,0,0,0);
  digitalWrite(BUTTON_1, HIGH);
}

void RunningLights(byte red, byte green, byte blue, int WaveDelay) {
  int Position=0;
 
  for(int j=0; j<LED_TOTAL*2; j++)
  {
      Position++; // = 0; //Position + Rate;
      for(int i=0; i<LED_TOTAL; i++) {
        // sine wave, 3 offset waves make a rainbow!
        //float level = sin(i+Position) * 127 + 128;
        //setPixel(i,level,0,0);
        //float level = sin(i+Position) * 127 + 128;
        setPixel(i,((sin(i+Position) * 127 + 128)/255)*red,
                   ((sin(i+Position) * 127 + 128)/255)*green,
                   ((sin(i+Position) * 127 + 128)/255)*blue);
      }
     
      strand.show();
      delay(WaveDelay);
  }
}

void BouncingBalls(byte red, byte green, byte blue, int BallCount) {
  float Gravity = -9.81;
  int StartHeight = 1;
 
  float Height[BallCount];
  float ImpactVelocityStart = sqrt( -2 * Gravity * StartHeight );
  float ImpactVelocity[BallCount];
  float TimeSinceLastBounce[BallCount];
  int   Position[BallCount];
  long  ClockTimeSinceLastBounce[BallCount];
  float Dampening[BallCount];
 
  for (int i = 0 ; i < BallCount ; i++) {   
    ClockTimeSinceLastBounce[i] = millis();
    Height[i] = StartHeight;
    Position[i] = 0;
    ImpactVelocity[i] = ImpactVelocityStart;
    TimeSinceLastBounce[i] = 0;
    Dampening[i] = 0.90 - float(i)/pow(BallCount,2);
  }

  while (true) {
    for (int i = 0 ; i < BallCount ; i++) {
      TimeSinceLastBounce[i] =  millis() - ClockTimeSinceLastBounce[i];
      Height[i] = 0.5 * Gravity * pow( TimeSinceLastBounce[i]/1000 , 2.0 ) + ImpactVelocity[i] * TimeSinceLastBounce[i]/1000;
 
      if ( Height[i] < 0 ) {                     
        Height[i] = 0;
        ImpactVelocity[i] = Dampening[i] * ImpactVelocity[i];
        ClockTimeSinceLastBounce[i] = millis();
 
        if ( ImpactVelocity[i] < 0.01 ) {
          ImpactVelocity[i] = ImpactVelocityStart;
        }
      }
      Position[i] = round( Height[i] * (LED_TOTAL - 1) / StartHeight);
    }
 
    for (int i = 0 ; i < BallCount ; i++) {
      setPixel(Position[i],red,green,blue);
    }
    digitalWrite(BUTTON_1, HIGH);
    strand.show();
    setAll(0,0,0);
  }
}

boolrules

Why are you writing to an input (BUTTON_!) 6 places?

NaCl19

#2
Sep 25, 2018, 02:09 am Last Edit: Sep 25, 2018, 02:23 am by NaCl19
Why are you writing to an input (BUTTON_!) 6 places?
if you're referring to:

#define BUTTON_1  6

I'm defining the pin for the button

groundFungus

#3
Sep 25, 2018, 02:17 am Last Edit: Sep 25, 2018, 02:20 am by groundFungus
 
Code: [Select]
for (int i = 0 ; i < BallCount ; i++) {
      setPixel(Position[i],red,green,blue);
    }
    digitalWrite(BUTTON_1, HIGH);

I think that he meant like here.  It makes sense to write HIGH to the button in setup() to turn on the internal pullup, but nowhere else.

Have a look at the state change example in the IDE for a way to, reliably, read the switch without using any delays.

boolrules

Code: [Select]
  pinMode( BUTTON_1, INPUT );
  digitalWrite (BUTTON_1, HIGH );         // don't get in the habit of doing this

If you need the pullup functionality on a pin, use the following:
Code: [Select]
pinMode( BUTTON_1, INPUT_PULLUP );
It's clear what you intend and it's self documenting.
Writing a HIGH to the pin works but it's one of those arduinoisms that causes nothing but problens -- as it did in your case.

DKWatson

In the real world,

DDRB &= ~(1 << BUTTON_1);
PORTB |= (1 << BUTTON_1);

is how you set a pin for INPUT_PULLUP.

Knowledge is never a bad thing, getting hung-up on Arduinoisms, not applicable elsewhere in the real world, is.
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

TomGeorge

#6
Sep 25, 2018, 04:22 am Last Edit: Sep 25, 2018, 04:24 am by TomGeorge
Hi,
Welcome to the forum.
I prefer;
Code: [Select]

 pinMode( BUTTON_1Pin, INPUT_PULLUP );


As you scan the code, you instantly know its INPUT and PULLUP in one line.

Also BUTTON_1Pin makes sure you are discriminating between a Pin and a variable/constant.

Tom... :)
Everything runs on smoke, let the smoke out, it stops running....

boolrules

Like "libraries", missing main( ), or putting returns in functions that never appear to have a caller?
Here is one place, INPUT_PULLUP, where it actually is not a bad idea, even though it is deceptive, but somehow it's wrong among all the "right" stuff.

TomGeorge

#8
Sep 25, 2018, 04:41 am Last Edit: Sep 25, 2018, 04:41 am by TomGeorge
Hi,


Code: [Select]
switch (visual) {
    case 0: return Sparkle(0xff, 0xff, 0xff, 50);
    break;
    case 1: return BouncingBalls(0xff, 0xff, 0xff, 5);
    break;
    case 2: return RunningLights(23, 43, 0, 50);
    break;
    default: Sparkle(0xff, 0xff, 0xff, 50);

  }

Is there a need for the "return" at the START in each of the case statements.
Isn't that what the "break" at the END of each case does?

Remove the "return" see what happens.

Tom... :)
Everything runs on smoke, let the smoke out, it stops running....

DKWatson

I use macros all the time to simplify the reading and debugging of code. I'm not at all suggesting it's a bad thing so long as the individual understands what is happening 'behind the scenes'. All too often the abstraction offered by the Arduino language, while it's great for beginners, detracts from the learning as the individual never really fully understands what (s)he is doing. And it's not portable. Somewhere along the curve we, as mentors, need to make a decision as to when to let go of the apron strings. If the individual demonstrates advanced, as I believe Saltine has, do you not agree the onus is on us (no pun intended) to nudge him/her beyond the comfort zone?
Live as if you were to die tomorrow. Learn as if you were to live forever. - Mahatma Gandhi

boolrules

@TomGeorge:  I was referring to the usage of a return in loop( ) that some have suggested to "short circuit" the flow and get back to the top of loop( ).  It works, but I hate to think what it must do to a mind trying to learn C/C++.  A break in a switch just drops you out of the switch.

@DKWatson: Looks to me like we basically agree with one another.

PaulS

Code: [Select]
In the real world,

DDRB &= ~(1 << BUTTON_1);
PORTB |= (1 << BUTTON_1);

is how you set a pin for INPUT_PULLUP.

For one specific pin, on one specific Arduino.

For that to work, you need to which DDRx and PORTx the pin is on, and that is not portable.
The art of getting good answers lies in asking good questions.

NaCl19

So after removing the returns and setting the pinmode to INPUT_PULLUP, the code doesn't quite fully work. I have noticed that if I only use the Sparkle() command in the cases,

Ex:

Code: [Select]
    case 0:  Sparkle(0xff, 0, 0, 50);
    break;
    case 1:  Sparkle(0, 0xff, 0, 50);
    break;
    case 2:  Sparkle(0, 0, 0xff, 50);
    break;


The visual counts correctly with each button press and resets after the last case, but if I add the BouncingBall() or RunningLights() the visual counter no longer updates correctly. Is there something I am missing in the code for those two visualizations causing the visual counter not to update?

boolrules


NaCl19

#14
Sep 26, 2018, 02:34 am Last Edit: Sep 26, 2018, 02:37 am by NaCl19
Code: [Select]
#include <Adafruit_NeoPixel.h> 
#ifdef __AVR__
#include <avr/power.h>   
#endif                   

#define LED_PIN   A5  //Pin for the pixel strand. Can be analog or digital.
#define LED_TOTAL 300  //Change this to the number of LEDs in your strand.
#define LED_HALF  LED_TOTAL/2
#define VISUALS   2   //Change this accordingly if you add/remove a visual in the switch-case in Visualize()
#define BUTTON_1  6 

Adafruit_NeoPixel strand = Adafruit_NeoPixel(LED_TOTAL, LED_PIN, NEO_GRB + NEO_KHZ800);  //LED strand objetcs

uint8_t visual = 0; 

void setup() {
  Serial.begin(9600);
  pinMode(BUTTON_1, INPUT_PULLUP);
  strand.begin();
  strand.show();
 }

void loop() {
  Serial.println(visual);
  CycleVisual();
  Visualize();
  delay(10);
}

void CycleVisual() {
   if (digitalRead( BUTTON_1 ) == LOW ) {
    visual++;
    if (visual > VISUALS) visual = 0;
    delay(350);
  }
}

void Visualize(){
  switch (visual) {
    case 0:  Sparkle(0xff, 0, 0, 50);
    break;
    case 1:  Sparkle(0, 0xff, 0, 50);
    break;
    case 2:  Sparkle(0, 0, 0xff, 50);
    break;
    default: Sparkle(0xff, 0xff, 0xff, 50);
  }
}

void showstrand() {
 #ifdef ADAFRUIT_NEOPIXEL_H
   // NeoPixel
   strand.show();
 #endif
 #ifndef ADAFRUIT_NEOPIXEL_H
   // FastLED
   FastLED.show();
 #endif
}

void setPixel(int Pixel, byte red, byte green, byte blue) {
 #ifdef ADAFRUIT_NEOPIXEL_H
   // NeoPixel
   strand.setPixelColor(Pixel, strand.Color(red, green, blue));
 #endif
 #ifndef ADAFRUIT_NEOPIXEL_H
   // FastLED
   leds[Pixel].r = red;
   leds[Pixel].g = green;
   leds[Pixel].b = blue;
 #endif
}

void setAll(byte red, byte green, byte blue) {
  for(int i = 0; i < LED_TOTAL; i++ ) {
    setPixel(i, red, green, blue);
  }
  strand.show();
}

void Sparkle(byte red, byte green, byte blue, int SpeedDelay) {
  int Pixel = random(LED_TOTAL);
  setPixel(Pixel,red,green,blue);
  strand.show();
  delay(SpeedDelay);
  setPixel(Pixel,0,0,0);
}

void RunningLights(byte red, byte green, byte blue, int SpeedDelay) {
  int Position=0;
  for(int i=0; i<LED_TOTAL; i++)
  {
      Position++; // = 0; //Position + Rate;
      for(int i=0; i<LED_TOTAL; i++) {
        setPixel(i,((sin(i+Position) * 127 + 128)/255)*red,
                   ((sin(i+Position) * 127 + 128)/255)*green,
                   ((sin(i+Position) * 127 + 128)/255)*blue);
      }
      strand.show();
      delay(SpeedDelay);
  }
}


Go Up