Turns on but not off, tried millis good + bad

Hi,
Super qik intro, 51yo Aussie guy, mechanic by trade, dabbled with electronics as a kid and thru life as needed. Into Arduino and ESP for 6 or so yrs on a casual basis, try and nag the kids about it. Lately had time to get some things done I wanted to for ages so 3d printer coming and ramped back up on eagle pcb etc (as best I can LOL).
Thought I had an account from years ago but couldn't find it so new profile today.
Also hope the code attachment goes ok and I'm in the right section :slight_smile:

Trying to do what should be simple as part of another project.

OK
I have purely for space reasons a wemos D1 mini in basic Arduino mode, no wifi, AP etc enabled.
Using a LED capacitance switch from heltec.cn and have added a smd 1K resistor to make it momentary as per data sheet.

Hooked up simply as follows,
vcc to 5v(onboard wemos), Gnd to Gnd on Wemos, OUT to pin D1 on wemos
Power via usb.
That's the hardware side.

What should it do?
Touch switch about 100 = run option 1 (ignite pin D4, ie, onboard led), count 5secs till turn off, turn off
Touch switch for 2 secs and runs option 2, turns on D4 led, counts time,after 10secs turns off.

What does it do?
Sorta works ok,
Touch and release and D4 glows up but wont turn off, for option one or two.

I had delays and it was affecting on off times so nested another mills loop in the basic code by Scuba Steve. Tried to follow syntax from existing code. Things compile and work but now wont shut off and despite trying to look at it logically I am not experienced enough in coding to see where I went wrong.

Am I stuck in a loop?

Left plenty of comments in the code to seperate the bits, hope that's not an issue folks??

/*Using a Single Button, create mutliple options based on how long the button is pressed

  I am modifying this code, original coders listed below.
  Using a Wemos D1 mini in basic Arduino mode, no funky wifi
  Also apologise in advance for my butchery

  Created DEC 2014 by Scuba Steve
  Modified JAN 2015 by Michael James
  Both members of https://programmingelectronics.com

  This code is in the public domain
*/


/////////Declare and Initialize Variables////////////////////////////

//We need to track how long the momentary pushbutton is held in order to execute different commands
//This value will be recorded in seconds
float pressLength_milliSeconds = 0;

// Define the *minimum* length of time, in milli-seconds, that the button must be pressed for a particular option to occur
int optionOne_milliSeconds = 100;
int optionTwo_milliSeconds = 2000;

//This MAY? count ON time
float ontimeLength_milliSeconds = 0;

//Define time it will run ON for each option
int runoptionOne_milliSeconds = 5000;
int runoptionTwo_milliSeconds = 10000;


//The Pin your button is attached to
int buttonPin = D1;

//Pin your LEDs are attached to
int ledPin_Option_1 = D4;
int ledPin_Option_2 = D4;

void setup() {

  // Initialize the pushbutton pin as an input 
  // Here I have a capacitance led switch 3 wire vvc, gnd, out(D1)
  // set in momentary mode,
  // https://www.aliexpress.com/item/1-pc-Green-Color-HTTM-Series-Capacitive-Touch-Switch-Button-Module-New-Arrived/32775714370.html?spm=2114.10010108.1000013.4.7d5356d4oyrMWT&traffic_analysisId=recommend_2088_2_90158_iswistore&scm=1007.13339.90158.0&pvid=8434f947-bb6c-479d-8d91-dda1049dbcba&tpp=1
  pinMode(buttonPin, INPUT);
  digitalWrite(buttonPin, LOW);

  //set the LEDs pins as outputs
  pinMode(ledPin_Option_1, OUTPUT);
  pinMode(ledPin_Option_2, OUTPUT);

  // D4 is the ledPin but I thought this may stop a dbl instance of it defaulting to LOW on startup if it does want to
  digitalWrite(D4, HIGH);

  //Start serial communication - for debugging purposes only
  Serial.begin(9600);

} // close setup


void loop() {

  //Record *roughly* the tenths of seconds the button in being held down
  while (digitalRead(buttonPin) == HIGH ) {

    delay(100);  //if you want more resolution, lower this number
    pressLength_milliSeconds = pressLength_milliSeconds + 100;

    //display how long button is has been held
    Serial.print("ms = ");
    Serial.println(pressLength_milliSeconds);

  }//close while


  //Different if-else conditions are triggered based on the length of the button press
  //Start with the longest time option first

  //Option 2 - Execute the second option if the button is held for the correct amount of time
  if (pressLength_milliSeconds >= optionTwo_milliSeconds) {

    digitalWrite(ledPin_Option_2, LOW);

    //ONtime timer check for runoptiontwo
    ontimeLength_milliSeconds = ontimeLength_milliSeconds + 100;

    // Has it been on for the time needed in this option?
    if (ontimeLength_milliSeconds >= runoptionTwo_milliSeconds) {

      // Hopefully turn off after time dictated in runoptionTwo
      digitalWrite(ledPin_Option_2, HIGH);
    }

  }

  //option 1 - Execute the first option if the button is held for the correct amount of time
  else if (pressLength_milliSeconds >= optionOne_milliSeconds) {

    digitalWrite(ledPin_Option_1, LOW);

    //ONtime timer check for runoptiontwo
    ontimeLength_milliSeconds = ontimeLength_milliSeconds + 100;

    // Has it been on for the time needed in this option?
    if (ontimeLength_milliSeconds >= runoptionOne_milliSeconds) {

      // Hopefully turn off after time dictated in runoptionOne
      digitalWrite(ledPin_Option_2, HIGH);
    }


  }//close if options


  //every time through the loop, we need to reset the pressLength_Seconds counter
  pressLength_milliSeconds = 0;

  // Since we need to reset that one I guess we have to do the same with ontimeLength_Seconds counter ??
  ontimeLength_milliSeconds = 0;

} // close void loop

It is invisible to you but the loop() function works like this:

int main()
{
    setup()

    while (true)
      loop();
}

So there is no real value in having another while loop inside your loop() function.

I would attach your touch switch to one of the interrupt pins e.g. pin 2
Something along the following lines should work
You need to disable interrupts until you have fully serviced the interrupt in loop.
Otherwise your interrupt will be continually interrupted by new calls to the interrupt....if that makes sense.

bool bButtonPressed = false;
uint8_t nButtonPin = 2, nLEDPin = 10;
uint16_t nOnTime = 0, nOffTime = 0;

void touchSwitchISR()
{
    noInterrupts();
    bButtonPressed = true;
}

void setup()
{
  pinMode(nLEDPin, OUTPUT);
  pinMode(nButtonPin, INPUT);
  attachInterrupt(digitalPinToInterrupt(nButtonPin ), touchSwitchISR, RISING);
}

void loop()
{
    if (bButtonPressed))
    {
         if (nOnTime == 0)
         {
             nOnTime = millis() + 10/*seconds*/ * 1000 /*milliseconds per second*/
             digitalWrite(nLEDPin, HIGH);
         }
         else
         {
             if (millis() >= nOnTime)
             {
                  digitalWrite(nLEDPin, LOW);
                  nOnTime = 0;
                  bButtonPressed = false;
                  interrupts();
             }
         }
    }
}

I very much appreciate the reply and the schooling.

It gets confusing when you're used to seeing code in one format and this looks totally different.

Now I have something to guide me so that's great.

I fear I might be asking for more help but I'm trying before I do that.
Cheers

Hi again, and Thanks again Boylesg.

I have loaded and toyed with that code and read about interrupts a bit but sheesh I have some to learn.

One thing I noticed with the code offered is that it seems to run only 9 or so presses @ 3 sec timing, 7 or so @ 5 sec timing, 2 presses @ 30 secs. It seems like it times out. Even tho I can see in the code it says that it resets to zero "nOnTime = 0;"

Tested it first up on a UNO then changed pins to D1 as IO on wemos, D4 is inbuilt led. Still the same timeout after x-presses * seconds

Any clue as to the timeout issue?

bool bButtonPressed = false;
uint8_t nButtonPin = D1, nLEDPin = D4;
uint16_t nOnTime = 0, nOffTime = 0;

void touchSwitchISR()
{
    noInterrupts();
    bButtonPressed = true;
}

void setup()
{
  pinMode(nLEDPin, OUTPUT);
   digitalWrite(nLEDPin, HIGH);
  pinMode(nButtonPin, INPUT);
  attachInterrupt(digitalPinToInterrupt(nButtonPin ), touchSwitchISR, RISING);
}

void loop()
{
    if (bButtonPressed)
    {
         if (nOnTime == 0)
         {
             nOnTime = millis() + 5/*seconds*/ * 1000 /*milliseconds per second*/;
             digitalWrite(nLEDPin, LOW);
         }
         else
         {
             if (millis() >= nOnTime)
             {
                  digitalWrite(nLEDPin, HIGH);
                  nOnTime = 0;
                  bButtonPressed = false;
                  interrupts();
             }
         }
    }
}

Variables changed by an ISR and accessed in the main code, such as bButtonPressed, should be declared volatile

There is no need to explicitly disable interrupts when in an ISR as this happens automatically.

Hi,
Welcome to the forum.

Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?

How have you got your momentary switch wired?

Thanks.. Tom.. :slight_smile:

HeliBob. I cant understand what/how that means atm. Will googlefu when I wake up again.

TomGeorge, cant do a drawing tonight but I have it simple as. Basically

I have purely for space reasons a wemos D1 mini in basic Arduino mode, no wifi, AP etc enabled.
Using a LED capacitance switch from heltec.cn and have added a smd 1K resistor to make it momentary as per data sheet.

Hooked up LED cap switch simply as follows,
vcc to 5v(onboard wemos), Gnd to Gnd on Wemos, OUT to pin D1 on wemos.
No pullups/downs etc, tho the WemosD1mini has them built into select pins and available if needed
Power via usb.
That's the hardware side.

If you can make sense of that good.
If not I can scratch a drawing or photo with legend in the AM.

Your interest and help is appreciated.

Hi,
So you have no pull up or down on the pin you have your button connected to?

How have you got your button connected?
Between the input pin and gnd?
OR
Between the input pin and 5V?

If either of those, what does the input pin do when the button is open?

Do you have a DMM?

Thanks.. Tom.. :slight_smile:

The code in the Original Post leaves a lot to be desired. You should not mix delay() and millis(). The demo Several Things at a Time illustrates the use of millis() to manage timing without blocking. It may help with understanding the technique.

You should not use WHILE when checking for things happening for a period of time because it blocks the Arduino until it completes.

If you are just detecting a human pressing a switch then there is no need to use interrupts - humans are very slow.

If you want to detect different button-press durations have a look at the code in this link

...R

I hope that describes the hookup clearly enough TomGeorge.
As mentioned in my original post I have added a resistor as per data sheet to change it from latching to momentary, no issues there, just soldered a 0805 smd resistor on.

Hi Robin2,
my original code is in fact a dogs breakfast.
Thankfully I have been steered in a better direction.

The link you posted looks uber useful to me. Never though would I have thought to google add a dbl click statement.

Thanks for sharing it with me.

Hi,
If you replace the cap sense switch with a normal switch switching your input between gnd then 5V, does the problem still occur.
Even just removing the cap switch and using a jumper to gnd or 5V in place of it.

Tom.. :slight_smile: