Programming efficiency

Projects Programming
1

Hi I have the following :
the issue is that when i press both left/right buttons (=High) it does the following:

-LeftBlinkerState - RightBlinkerState - then strobe effect .

what i need is the strobe only to work when both are high, something i am doing wrong in the loop...

#include <Adafruit_NeoPixel.h>
#include "OneButton.h"
#include <EEPROM.h>

byte address = 0;
int i;

#define LeftSignal 2
#define RightSignal 3
OneButton button(4, false); //OneButton(int pin, int activeLow, bool pullupActive)
#define PIN 13
#define BUTTON 5
#define NUM_PIXELS  16

Adafruit_NeoPixel strip = Adafruit_NeoPixel(NUM_PIXELS, PIN, NEO_GRB + NEO_KHZ800);
int brightnessCounter = EEPROM.read(0);
int LeftBlinkerState = 0;
int RightBlinkerState = 0;
int optionread = 0;
int runLightState = 0;
int AMBER = strip.Color(255, 130, 0);
int OFF = strip.Color(0, 0, 0);

void setup()
{
  Serial.begin(115200);  //start Serial in case we need to print debugging info
  pinMode(LeftSignal, INPUT);
  pinMode(RightSignal, INPUT);
  pinMode(BUTTON, INPUT);

  //*********************************************
  button.attachDoubleClick(doubleclick);      // link the function to be called on a doubleclick event.
  button.attachClick(singleclick);            // link the function to be called on a singleclick event.
  button.attachLongPressStop(longclick);        // link the function to be called on a longpress event.
  //*********************************************

  EEPROM.begin();
  strip.begin();
  strip.setBrightness(brightnessCounter);
  strip.show();
  knightRider(1, 16, 5, 0xFF0000);
  knightRider(1, 16, 5, 0x0000FF);
  knightRider(1, 16, 2, 0xFFFFFF);
  //attachInterrupt (digitalPinToInterrupt(BUTTON), changeEffect, CHANGE); // pressed 2 or 3 only
  Serial.println(brightnessCounter);

}

void loop()
{
  button.tick();                                    // check the status of the button

  LeftBlinkerState = digitalRead(LeftSignal);
  //float LeftVoltage = LeftBlinkerState * (5.0 / 1023.0);
  RightBlinkerState = digitalRead(RightSignal);
  //float RightVoltage = RightBlinkerState * (5.0 / 1023.0);
  //optionread = digitalRead(Option);

  if (LeftBlinkerState == HIGH)
  {
    runLightState = 0;
    clearStrip();
    leftTurn();\
  }

  if (RightBlinkerState == HIGH)
  {
    runLightState = 0;
    clearStrip();
    rightTurn();
  }

  if (LeftBlinkerState == LOW && RightBlinkerState == LOW)
  {
    runLight();
  }

  if (LeftBlinkerState == HIGH && RightBlinkerState == HIGH)
  {
    clearStrip();
    Strobe(0x00, 0x00, 0xff, 5, 20, 500);
    Strobe(0xff, 0x00, 0x00, 5, 20, 500);
  }

}

void Strobe(byte red, byte green, byte blue, int StrobeCount, int FlashDelay, int EndPause) {
  for (int j = 0; j < StrobeCount; j++) {
    setAll(red, green, blue);
    showStrip();
    delay(FlashDelay);
    setAll(0, 0, 0);
    showStrip();
    delay(FlashDelay);
  }
  delay(EndPause);
}

void(* resetFunc) (void) = 0;

void singleclick()
{
  SnowSparkle(0x00, 0x00, 0x00, i, 10); // black background / white sparkle
}

void longclick()
{
  Indicator();
}

void doubleclick()
{
  resetFunc();
}

void Indicator()
{
  runLightState = 0;
  clearStrip();
  strip.setBrightness(brightnessCounter);
  brightnessCounter = brightnessCounter + 51;
  Serial.println(brightnessCounter);

  if (brightnessCounter == 102)
  {
    knightRider(2, 16, 4, 0xFF0000);
  }
  else if (brightnessCounter == 153)
  {
    knightRider(2, 16, 4, 0xFFFF00);
  }
  else if (brightnessCounter == 204)
  {
    knightRider(2, 16, 4, 0xFFFFFF);
  }
  else if (brightnessCounter == 255)
  {
    knightRider(2, 16, 4, 0x00FFFF);
  }
  else if (brightnessCounter == 306)
  {
    knightRider(2, 16, 4, 0xFF00FF);
    brightnessCounter = 51;
  }
  EEPROM.update(0, brightnessCounter);
}

void SnowSparkle(byte red, byte green, byte blue, int SparkleDelay, int SpeedDelay)
{
  setAll(red, green, blue);
  int Pixel = random(NUM_PIXELS);// number of pixel to sparkle over
  setPixel(Pixel, 0xff, 0xff, 0xff);
  showStrip();
  delay(SparkleDelay);
  setPixel(Pixel, 0xff, 0xff, 0xff); //set pixel color to show RGB
  showStrip();
  delay(SpeedDelay);
}

void rightTurn()
{
  strip.setBrightness(brightnessCounter);
  for (uint16_t i = 0; i < NUM_PIXELS; i++)
  {
    strip.setPixelColor(i, AMBER);
    strip.show();
    delay(30);
  }

  for (uint16_t i = 0; i < NUM_PIXELS; i++)
  {
    strip.setPixelColor(i, OFF);
    strip.show();
    delay(10);
  }
}
//************************************************************************************************
void leftTurn()
{
  strip.setBrightness(brightnessCounter);
  for (int16_t i = (NUM_PIXELS - 1); i > -1; i--)
  {
    strip.setPixelColor(i, AMBER);
    strip.show();
    delay(30);
  }

  for (int16_t i = (NUM_PIXELS - 1); i > -1; i--)
  {
    strip.setPixelColor(i, OFF);
    strip.show();
    delay(10);
  }
}


void all(uint32_t c)
{
  for (uint16_t i = 0; i < strip.numPixels(); i++) {
    strip.setPixelColor(i, c);
  }
  strip.show();
}


void runLight()
{
  strip.setBrightness(brightnessCounter / 2);
  if (runLightState == 0)
  {
    spread(15, 0xFFFFFF);
    runLightState = 1;
  }
  all(strip.Color(255, 255, 255));
  strip.show();
}


void knightRider(uint16_t cycles, uint16_t speed, uint8_t width, uint32_t color)
{
  uint32_t old_val[NUM_PIXELS]; // up to 256 lights!
  for (int i = 0; i < cycles; i++) {
    for (int count = 1; count < NUM_PIXELS; count++) {
      strip.setPixelColor(count, color);
      old_val[count] = color;
      for (int x = count; x > 0; x--) {
        old_val[x - 1] = dimColor(old_val[x - 1], width);
        strip.setPixelColor(x - 1, old_val[x - 1]);
      }
      strip.show();
      delay(speed);
    }

    for (int count = NUM_PIXELS - 1; count >= 0; count--)
    {
      strip.setPixelColor(count, color);
      old_val[count] = color;
      for (int x = count; x <= NUM_PIXELS ; x++) {
        old_val[x - 1] = dimColor(old_val[x - 1], width);
        strip.setPixelColor(x + 1, old_val[x + 1]);
      }
      strip.show();
      delay(speed);
    }

  }
}

void clearStrip()
{
  for ( int i = 0; i < NUM_PIXELS; i++)
  {
    strip.setPixelColor(i, 0x000000);
    strip.show();
  }
}

uint32_t dimColor(uint32_t color, uint8_t width) {
  return (((color & 0xFF0000) / width) & 0xFF0000) + (((color & 0x00FF00) / width) & 0x00FF00) + (((color & 0x0000FF) / width) & 0x0000FF);
}

void spread(uint16_t speed, uint32_t color) {
  clearStrip();
  delay(300);
  int center = NUM_PIXELS / 2;
  for (int x = 0; x < center; x++) {
    strip.setPixelColor(center + x, color);
    strip.setPixelColor(center + (x * -1), color);
    strip.show();
    delay(speed);
  }
}


void showStrip()
{
#ifdef ADAFRUIT_NEOPIXEL_H
  // NeoPixel
  strip.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
  strip.setPixelColor(Pixel, strip.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 < 16; i++ ) {
    setPixel(i, red, green, blue);
  }
  showStrip();
}
2

You need to fully decode the 4 possible states of left and right. Obviously if you only check if LEFT is HIGH, and ignore RIGHT when doing this, that code will execute solely on the state of LEFT regardless of if RIGHT is HIGH OR LOW. Similarly for RIGHT.

if (LeftBlinkerState == HIGH && RightBlinkerState == HIGH) {
...
} else if (LeftBlinkerState == LOW && RightBlinkerState == LOW) {
...
} else if (LeftBlinkerState == HIGH) {
...
} else if (RightBlinkerState == HIGH) { //Edit: Actually this test is redundant as it is the only combination left, so could just be  } else {
...
}

Or in your existing code with sequential instead of nested if’s, where you are testing left and right ONLY against HIGH, you must test the OTHER side also...

if (LeftBlinkerState == HIGH && RightBlinkerState == LOW) {
...
}

if (LeftBlinkerState == LOW && RightBlinkerState == HIGH) {
...
}
3

pcbbc:
You need to fully decode the 4 possible states of left and right. Obviously if you only check if LEFT is HIGH, and ignore RIGHT when doing this, that code will execute solely on the state of LEFT regardless of if RIGHT is HIGH OR LOW. Similarly for RIGHT.

if (LeftBlinkerState == HIGH && RightBlinkerState == HIGH) {

...
} else if (LeftBlinkerState == LOW && RightBlinkerState == LOW) {
...
} else if (LeftBlinkerState == HIGH) {
...
} else if (RightBlinkerState == HIGH) { //Edit: Actually this test is redundant as it is the only combination left, so could just be  } else {
...
}




Or in your existing code with sequential instead of nested if’s, where you are testing left and right ONLY against HIGH, you must test the OTHER side also...

if (LeftBlinkerState == HIGH && RightBlinkerState == LOW) {
...
}

if (LeftBlinkerState == LOW && RightBlinkerState == HIGH) {
...
}

changed it to the following: seemed to respond better now - thanks
but a little delay where either right or left triggers before constant both = high

void loop()
{
  button.tick();                                    // check the status of the button

  LeftBlinkerState = digitalRead(LeftSignal);
  //float LeftVoltage = LeftBlinkerState * (5.0 / 1023.0);
  RightBlinkerState = digitalRead(RightSignal);
  //float RightVoltage = RightBlinkerState * (5.0 / 1023.0);
  //optionread = digitalRead(Option);

  if (LeftBlinkerState == HIGH && RightBlinkerState == LOW)
  {
    runLightState = 0;
    clearStrip();
    leftTurn();
  }

  if (RightBlinkerState == HIGH && LeftBlinkerState == LOW)
  {
    runLightState = 0;
    clearStrip();
    rightTurn();
  }

  if (LeftBlinkerState == HIGH && RightBlinkerState == HIGH)
  {

    Strobe(0x00, 0x00, 0xff, 5, 20, 500);
    Strobe(0xff, 0x00, 0x00, 5, 20, 500);
  }

  if (LeftBlinkerState == LOW && RightBlinkerState == LOW)
  {
    runLight();
  }

}
4

You need to read the tutorials about how to blink without delay. While your code is off running your flash patterns, most of which involve delay(), it isn’t checking for button presses.

You may also want to wait for a bit after a button changes state, before getting the program to change pattern. That’s because it’s not possible for a user to press both buttons exactly simultaneously. So you might accidentally start the LEFT flash pattern, when the user really wanted the BOTH pattern, just that the right button was pressed a fraction of a second too late.

However, if you implement your “blink without delay” changes correctly the above probably won’t be noticeable, since it should immediately switch to the correct pattern immediately the second button press is detected (not after one cycle of pattern as it currently does).

5

thanks for insight
actually the simultaenous button press is mimic of hazard turn signal.

i tried to kind of change it to removing delays with example but found it challenging
if you can assist me in on example of my patterns without delay i can port it to the rest

pcbbc:
You need to read the tutorials about how to blink without delay. While your code is off running your flash patterns, most of which involve delay(), it isn’t checking for button presses.

You may also want to wait for a bit after a button changes state, before getting the program to change pattern. That’s because it’s not possible for a user to press both buttons exactly simultaneously. So you might accidentally start the LEFT flash pattern, when the user really wanted the BOTH pattern, just that the right button was pressed a fraction of a second too late.

However, if you implement your “blink without delay” changes correctly the above probably won’t be noticeable, since it should immediately switch to the correct pattern immediately the second button press is detected (not after one cycle of pattern as it currently does).