two pots and two leds and a confused newbie

hi!
i’m super fresh with this type of language and with electronics in general so forgive me my thickness.

i have a circuit with two pots each controlling a led and i think there’s something wrong with the code (maybe my “if” syntax is wrong?) because potA also influences ledB and vice-versa.

any help please?

here’s the code (which does seem kinda bulk also):

int ldrA = 3;
int ldrB = 4;
int potPinA = 1;
int potPinB = 2;// select the input pin for the potentiometer
int ledPinA = 12;
int ledPinB = 13; // select the pin for the LED
int valA, valB, valLsensA, valLsensB;       // variable to store the value coming from the sensor

void setup() {
  Serial.begin(9600);
  pinMode(ledPinA, OUTPUT); 
  pinMode(ledPinB, OUTPUT); // declare the ledPin as an OUTPUT
}

void loop() {
  valA = analogRead(potPinA);
  valB = analogRead(potPinB); 
  valLsensA = analogRead(ldrA);
  valLsensB = analogRead(ldrB);
if( valA >= 1000){
digitalWrite(ledPinA, LOW);
}else if (valA <= 1){
  digitalWrite(ledPinA, HIGH);
}else {
  digitalWrite(ledPinA, HIGH);  // turn the ledPin on
  delay(valA);                  // stop the program for some time
  digitalWrite(ledPinA, LOW);   // turn the ledPin off
  delay(valA); 
 }
 if( valB >= 1000){
digitalWrite(ledPinB, LOW);
}else if (valB <= 1){
  digitalWrite(ledPinB, HIGH);
}else {
  digitalWrite(ledPinB, HIGH);  // turn the ledPin on
  delay(valB);                  // stop the program for some time
  digitalWrite(ledPinB, LOW);   // turn the ledPin off
  delay(valB); 
 }
Serial.println('A', valLsensA);
Serial.println('B', valLsensB);  // stop the program for some time
}

I’m also trying to read the values off two different ldrs, so nevermind that part of the code…
Maybe this is already answered somewhere. If so, could you please point me in the right direction? Thank you!!

David

potA also influences ledB and vice-versa

Do you mean the blink rate is changed for both LEDs when you adjust either POT? If the answer is "yes" then...

Assume that PotA and PotB are both greater than 1 and less then 1000. In this case, the last "else" for each "if" is executed. These are the lines of code that are executed...

digitalWrite(ledPinA, HIGH); // turn the ledPin on delay(valA); // stop the program for some time digitalWrite(ledPinA, LOW); // turn the ledPin off delay(valA);

digitalWrite(ledPinB, HIGH); // turn the ledPin on delay(valB); // stop the program for some time digitalWrite(ledPinB, LOW); // turn the ledPin off delay(valB);

If PotA is changed, the first pair of delays are different which means it takes a different amount of time to get to the PotB part of the code. In other words, the blink rate for LedB is different because it takes a different amount of time to get to the code that manipulates LedB.

To overcome this problem, you'll need to blink the LEDs using a "state machine". I believe there's LED management code in the Playground that will help. If not (or if I've confused you more than I've helped), just ask for more assitance.

  • Brian

hey brian! thank you for the kind reply. the answer is yes, of course. i knew that the problem lied there, but was (and still am) unable to fix it. i've looked around the playground for led management but couldn't find it. also, i have no idea how to code for the state machine... could you point me in the right direction please? someone else? thank you! david

The delay() will get you every time.

I think this will do what you want. It is a state machine of a sort. The states are the BlinkA = 0 (A doesn’t blink) BlinkA = 1 (A blinks) and the same 2 states for led B.

A modified version of your code sets up the blinking parameters and then code following it in loop() checks to see if its time to blink or not and takes care of it. I have NOT tested the code but I’m pretty sure it will work for ya.

(I omitted the valLsens stuff)

int ldrA = 3;
int ldrB = 4;
int potPinA = 1;
int potPinB = 2;// select the input pin for the potentiometer
int ledPinA = 12;
int ledPinB = 13;  // select the pin for the LED
int valA, valB;    // variables to store the values coming from the sensors
int lastvalA = -1; //previous reading of potPinA - start with illegal value
int lastvalB = -1; //ditto for B
int BlinkA;        // set to 1 if ledPinA is to blink else set to 0
int BlinkB;        // ditto for B
int BlinkmsA;      // the on & off blink length in ms for ledPinA
int BlinkmsB;      // ditto for B
int nextBlinkmsA;  // if ledPinA is to blink, this is the next millis() reading when we'll toggle it
int nextBlinkmsB;  // ditto for ledPinB


void setup() 
{
  Serial.begin(9600);
  pinMode(ledPinA, OUTPUT);
  pinMode(ledPinB, OUTPUT); // declare the ledPin as an OUTPUT
}

void loop() 
{
  valA = analogRead(potPinA);
  valB = analogRead(potPinB);
  
  if(valA != lastvalA) // if the reading hasn't changed we don't need to do anything
  { 
    lastvalA = valA;     // store new reading as last reading 
    BlinkA =0;          // Assume no blinking
    if( valA >= 1000)
    {
      digitalWrite(ledPinA, LOW);
    }
    else 
      if (valA <= 1)
      {
        // write a HIGH to ledPinA if valA <=1
        digitalWrite(ledPinA, HIGH);
      }
      else 
      {
        // blinks ledPinA if valA >1 and valA < 1000
          BlinkA = 1; // we want to blink ledPinA
          BlinkmsA = valA; // set the blink on/off time to the reading
          nextBlinkmsA = millis() + BlinkmsA; // the next millis() reading where
                                                  // ledPinA is to toggle
          digitalWrite(ledPinA, HIGH);  // don't really need this since if it's off
                                        // it will toggle on after BlinkLengthA ms
      }
    }
    
  if(valB != lastvalB) // if the reading hasn't changed we don't need to do anything
  { 
    lastvalB = valB;     // store new reading as last reading 
    BlinkB = 0; // Assume no blinking
    if( valB >= 1000)
    {
      digitalWrite(ledPinB, LOW);
    }
    else 
      if (valB <= 1)
      {
        // write a HIGH to ledPinA if valA <=1
        digitalWrite(ledPinB, HIGH);
      }
      else 
      {
        // blinks ledPinB if valB >1 and valB < 1000
          BlinkB = 1; // we want to blink ledPinA
          BlinkmsB = valB; // set the blink on/off time to the reading
          nextBlinkmsB = millis() + BlinkmsB; // the next millis() reading where
                                                  // ledPinB is to toggle
          digitalWrite(ledPinB, HIGH);  // don't really need this since if it's off
                                        // it will toggle on after BlinkLengthB ms
      }
    }
    
    // Here we will take care of the blinking, if any
    if(BlinkA > 0) // or simply if(BlinkA)
      if (millis() >= nextBlinkmsA) // time to toggle the LED yet?
      {
        digitalWrite(ledPinA, !digitalRead(ledPinA)); // output the inverse of what the
                                                      // pin reads - toggle it
        nextBlinkmsA = millis() + BlinkmsA;  // add the blink length to the current time
                                            // to get the new time
      }
    if(BlinkB > 0) // or simply if(BlinkB)
      if (millis() >= nextBlinkmsB) // time to toggle the LED yet?
      {
        digitalWrite(ledPinB, !digitalRead(ledPinB)); // output the inverse of what the
                                                      // pin reads - toggle it
        nextBlinkmsB  = millis() + BlinkmsB;  // add the blink length to the current time
                                            // to get the new time
      }
}

hey! thank you so much for your response! i'm sad to say i can't make it work... i understand what the problem is, and i've spent the last two hours messing around with your code, but i can't make it work... it's probably something really simple (two pots each controlling a LED blink) but i can't make it work... any more suggestions are desperately welcome! :) thank you! david

What does it do, exactly?

Did you stick some serial.prints into the code to print out the values of your variables at different points in the code (like valA & valB) and see if you can get a handle on what's happening?

Are you sure the pots are wired correctly (one end to +5v, the other to GND wipers to ANALOG pins 1 & 2?

Try taking out the two digitalWrite( , HIGH) lines I marked as don't really need this.

Comment out the two lines with the comments // don't really need this since if it's off

change the data type for nextBlinkmsA & nextBlinkmsB from int to unsigned long.

It's working for me.

I did notice some noise on the analog input pin that caused the if(valA != lastvalA) to always be true even if the pot wasn't moving which kept resetting the blink time. I put a filter cap on my proto board that cured it. Another approach might be to change the if statement to require a change of 2 or 3 before evaluating true - maybe to if(abs((valA - lastvalA) >1) (or > 2)

hey!
thank you so much once more for your quick answer. i still did not manage to get it to work for me, the LEDs just dimmed and acted erratically (and before you ask, yes, everything’s well hooked up ;)), but i did get you suggestion of using millis() instead of delay() and got something that’s working a little bit better:

int ledPinA = 12;
int ledPinB = 13; // LED connected to digital pin 13
int potA = 1;
int potB = 2;
int valueA = LOW; 
int valueB = LOW; // previous value of the LED
long previousMillisA, previousMillisB;        // will store last time LED was updated
long intervalA, intervalB;           // interval at which to blink (milliseconds)

void setup()
{
  Serial.begin(9600);
  pinMode(ledPinA, OUTPUT);
  pinMode(ledPinB, OUTPUT);   // sets the digital pin as output
}

void loop()

{
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, is the difference
  // between the current time and last time we blinked the LED bigger than
  // the interval at which we want to blink the LED.
 intervalA = analogRead(potA);
 intervalB = analogRead(potB);

if (intervalA > 1020)
{
valueA = LOW;
digitalWrite (ledPinA, valueA);
}
else if (intervalA < 5)
{
  valueA = HIGH;
  digitalWrite (ledPinA, valueA);
  }

else if (millis() - previousMillisA > intervalA) {
    previousMillisA = millis();   // remember the last time we blinked the LED

    // if the LED is off turn it on and vice-versa.
    if (valueA == LOW)
      valueA = HIGH;
    else
      valueA = LOW;

    digitalWrite(ledPinA, valueA);
  }
  if (intervalB > 1020)
{
valueB = LOW;
digitalWrite (ledPinB, valueB);
}
else if (intervalB < 5)
{
  valueB = HIGH;
  digitalWrite (ledPinB, valueB);
  }
  
  if (millis() - previousMillisB > intervalB) {
    previousMillisB = millis();   // remember the last time we blinked the LED

    // if the LED is off turn it on and vice-versa.
    if (valueB == LOW)
      valueB = HIGH;
    else
      valueB = LOW;

    digitalWrite(ledPinB, valueB);
 
  }
Serial.println (intervalB);  
}

the only problem now is that in ledB, the if (intervalB < 1020) and if (intervalB < 5) don’t seem to work that well, probably because they come later in the code as comparec to ledA. if(intervalB>1020) just gives a very short blink followed by a long off state and if(intervalB<5) just dimmers very smoothly. i’ve tried switching the LEDs and the pots and it’s definitely in the code…
ledA works just fine and there is no influence whatsoever of one blink on the other.

any ideas?
thank you so much for the help!
david

previousMillisA & previousMillisB should be unsigned long

Don't do this sort of thing:-

if (millis() - previousMillisA > intervalA)

but this sort of thing:- if(millis() > nextTime) { nextTime=millis() + interval; ...... rest of stuff

To change the blink ratio have the number added to nextTime different depending if you are turning the LED on or off.

if(millis() > nextTime) { nextTime=millis() + interval; ...... rest of stuff

I wouldn't do it that way either - it is potentially prone to slip. Read "nextTime" from "millis ()" once at the start, then add your interval to that each time the timer triggers: "nextTime += interval;"

I use:
unsigned long lastTime;
unsigned int interval = 20; // or unsigned long if interval is big.

if((millis() - lastTime) >= interval)
{
lastTime = millis();
… whatever needs to be done
}

because I believe it will handle rollovers of millis() reliably.
for example:
lastTime = 0xffffffff; // was set just before the rollover
we next check 20 milliseconds later and millis() returns 0x00000013
0x00000013UL - 0xffffffffUL equals 0x0014 (20 decimal) so we’ll satisfy the if.

However with:
if(millis() > nextTime) { nextTime=millis() + interval;

if, say nextTime was 0xffffffff then millis() could never be greater than nextTime because one more than 0xffffffff is 0x00000000…

The point about :- if((millis() - lastTime) >= interval)

is that you are doing a calculation each time this is encounter.

f, say nextTime was 0xffffffff then millis() could never be greater than nextTime because one more than 0xffffffff is 0x00000000...

Yes then just do:- if(millis() >= nextTime) { nextTime=millis() + interval;

if(millis() >= nextTime) { nextTime=millis() + interval;

will work IF you are running the check more often than every millisecond.

One delay() in the code would make that impossible - like for instance if you are using the liquidcrystal library.

The subtraction takes, what? < 10 clock cycles for 4 bytes? At 16mhz that’s less than a microsecond.

Most of the code I write is for products that run 24/7 for years on end (alarm systems). It has to work reliably every time. I’m willing to pay that small price for something I’m SURE will work.

will work IF you are running the check more often than every millisecond.

So what makes you think that?

It will work no matter how little you check it. It has to be checked every interval to make sure it runs every time but if you check less it will still run.

One delay() in the code would make that impossible

How does that work then?

Sorry if this is a double post but I think the network dropped the first one.

concering: if (millis() >= nextTime) ....

Well, I've already shown that if nextTime = 0xffffffff then the > condition cannot possibly be met because 0xffffffff is the largest unsigned long value. That means you have to satisfy the = condition. The only way you can guarantee that is to have millis() return exactly 0xffffffff. And the only way that can be guaranteed is if you make the check at least slightly more often than once/millisecond. Right? otherwise you might skip right over 0xffffffff and have to wait, what? 9.5 hours for another chance? Right?

A delay(1) - the minimum value -- elsewhere will take 1 millisecond and that would be long enough so that you can't check the condition more often than once/ms. therefore you can't be certain you'll ever meet the = condition. Longer delays would make it even more unlikely.

I'm curious to know how you obtained the 9.5 hours?

  • Brian

I think I read it here somewhere and it isn’t correct.

The timer would have to count all the way back up to 0xffffffff before we could have a second chance for the = to be met.

That would take about 4294967296 milliseconds (2 to the 32nd power for the 4 byte counter) depending on how much we overshot by when we made the check. That’s about 49.7 days.

it’s working!!

thank you so much everyone for the help! the last code i posted was actually almost good, but there was an “else” missing that i hadn’t noticed.

so with this code, i can control the blinking of both leds independent of each other.

thank you so much

here’s the code that’s working:

int ledPinA = 12;
int ledPinB = 13; // LEDs connected to digital pins 12 and 13
int potA = 0;
int potB = 1; //pots
int ldrA = 3;
int ldrB = 4; //ldrs - use 2.2k ohm resistors (red red red gold)
int valueA = LOW; 
int valueB = LOW; // previous value of the LED
int lightvalA, lightvalB;
long previousMillisA, previousMillisB;        // will store last time LED was updated
long intervalA, intervalB;           // interval at which to blink (milliseconds)

void setup()
{
  Serial.begin(9600);
  pinMode(ledPinA, OUTPUT);
  pinMode(ledPinB, OUTPUT);   // sets the digital pins as outputs
}

void loop()

{
  
  // check to see if it's time to blink the LED; that is, is the difference
  // between the current time and last time we blinked the LED bigger than
  // the interval at which we want to blink the LED.
 intervalA = analogRead(potA);
 intervalB = analogRead(potB);
 lightvalA = analogRead(ldrA);
 lightvalB = analogRead(ldrB);

if (intervalA > 1020)
{
valueA = LOW;
digitalWrite (ledPinA, valueA);
}
else if (intervalA < 5)
{
  valueA = HIGH;
  digitalWrite (ledPinA, valueA);
  }

else if (millis() - previousMillisA > intervalA) {
    previousMillisA = millis();   // remember the last time we blinked the LED

    // if the LED is off turn it on and vice-versa.
    if (valueA == LOW)
      valueA = HIGH;
    else
      valueA = LOW;

    digitalWrite(ledPinA, valueA);
  }
  if (intervalB > 1020)
{
valueB = LOW;
digitalWrite (ledPinB, valueB);
}
else if (intervalB < 5)
{
  valueB = HIGH;
  digitalWrite (ledPinB, valueB);
  }
  
  else if (millis() - previousMillisB > intervalB) {
    previousMillisB = millis();   // remember the last time we blinked the LED

    // if the LED is off turn it on and vice-versa.
    if (valueB == LOW)
      valueB = HIGH;
    else
      valueB = LOW;

    digitalWrite(ledPinB, valueB);
 
  }
Serial.println (lightvalA);  
}