Is there a way of stopping a counter or delay in a statement when switch is still on

Hi,
I am new to coding and do not know a lot, So please bear with me !
I am using Ardunio IDE with MightyCore and a pickit4 to an Atmega8535
I have a function that use a push button switch (SW_REV) to turn them on and I would like to stop the function from running beyond a certain time (in this case, 2 to 3 seconds) even if the button is still pushed. This is to stop the hydraulic piston that is operated with a motor (PIN_M)and a solenoid valve(PIN_UP) for hyper-extending and straining the motor.
I tried delay, that won't shut off till the switch is released, also tried a counter, but that also does not stop (I have part of this blanked out in the attached partial code) and I tried to use sensorVal to read the counter to see if that would work, But NO. I also looked into millis() , but I can't figure out a way to stop that from cycling either.
the function does shut down and go into the next part of the function , but only when I release the switch.
I did have something similar to the clock function working on a PIC, but on this avr, it isn't working the same way
Any thoughts of what might work

   int sensorVal7 = digitalRead(LED_HF2);
   while (sensorVal7 == HIGH) 
     {       
              int sensorVal9 = digitalRead(SW_REV);
              if (sensorVal9 == HIGH)
            { for (counter = 0; counter<= 10; counter++)
            { 
              {digitalWrite(PIN_DN, LOW);  digitalWrite(LED_DN, LOW);}
              {digitalWrite(PIN_UP, HIGH); digitalWrite(PIN_M,  HIGH);delay(200);} 
            }
            }
             int sensorVal10 = digitalRead(counter);
             if (sensorVal10 == 0)
            {digitalWrite(PIN_UP, LOW);  digitalWrite(PIN_M, LOW);} 
          // else
          //{
          // (counter= 0); digitalWrite(PIN_UP, LOW);digitalWrite(PIN_M, LOW);
          //}
              
          int sensorVal8 = digitalRead(SW_REV);
          if (sensorVal8 != HIGH)
        
                {digitalWrite(PIN_DN, HIGH); digitalWrite(LED_DN, HIGH);
                }
                else
                {
                  digitalWrite(PIN_DN, LOW); digitalWrite(LED_DN, LOW);
                }

       break;
      }

where do you update sensorVal7 in the while loop?

Don't post snippets (Snippets R Us!)

Val7 creates that while loop. It senses that LED_HF2, which was turned on a step before this loop, is on. THANKS for replying

Then it must ALSO stop the loop!

1 Like

I had not seen the break at the end of the code. why do you use a while() when you want to do a if() ?

why do you have this for loop?

    for (counter = 0; counter <= 10; counter++) {
      {
        digitalWrite(PIN_DN, LOW);
        digitalWrite(LED_DN, LOW);
      }
      {
        digitalWrite(PIN_UP, HIGH);
        digitalWrite(PIN_M,  HIGH);
        delay(200);
      }
    }

this is equivalent to

        digitalWrite(PIN_DN, LOW);
        digitalWrite(LED_DN, LOW);
        digitalWrite(PIN_UP, HIGH);
        digitalWrite(PIN_M,  HIGH);
        delay(2000);

if you don't want to wait 2000ms, just write a timed loop using millis() and your exit-early condition

something like

digitalWrite(PIN_DN, LOW);
digitalWrite(LED_DN, LOW);
digitalWrite(PIN_UP, HIGH);
digitalWrite(PIN_M,  HIGH);
for (uint32_t start=millis(); millis() - start <= 2000; ) {
    if (digitalRead(SW_REV) == LOW) break;
}

The first while I have is near the top of the program and is there so that the power on switch will let the rest of the stuff in that while loop to work, and when II power off, none of the functions will work.
This while loop is a loop inside that other powered up loop, so that the other functions above this loop wont interfere with this loops function.
his loop also relies on two different ifs (1 is the one led being on and the REV input being on or off). The second part of this loop which is almost the same as this actually runs in reverse is in opposite positions and when it senses a different led is on
Like I said, I am new to this And just started writing code 3 weeks ago.
I built hundreds of boards from the designing to assembly but the chips that I was using were already written. so I could not tweek them at all.
I will look over what you sent in a bit.
again< Thanks for replying. Fran

this one also stays on while the switch is active
I even tried adding this after the delay,
digitalWrite(PIN_UP, LOW); digitalWrite(PIN_M, LOW);
and it still did not shut off

same for

for (uint32_t start=millis(); millis() - start <= 2000; ) {
    if (digitalRead(SW_REV) == LOW) break;

being added in, It won't shut down till the switch is released and goes back to HIGH. if I add

digitalWrite(PIN_UP, LOW); digitalWrite(PIN_M, LOW);

it blinks and keeps blinking , till the switch is released.

@chief_controllers

be a real chief and write down a description of pure functionality.

With pure functionality I mean describing in normal words what the whole thing shall do
I make a start to show what I mean as a step by step description
A button is pushed to start a motor and to open a solenoid-valve
after 3 seconds

  • the motor shall be stopped
  • the solenoid shall be de-energised to close

even in case that the button is still pushed.

The description above has zero words about IO-pins or any other programming terms

This is what I mean with pure functionality.
It might be that there are some misconceptions about programming or programming terms

Writing the pure functionality with everyday words makes sure that there are no mis-conceptions.
And additionally the step by step description with exact naming of all detail makes sure that everything is clear.

A sentence like

it is unclear: what is "them" ?
Which function is "the function" ?
Do you really want to hyper-extend it? or should hyper-extending be avoided through stopping before hyper-extending happens?

To give really good advice anybody needs to have a very clear picture about the pure functionality that you want.

best regards Stefan

I did say what the functions were and what did what.

I did make a typo in the original post before the word hyper-extend.
I typed "for" instead of "from ", but ,STILL !! who would want to hyper extend anything .

(SW_REV (the switch) is a push button and turns on PIN-M(motor) and PIN_UP (valve coil)
Normally this will shut off if the switch is released,
BUT, I WANT IT TO SHUT OFF, at 3seconds, even if the switch is pushed on for more than 3 seconds .
I know it is possible, as, I have had controllers that did that.

surely it is possible. There a zillions of functionalities possible.
The question was which functionality to do want to have?

one way to get this kind of functionality is a state-machine
you can imagine this as different operation-modes of a part of your code

mode 1: wait for button to be pressed
if button is pressed change to mode 2

mode 2: turn on pin_M and turn on Pin_up , additionally store a timestamp into a variable
switch to mode 3

mode 3:
check if button is released
if button IS released => change to mode 4

check if 3 seconds have passed by.
if 3 seconds HAVE passed by => change to mode 4

mode 4
turn off pin_M and turn off PIN_UP

change to mode __ yeah which mode then ? change to mode 1 (wait for button-press?) or is there something else

this different operation-modes are achieved through a switch-case-break-statement
The break is very important.

another important detail is that you quickly jump_in / jump_out of the "state-machine-function
again and again.

This enables to do other things "in parallel"
The checking if 3 seconds have passed by is done by non-blocking timing which works very different than a delay(). It works like baking a pizza where you do a very often repeated look into your watch "is baking time over?"

If you find this approach interesting you have to learn two new things:

  1. how non-blocking timing works
  2. how state-machines work
    then to build up a state-machine that does the things like described above.

If you would like to stay very close to your PIC-microcontroller-solution you would have to take the effort of describing very detailed how you did all this on a pic-controller. Without this information nobody will be able to tell you how you could achieve a similar thing on an AVR-controller. simply claiming "it must be possible" does not tell any detail of how it is done on a PIC-controller.

best regards Stefan

1 Like

Not sure. WHat you are saying.

This will poll the pin and exit the for loop if the condition is met or will exit the loop after 2s regardless of the state of the pin in the condition.

I replaced my timer section

              int sensorVal9 = digitalRead(SW_REV);
              if (sensorVal9 == HIGH)
            { for (counter = 0; counter<= 10; counter++)
            { 
              {digitalWrite(PIN_DN, LOW);  digitalWrite(LED_DN, LOW);}
              {digitalWrite(PIN_UP, HIGH); digitalWrite(PIN_M,  HIGH);delay(200);} 
            }
            }
             int sensorVal10 = digitalRead(counter);
             if (sensorVal10 == 0)
            {digitalWrite(PIN_UP, LOW);  digitalWrite(PIN_M, LOW);} 

with what you said

int sensorVal9 = digitalRead(SW_REV);
 if (sensorVal9 == HIGH)
{
digitalWrite(PIN_DN, LOW);
digitalWrite(LED_DN, LOW);
digitalWrite(PIN_UP, HIGH);
digitalWrite(PIN_M,  HIGH);
}
for (uint32_t start=millis(); millis() - start <= 2000; )
 {
    if (digitalRead(SW_REV) == LOW) break;
}

and PIN_UP and PIN_M would not shut off while the switch was still depressed and at LOW.
I had to release the switch for the function to stop. and the second part of the function to turn on.

You need to add what needs to happen in case of timeout if you want a forced LOW

What I provided is just a timed polling

Why do you put so many {}?

I thought that the break;
is what happens at the timeout , and then, when I do release the switch, then it would move on to the next function.
SO< can I put PIN_M = LOW and PIN_UP = LOW as something to do ?
I use that many brackets because some clown at stackoverflow complained that I did not have them around all my if statements which worked fine without them. He also said I was using too many commas and it was making my code hard to read, but if I removed commas, it would not compile.
Needless to say, I do not go there anymore since, even after I told them that I was learning, all I got was criticism on the way the code looked and others telling me to go learn C !
I did notice that you did not have brackets on what you said to try, but, without them, it did not want to work at all or did not compile . I do not remember which it was.
I really appreciate the help !
So, Thanks

Brackets are used (among other things) to build compound statements. That means grouping many statements as one. If you have only one statement in a if, for, while then no need for {} although it’s recommended for clarity and avoiding future bugs if you add one more statement

Regarding the code

for (uint32_t start=millis(); millis() - start <= 2000; ) {
    if (digitalRead(SW_REV) == LOW) break;
}

It says
Initialize a start variable with the value of millis
Repeat what’s within the brackets as long as the condition is met, so for two seconds
Then the for loop is done.

Now If what you repeat says break at some point, you exit the for loop early

So the code above says, wait for 2 seconds or until the pin gets LOW, whatever arrives first.

Wasn’t that your ask?

What you do before this conditional delay and after is up to you.

I guess I got it wrong )-: because this one below turns on about 2 seconds after uploading it , without a switch and wont turn off with a switch. It also turns on about 2 seconds after uploading it with sensorVal ==LOW

   int sensorVal = digitalRead(SW_REV);
   if (sensorVal == HIGH)
  for (uint32_t start=millis(); millis() - start <= 2000; )
 {   
    digitalWrite(PIN_UP, HIGH);
    digitalWrite(PIN_M, HIGH);
    
    break;
 }

SO I TRIED the one below, this one doesn't turn on right away . it does turn on when the switch is pushed, BUT it does not shut off with the switch on or released.

   int sensorVal = digitalRead(SW_REV);
    if (sensorVal == LOW)
 {   
    digitalWrite(PIN_UP, HIGH);
    digitalWrite(PIN_M, HIGH);
  for (uint32_t start=millis(); millis() - start <= 2000; )
    break;
 }

You need an ON delay, see if you can adapt this to your program: Written for Nano.

uint32_t timer;
int delayTime = 2000;
const byte triggerPin = 4; // input
const byte led = LED_BUILTIN;
bool timing;
bool trigger;
bool lastTrigger = true;

void setup()
{
  pinMode(led,OUTPUT);
  pinMode(triggerPin,INPUT_PULLUP);
}

void loop()
{
   trigger = digitalRead(triggerPin);
   if(trigger != lastTrigger)
   {
     lastTrigger = trigger;
   if(!timing && !lastTrigger)
   {
     timer = millis();
     timing = true;
   }  
   }
    if(millis() - timer > delayTime)
      timing = false;
    digitalWrite(led,timing); // output
     
}

Hi Stefan
Thanks for the info.

If I understand switch cases correctly.
Each one has a statement that is run if the case is selected.
If there is no breaks, then case 1 gets followed up by case 2 etc, until a break occurs (assuming case 1 is picked first)
if they all have breaks, then you need to select one or another case #.
Unless ,I am wrong ? This seems appropriate for using multiple switches or digital keypad, more-so than a single physical switch.

My Big question is ?
How do I select a case with an actual push-button SWITCH ?

A state-machine has one extra variable that is set to different values.
This variable (i.e. the value of this variable) represents the mode of operation

In the description above I used the word "mode"
most explanations of the state-machine use the word "state" which I think right now is a not a good choice in the field of using code to real-world applications.

You really should read the examples.

As a rough sketching the code looks similar to this

// constants for the different modes of operation (the different STATES of the state-machine
const byte sm_WaitForButtonPress     = 1;
const byte sm_SwitchOnMotorAndValve  = 2;
const byte sm_CheckButtonAndTime     = 3;
const byte sm_SwitchOFFMotorAndValve = 4;

byte myMode = sm_WaitForButtonPress; // initialise to first mode (i.e. State)

unsigned long MotorOnStart;

void myStateMachine() {

switch (myMode) {

  case sm_WaitForButtonPress:
     if (button is pressed) {
       myMode = sm_SwitchOnMotorAndValve;
     }
     break; // immidiately jump down to end of switch

  case sm_SwitchOnMotorAndValve:
    digitalWrite(PIN_UP, HIGH);
    digitalWrite(PIN_M,  HIGH);
    MotorOnStart = millis(); // store timestamp for timeout
    myMode = sm_CheckButtonAndTime;
     break; // immidiately jump down to end of switch

  case sm_CheckButtonAndTime:
    if (button is released) {
      myMode = sm_SwitchOFFMotorAndValve;
    }

    if ( millis() - MotorOnStart >= 2000) {
      // 2000 milliseconds have passed by
      myMode = sm_SwitchOFFMotorAndValve;
    }
     break; // immidiately jump down to end of switch

  case sm_SwitchOFFMotorAndValve:
    digitalWrite(PIN_UP, LOW);
    digitalWrite(PIN_M,  LOW);
    myMode = sm_WaitForButtonPress;
    break; // immidiately jump down to end of switch    
  }
}

I haven't read all the other postings because they talk about details.
Good to learn in general but talking about details is not focused on a complete solution

My postings suggest to use a state-machine as the solution

best regards Stefan