Using a analog pot to control leds

[madinfo]

Hi, i am making a simple program to control 2 leds.
here is the code:

int pressCnt = 0;

const int LED1 = 8;       //  pins
const int LED2 = 9;       //  pins

void loop() {
  int sensorValue = analogRead(A5);

if (sensorValue > 800) {
   ledcontrol(0);
  }
if ((sensorValue >= 600) && (sensorValue < 799))  {
   ledcontrol(1);
  }
if (sensorValue < 599) {
   ledcontrol(2);
  }
  
  }

void ledcontrol(int a){

switch (a){
    case 2: // all led on
      digitalWrite(LED1, HIGH);
      digitalWrite(LED2, HIGH);
      pressCnt++;
      break;
    case 1: // just one led on 
      digitalWrite(LED1, pressCnt % 2);
      digitalWrite(LED2, !(pressCnt % 2));
      break;
    case 0: // all led off
      digitalWrite(LED1, LOW);
      digitalWrite(LED2, LOW);
      pressCnt++;
      break;
    default: // should never get here
      digitalWrite(LED1, LOW);
      digitalWrite(LED2, LOW);
      pressCnt++;
      break;
  }

}

is this the best way to do this ?
it works but i have the pressCnt that will have a large number in a few minutes.
and i am always setting digitalWrite(LED2, HIGH) a lot of times (each time the function runs)

tanks

[gcjr]

you probably only want to increment the count when the analog value changes (by +/- something). (you could have also just incremented in ledcontrol() once, outside of any of the switch cases.

in loop() you don't need to check for ranges of the value using multiple if statements. you can use if/else and sequentially check from highest or lowest value

    if (sensorValue > 800) {
        ledcontrol(0);
    }
    else if (sensorValue >= 600)  {
        ledcontrol(1);
    }
    else
        ledcontrol(2);

you could set the LEDs in the statements above and get rid of ledcontrol()

[johnwasser]

You can simplify the 'if' statements a bit by using 'else':

void loop()
{
  int sensorValue = analogRead(A5);

  if (sensorValue > 800)
  {
    ledcontrol(0);
  }
  else if (sensorValue >= 600)
  {
    ledcontrol(1);
  }
  else // < 600
  {
    ledcontrol(2);
  }
}

You only use the low bit of 'pressCnt' so it doesn't really matter how high the value gets. I would change 'pressCnt' from 'int' to 'byte' so the value rolls over to 0 after reaching 255.

[paulpaulson]

Hello
try this fast and home brewed solution

  switch (analogRead(A5)) {
    case 0 ... 599:
      digitalWrite(LED1, HIGH);
      digitalWrite(LED2, HIGH);
      pressCnt++;
      break;
    case 600 ... 799:
      digitalWrite(LED1, pressCnt % 2);
      digitalWrite(LED2, !(pressCnt % 2));
      break;
    case 800 ... 1023:
      digitalWrite(LED1, LOW);
      digitalWrite(LED2, LOW);
      pressCnt++;
      break;
  }
}

[madinfo]

Hi gcjr,
if i put the increment outside the switch, on the ( case 1: )
if value is on that range leds will be blinking between them.
And i want just one off them on.

the prupose of the ledcontrol() is because code is a litle bigger that that, and gets easy to read like that.

[gcjr]

@madinfo

if i put the increment outside the switch, on the ( case 1: )

i assume there was a misunderstanding

take a look at this code i tested on MyHW

int pressCnt = 0;

#undef MyHW
#ifdef MyHW
const int LED1 = 10;
const int LED2 = 11;
const int POT  = A0;

const int THR1 = 900;
const int THR2 = 870;

#else
const int LED1 = 8;       //  pins
const int LED2 = 9;       //  pins
const int POT  = A5;

const int THR1 = 800;
const int THR2 = 600;
#endif


// -----------------------------------------------------------------------------
void setup () {
    Serial.begin (9600);
    pinMode (LED1, OUTPUT);
    pinMode (LED2, OUTPUT);
}

// -----------------------------------------------------------------------------
int sensorValueLst = 0;

void loop() {
    int sensorValue = analogRead (POT);

    // return if no change
    if (abs(sensorValue - sensorValueLst) < 3)
        return;

    // update LEDs
    if (sensorValue > THR1) {
        digitalWrite(LED1, HIGH);
        digitalWrite(LED2, HIGH);
        Serial.print (" THR1 ");
    }
    else if (sensorValue >= THR2)  {
        digitalWrite(LED1,   pressCnt % 2);
        digitalWrite(LED2, !(pressCnt % 2));
        Serial.print (" THR2 ");
    }
    else {
        digitalWrite(LED1, LOW);
        digitalWrite(LED2, LOW);
    }

    // update state
    sensorValueLst = sensorValue;
    pressCnt++;

    Serial.print (pressCnt);
    Serial.print ("  ");
    Serial.println (sensorValue);
}

[jimLee]

What the heck? I've never seen ranged case statements before.

So I looked it up.

The case range extension of the GNU C compiler and not standard C or C++

Hmph, what will they think of next?

-jim lee

[madinfo]

@gcjr

int sensorValueLst = 0;

made a few changes to my code using yours, but i still have the same problem:

on the middle one where i should have only one led on the probability of hitting the last same prime number is strong so it would be the same led on .... cause the pressCnt++ stills goes...

[gcjr]

you need to post the code instead of assuming we know what you did

[madinfo]

sure but its pretty much the same:

byte pressCnt = 0;

const int POT  = A5;      // POT

const int LED1 = 8;       //  pins
const int LED2 = 9;       //  pins

const int THR1 = 800;
const int THR2 = 600;


void setup() {
  // put your setup code here, to run once:
    Serial.begin (9600);
    pinMode (LED1, OUTPUT);
    pinMode (LED2, OUTPUT);

}

void loop() {
  int sensorValue = analogRead(POT);

if (sensorValue > THR1) {
   ledcontrol(0);
  }
else if (sensorValue >= THR2){
   ledcontrol(1);
  }
else {
   ledcontrol(2);
  }
   
delay(250);
}

// control function
void ledcontrol(int a){

switch (a){
    case 2: // all led on
      digitalWrite(LED1, HIGH);
      digitalWrite(LED2, HIGH);
      pressCnt++;
      break;
    case 1: // just one led on 
      digitalWrite(LED1, pressCnt % 2);
      digitalWrite(LED2, !(pressCnt % 2));
      break;
    case 0: // all led off
      digitalWrite(LED1, LOW);
      digitalWrite(LED2, LOW);
      pressCnt++;
      break;
  }
  
      Serial.println(pressCnt);
}

[gcjr]

    int sensorValue = analogRead (POT);

    // return if no change
    if (abs(sensorValue - sensorValueLst) < 3)

the analog input will dither (not constant). in the code i posted (see above) the analog value is compared to a previous value and an action only taken when it changes by some about (3)

[madinfo]

yes i saw that, and i will add that feature cause is important, but is not actually my problem atm

[gcjr]

@madinfo

on the middle one where i should have only one led on the probability of hitting the last same prime number is strong so it would be the same led on … cause the pressCnt++ stills goes…

i thought you problem is above. is it?

[madinfo]

@madinfo

    case 1: // just one led on 
      digitalWrite(LED1, pressCnt % 2);
      digitalWrite(LED2, !(pressCnt % 2));
      break;

my problem is here in this pseudo random, cause the pressCnt, could be always even or always odd at the time of the case 1.

i wanted to be always different from the last time of run.

[gcjr]

yes. in my code the LEDs changed state as i turned the pot because pressCnt only gets incremented when the value changes by 3

[johnwasser]

enum Modes {ALL_ON, ALL_OFF, ALTERNATE, BOGUS} PreviousMode = BOGUS;

const int POT  = A5;      // POT

const int LED1 = 8;       //  pins
const int LED2 = 9;       //  pins

const int THR1 = 800;
const int THR2 = 600;

void setup()
{
  pinMode (LED1, OUTPUT);
  pinMode (LED2, OUTPUT);
}


void loop()
{
  Modes mode = BOGUS;

  int sensorValue = analogRead(POT);

  if (sensorValue > THR1)
  {
    mode = ALL_OFF;
  }
  else if (sensorValue >= THR2)
  {
    mode = ALTERNATE;
  }
  else
  {
    mode = ALL_ON;
  }

  // Only change the lights when the mode changes
  if (mode != PreviousMode)
  {
    PreviousMode = mode;
    ledcontrol(mode);
  }
}

// control function
void ledcontrol(enum Modes mode)
{
  static bool toggleState = false;
  switch (mode)
  {
    case ALL_ON: // all led on
      digitalWrite(LED1, HIGH);
      digitalWrite(LED2, HIGH);
      break;

    case ALTERNATE: // just one led on, alternating
      toggleState = !toggleState;
      digitalWrite(LED1, toggleState);
      digitalWrite(LED2, !toggleState);
      break;

    case ALL_OFF: // all led off
      digitalWrite(LED1, LOW);
      digitalWrite(LED2, LOW);
      break;

    default: break;
  }
}

[madinfo]

Amazing approach many tanks m8 no need to presscnt no more!