Problem with toggling on off with momentary button

I've written some code and wanted it to execute with a momentary button push and then stop or deenergize my relays and motors with another. Any help with this would be greatly appreciated. Oh the problem is it initiates the code but will not stop it. Here is the code:

int buttonPin;
int x;
int run;
void setup()
{
pinMode(46,OUTPUT); // set Pin46 as PUL
pinMode(48,OUTPUT); // set Pin48 as DIR
pinMode(22, OUTPUT); // Configure pin 22 as an Output
pinMode(24, OUTPUT); // Configure pin 24 as an Output
pinMode(26, OUTPUT); // configure pin 26 as an Output
pinMode(28, OUTPUT); // configure pin 28 as an Output
  run=0;
   buttonPin = 7; 
   pinMode(7, INPUT_PULLUP);
}

void loop()
{
   
   if(digitalRead(7) == LOW) 
   {
    if(run == 0)
    {
      run = 255;
    }
    else
    {
      run = 0;
    }
   }
   if(run > 0)
   {
     digitalWrite(48,HIGH); // set high level direction
for(x = 0; x < 25000; x++) // repeat 25000 times a revolution when setting 25000 on driver
{
digitalWrite(46,HIGH); // Output high
delayMicroseconds(10); // set rotate speed
digitalWrite(46,LOW); // Output low
delayMicroseconds(10); // set rotate speed
  // Energize Valve
  digitalWrite(22, LOW);
  digitalWrite(24, HIGH);
  digitalWrite(26, HIGH);
  digitalWrite(28, LOW);
   
   }
   }}

Read this on debouncing a button to make button presses more reliable...
https://docs.arduino.cc/built-in-examples/digital/Debounce/

Thanks I read up on it tried to modify the code a little and either I got it wrong or not the route I should be going. Is it wise to depower the arduino in the middle of a sketch.

That could run into button bounce. 2+ ms of uncommanded ON/OFFs that look like a series of press-release events at Arduino speed. Press once, read ~12~.

When that 0 to 25000 loop with 20 micros in delays runs it will block button reads for about 1/2 second at a time while pulsing one pin and setting others the same over and over, why when nothing changes them between?
You seem to be missing at least one part but really best to take a different approach to timing and process organization, the lesson du jour at cafe forum here
.
But until you are ready, change that for-loop into what (run > 0) does, make the pulse and set the outputs. But run gets set with time code.

Show the new program.

The pulse and timing is for a dm556 driver running steppers

Change the type of x to

unsigned int x;

or even better

unsigned long x;

When you say

do you mean you want the pushbutton to be able to stop the process managed by your for loop prematurely?

a7

If you learn this timing and state machine technique, you can automate steps, I would make the driver pulse a task that can be turned on/off with a flag that the machine can set.

My bad on not showing what I changed that code is gone I'll try and write another and post it. (maybe now maybe later) I want to start the sketch with a button push then de-energize the steppers and relays so as to modify the code on the fly. write now I just upload a blank sketch but the time it takes to upload is to long. I'm using the relays to energize some valves for air cylinders when the valves deenergize they retract back I can put a switch in between the relays and vcc pin but it will just drag the spinning steppers back with the cylinders they are attached to and wreck some hard fought mechanical work ive done. I know the code to stop a loop (not really stop) is a little convalooted just trying to start stop easily on comand to change code, start again stop again there are many different power suuplies so just killing power to things kinda not possible for what i want

That is what a state machine is for. State is process state.

state 0 --- waiting for button
state 1 --- button pressed
state 2 ---

The C structure is switch-case statements, able to replace if-else ladders and reduce if-tracing.

How that works is via loop() that runs it over and over. It keeps track of state and when it runs it runs that state's case code then exits.
When loop() cycles the machine runs again what state is set.
So waiting for button sees button and changes state to button pressed and runs that case.

That allows more than one thing to run after the machine steps and that allows indpendent tasks to run as well.

PS, the digitalWrites can be sped up by writing to PINx registers.

so state 1's button press initiates a stop state meaning it runs the code associated with that state and waits for button press to run next state witch would be run if so desired yes... kinda? and would you declare all pins as low or thats what you mean with the registers

I've been at this to long today read tens of articles my brain needs a break and I need a shower.
Thanks for the help I am going to read about this later write new code and post

I have waiting for button as state 0. That state runs until button detected then the next lines do things instantly required like flag the motor driver function not in the state machine because it is critical to run until shut off so that task is Independent, only stops when the flag is off.
And last the wait for button task changes the state to button pressed so that runs next time, not waiting but pressed runs next.

Loop is a wheel. The wheel turns, time increases, process progresses from state to state.

Time must be with unsigned integers only. Unsigned math makes rollover a non-problem. The formula is
End Time - Start Time = Elapsed Time
if (micros() - startTime >= desiredInterval ) {timeout is over code here}

In the real world, errors may happen. Sometimes there will be states for handling error conditions. A good coder anticipates screwups.

Smart, don't burn out! The messages will be here later!

Some more generalised comments

You should use self-explaining names for everything

Your function setup()

void setup() {
  pinMode(46, OUTPUT); // set Pin46 as PUL
  pinMode(48, OUTPUT); // set Pin48 as DIR
  pinMode(22, OUTPUT); // Configure pin 22 as an Output
  pinMode(24, OUTPUT); // Configure pin 24 as an Output
  pinMode(26, OUTPUT); // configure pin 26 as an Output
  pinMode(28, OUTPUT); // configure pin 28 as an Output
  run = 0;
  buttonPin = 7;
  pinMode(7, INPUT_PULLUP);
}

with self-explaining constants

const byte pulsePin = 46;
const byte dirPin   = 48;
const byte buttonPin = 7;

your setup looks like this

void setup() {
  pinMode(pulsePin, OUTPUT); // no comment needed constant explains itself  
  pinMode(dirPin, OUTPUT); // no comment needed constant explains itself  
  pinMode(22, OUTPUT); // what valve? what function?
  pinMode(24, OUTPUT); // what valve? what function?
  pinMode(26, OUTPUT); // what valve? what function?
  pinMode(28, OUTPUT); // what valve? what function?
  run = 0;
  pinMode(buttonPin, INPUT_PULLUP);
}

additionally if you ever want to change the pin-number there is one single place to change the number instead of many places

      // what does it mean if your valve ist set into this combination of outputs LOW/HIGH??
      digitalWrite(22, LOW);
      digitalWrite(24, HIGH);
      digitalWrite(26, HIGH);
      digitalWrite(28, LOW);

For driving a stepper-motor there are several libraries that make life much more easier once you have understood the function-calls.

I recommend that you use the MobaTools-library.

The MobaTools-library creates the step-pulses in the background based on a timer-interrupt.
This enables that your code can do other things in parallel to the step-pulse-creation

You can install the MobaTools with the library-manager of the Arduino-ide

the code is busy stepping the motor and doesn't check for a button press. loop() is repeatedly executed. you can step the motor each iteration of loop()

look this over
a couple things only need to be done once

const byte ButtonPin = 7;

const byte PinPul   = 46;
const byte PinDir   = 48;

const byte PinOut1  = 22;
const byte PinOut2  = 24;
const byte PinOut3  = 26;
const byte PinOut4  = 28;

byte butState;

bool run = false;
int  x   = 0;
int  Cnt = 250;     // 25000;


void loop ()
{

    byte but =digitalRead (ButtonPin);
    if (butState != but)  {
        butState = but;
        delay (20);         // debounce

        if (LOW == but)  {
            if (! run)  {
                Serial.println ("run");
                run = true;
                x   = 0;
    
                // Energize Valve
                digitalWrite (PinOut1, LOW);
                digitalWrite (PinOut2, HIGH);
                digitalWrite (PinOut3, HIGH);
                digitalWrite (PinOut4, LOW);
            }
            else  {
                Serial.println ("stop");
                run = false;
    
                // de-Energize Valve   ???
                digitalWrite (PinOut1, HIGH);
                digitalWrite (PinOut2, LOW);
                digitalWrite (PinOut3, LOW);
                digitalWrite (PinOut4, HIGH);
            }
        }
    }

    // step motor up to Cnt times
    if (run && x < Cnt) {
        x++;
        Serial.println (x);

        // generate pulse
        digitalWrite      (PinPul,HIGH);
        delayMicroseconds (10);
        digitalWrite      (PinPul,LOW);
        delayMicroseconds (10);
    }
}

void setup ()
{
    Serial.begin (9600);

    pinMode (PinPul,  OUTPUT);
    pinMode (PinDir,  OUTPUT);
    digitalWrite (PinDir,HIGH);

    pinMode (PinOut1, OUTPUT);
    pinMode (PinOut2, OUTPUT);
    pinMode (PinOut3, OUTPUT);
    pinMode (PinOut4, OUTPUT);

    pinMode (ButtonPin, INPUT_PULLUP);
    butState = digitalRead (ButtonPin);
}

plug this code right in. seems great but stepper not turning now. wants to but does not. I had the stepper going quickly. I see what everyone means about the code being to busy and how things can be "declared", "labeled" once, and acted upon over and over. the code I wrote is what I know still learning

i reduced Cnt from 25000 to 250 to make testing quicker.

I changed it when I tried the code.