code ether don't recognize or don't accept a variable that changes value

hay guys

my question relates to the code for a simple learning tutorial in witch a tri coloured led is made to change colour by increasing and decreasing duty cycle of the PWM pins.

The analogWrite value, for each pin oscillates between 0 and 255. A variable called “int increment” provides the value that the analogWrite will increase or decreases its value by each cycle of the code.
This all worked fine when “int increment” was a fixed value.
However when i tried to incorporate a potentiometer to adjust the value of “int increment” while the code was running the led remaind stuck on its initial color. Ive uses serial monitor to confirm
that “int increment” is changing value as it should.

can someone pls tell me why the potentiometer modification is screwing up the code?

// Analog pin pot is attached to
int sensorValue = 0; // value read from pot
int REDPin = 11;        // RED pin of the LED to PWM pin 11
int GREENPin = 10;      // GREEN pin of the LED to PWM pin 10
int BLUEPin = 9;        // BLUE pin of the LED to PWM pin 9
int brightnessG = 0;    // green LED brightness
int brightnessR = 127;   // red LED brightness
int brightnessB = 255;  // blue LED brightness
int increment = 5;      // brightness increment
void setup()

{
  pinMode(REDPin, OUTPUT);
  pinMode(GREENPin, OUTPUT);
  pinMode(BLUEPin, OUTPUT);
  Serial.begin(9600);
}

void loop()
{
    int analogInPin = analogRead(A0);  // read the input on analog pin 0:
analogInPin = map(analogInPin, 0, 1023, 50, 1);  //convert value to 1-50
int increment = analogInPin;                    //link varable "increment" to "analogInPin
Serial.println(increment);  // print incement value

  // red LED
brightnessR = brightnessR + increment;// brightnessR for next loop
  if (brightnessR <= 0 || brightnessR >= 255)
  // reverse the direction of fade
  {
    increment = -increment;
  }
  
  // green LED
brightnessG = brightnessG + increment;// brightness for next loop
  if (brightnessG <= 0 || brightnessG >= 255)
  // reverse the direction of fade
  {
    increment = -increment;
  }

    // blue LED
brightnessB = brightnessB + increment;// brightnessR for next loop
  if (brightnessB <= 0 || brightnessB >= 255)
  // reverse the direction of fade
  {
    increment = -increment;
  }
brightnessR = constrain(brightnessR, 0, 255);
brightnessG = constrain(brightnessG, 0, 255);
brightnessB = constrain(brightnessB, 0, 255);
  analogWrite(REDPin, brightnessR);
  analogWrite(GREENPin, brightnessG);
  analogWrite(BLUEPin, brightnessB);
    {

delay(20);  // wait for 20 milliseconds to see the dimming effect
}


}

Don't you need 3 different increment variables?

incrementR
incrementG
incrementB

int increment = analogInPin;                    //link varable "increment" to "analogInPin

The comment on this line suggests a fundamental misunderstanding. You aren't linking anything. You are doing a one time check here. You are putting the current value of analogInPin into increment. If analogInPin changes later, it won't be reflected in this variable until the next time the loop comes around and this gets read again.

It should also be noted that this int increment is not the same one as the one you declared globally with an initial value of 5. When you specify the type like that, the statement is called a definition and it creates a new variable. Having two variables with the same name is a bad idea.

i know i shoud hav included a disclaimer for being a noob :slight_smile:
i know the code is pretty dirty guys! but at the moment i just want to focus on why its not working rather than best practice.
also forgive me if im useing terms like "link" in a sloppy rather than technical way.

@Delta
u mention it only samples the value once a cycal but to my understanding that isnt a problem, as its taking readings every 20ms.
but that aside dosent explain why its frozen, dose it?

@arduinodlb
i dont think so. as i said it worked fine befor, although incement would have been a positive and negative value for differant colures at the same time.
i dont fully understanh that myself, but that was part of the original sketch, that i didnt write

SavageRodent:
@arduinodlb
i dont think so. as i said it worked fine befor, although incement would have been a positive and negative value for differant colures at the same time.
i dont fully understanh that myself, but that was part of the original sketch, that i didn’t write

OK. Let me re-phrase, you are checking 3 different boundaries, but modifying the same variable in all 3 cases. I don’t know exactly what you’re trying to do, but this is not the way to do it.

For example, if R is at a high boundary, so reverses the increment to be negative, but G is at a low boundary, sees the negative increment and reverses it back to positive.

If that really was part of the original sketch, then it’s not a good example sketch.

You either want 3 independent incrementers, if you want the LED colours independent of each other, or you want to set all 3 leds at the same time, to the same value, and only need one if check, not the 3 that you have

If you don’t want 3 incrementers, just do this:

  // brightness
  brightness = brightness + increment;// brightness for next loop
  if (brightness <= 0 || brightness >= 255)
  // reverse the direction of fade
  {
    increment = -increment;
  }

Then set R,G, and B to the same brightness value. Otherwise, you must have 3 different incrementers.

You do

   increment = -increment;

then next time through loop() you do this

 int increment = analogInPin;                    //link varable "increment" to "analogInPin

thus overwriting the value of increment. What is that all about ?

brightnessR (and the other two) will keep getting larger until they are constrained then stay at the same value because increment will never go negative to reduce them.

you guys were right about separate increments for each colors. i thought it was working from just looking at the led change colure but the serial monitor showed it was messed up as u said.

i finerly got my head round what u were saying about overwriting increment, its obvious looking at it now :-[
so i've solved the main issue by multiply increment by the analogRead value, in stead of replacing it.

thanks for all the help