Thought this would be simple

Hi all,

The code here works ok for outputting a PWM sweep on a button release, and then stopping the sweep if the button is pushed again.

I need to also drive another pin high when the same button is pushed. When I add code for this, the new pin does go high, but the PWM sweep no longer works.

I'm sure I am doing something foolish/stupid here but can't seem to find ther answer.

Thanks,
Geno

const int  buttonPin = 2;   
const int ledPin = 9;
const int controlPina = 8;

  
int buttonState = 0;         
int lastButtonState = 0;     

void setup() 
{
  pinMode(buttonPin, INPUT);
  pinMode(ledPin, OUTPUT);
  pinMode(controlPina, OUTPUT);
}

void loop() 
{
  buttonState = digitalRead(buttonPin);
  if (buttonState != lastButtonState) 
  {
    if (buttonState == HIGH) 
    {
      analogWrite(ledPin, 0 );   
      //would like to control another pin as below when the buttonState goes high
      //digitalWrite(controlPina, HIGH);
      //but this breaks the other functions in the loop
    }                                       
    else {
      delay(2000);
      for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=.5) 
      { 
        if (digitalRead(buttonPin) == HIGH)
        {
          break;

        }
        analogWrite(ledPin, fadeValue);            
        delay(30);                            
      } 

    }
  }
  
  
  lastButtonState = buttonState;

}
 fadeValue -=.5)

Minus a half?
For an integer dataype?
No.
You may as well have written fadeValue -= 0).

you could double the value to use integers but it will not give you more distinct steps …

      for(int fadeValue = 510; fadeValue >= 0; fadeValue -=1) 
      { 
        if (digitalRead(buttonPin) == HIGH)
        {
          break;

        }
        analogWrite(ledPin, fadeValue/2);            
        delay(30);                            
      }

...or you could double the delay :stuck_out_tongue_closed_eyes:

I suspect adding a digitalWrite() doesn’t break anything… it just shows that the code doesn’t work exactly as you expect.

What I would do is:

  • get rid of all calls to delay() (and understand the blink without delay example)
  • decouple the detection of button press from the sweep code
  • add a state variable (e.g. isFading) that controls whether the sweep can run or must stop
  • make the button active low

(going to post some code soon)

const int buttonPin = 2;   
const int ledPin = 9;
const int controlPina = 8;

// button read timing
unsigned long btnPrevMillis = 0;
const unsigned long BTN_READ_DELAY_MS = 50;

int buttonState = 0;         
int lastButtonState = 0;

// fade timing
unsigned long fadePrevMillis = 0;
const unsigned long FADE_DELAY_MS = 30;

// fade state
boolean isFading = false;
int fadeValue;
int fadeStep = 1;


// fade control
void fadeStop() {
    isFading = false;
    analogWrite(ledPin, 0);
}

void fadeStart() {
    fadeValue = 255;
    analogWrite(ledPin, fadeValue);
    isFading = true;
    fadePrevMillis = millis();
}

boolean fadeIsRunning() {
    return isFading;
}


// what to do when the button is pressed
void btnPressed() {
    if (fadeIsRunning()) {
        fadeStop();
    }
    else {
        fadeStart();
    }
}

// what to do when the button is released
void btnReleased() {
}


void setup() 
{
    pinMode(buttonPin, INPUT);
    digitalWrite(buttonPin, HIGH);
    pinMode(ledPin, OUTPUT);
    pinMode(controlPina, OUTPUT);
}


void loop() 
{

    // button read section

    if (millis() - btnPrevMillis >= BTN_READ_DELAY_MS) {
        btnPrevMillis = millis();

        buttonState = digitalRead(buttonPin);

        if ((lastButtonState == LOW) && (buttonState == HIGH)) {
            btnReleased();
        }
        else if ((lastButtonState == HIGH) && (buttonState == LOW)) {
            btnPressed();
        }

        lastButtonState = buttonState;
    }


    // fading section

    if (isFading) {
        if (millis() - fadePrevMillis >= FADE_DELAY_MS) {
            fadePrevMillis = millis();

            if (fadeValue >= fadeStep) {
                fadeValue -= fadeStep;
                analogWrite(ledPin, fadeValue);
            }
            else {
                fadeStop();
            }
        }
    }
}

(this code doesn't drive the control pin)

(edit: warning, I haven't tested this code on real hardware, just compiled it)