using interrupt to count frequency and drive stepper accordingly need help

Alright everyone,I’m in need of some help.
My code is listed below the input frequency is pictured below as well. The stepper doesn’t move like i expect (or at all). Did I miss and or code something wrong? I I am using a transistor H bridge to drive a bipolar stepper. I have verified functionality of the stepper and the stepper code used to eliminate hardware failure. Any suggestions?

/*

*/
#include <AccelStepper.h>
volatile int TachPosition;
int CurrentState;
int PreviousState;
int TachIn = 2;
int ignition = 4;
int CurrentIgnition;
int PreviousIgnition;
AccelStepper Tach(AccelStepper::FULL4WIRE, 6, 7, 8, 9);

void setup(){
  attachInterrupt(digitalPinToInterrupt(TachIn), Interrupt, RISING);
  Tach.setMaxSpeed(270);
  Tach.setSpeed(250);
  Tach.setAcceleration(1000);

  
}
void loop() {
//Check ignition
CurrentIgnition = digitalRead(ignition);
{
  if(CurrentIgnition == HIGH && PreviousIgnition == LOW)
  {
    PreviousIgnition = HIGH;
  Tach.runToNewPosition(-20);
  Tach.setCurrentPosition(0);
  Tach.runToNewPosition(480);
  Tach.runToNewPosition(20);
  }
 else if (CurrentIgnition == LOW && PreviousIgnition == HIGH)
  {
    Tach.runToNewPosition(0);
  PreviousIgnition = LOW;
}
}
 //Tach Movement
 Tach.runToNewPosition(TachPosition);
  
}
void Interrupt()
{ static int Pulse;
static int tempo;
static int freq;
  ++Pulse;
  tempo = millis();
   if(tempo%500==0)
  {
    freq = Pulse * 2;
    TachPosition = (2/3 * freq);
        Pulse=0;  
  }

}

PreviousIgnition == HIGH;

This doesn't set PreviousIgnition to HIGH. It compares it to HIGH and throws away that result.

PreviousIgnition == LOW;

Same issue here.

TachPosition == (2/3 * freq);

and here.

Be careful with = and ==. They mean two totally different things.

I am using a transistor H bridge to drive a bipolar stepper.

That's not a very good choice. A real stepper driver would be much better.

 tempo = millis();
   if(tempo%500==0)

Bad idea. The modulo operator is slow, and millis() is not necessarily going to increase by one every time the clock ticks.

You should keep track of the last time you did something, and if 500 milliseconds has elapsed since then, do whatever needs doing again, and update the last time.

   TachPosition == (2/3 * freq);

Why are you using the equality operator here? The assignment operator (=) is a much better choice.

The same mistake occurs in other places.

ALL variables used in the ISR and in other non-interrupt-driven functions (like loop()) must be volatile.

'freq', 'tempo' are only used in Interrupt(). They should be local, not global. As @PaulS said, the way you are using 'tempo' is a bad idea.

'Pulse' is only used in Interrupt(). It should be static local, not global.

Accesses to 'TachPosition' in loop() should be protected by a noInterrupts() / interrupts() pairs. Just copy to a local variable and use that to call Tach.runToNewPosition().

Delta_G: PreviousIgnition == HIGH;

This doesn't set PreviousIgnition to HIGH. It compares it to HIGH and throws away that result.

PreviousIgnition == LOW;

Same issue here.

TachPosition == (2/3 * freq);

and here.

Be careful with = and ==. They mean two totally different things.

That is extremely helpful. I thought they meant the same thing. Thank you!

PaulS: That's not a very good choice. A real stepper driver would be much better.

 tempo = millis();
   if(tempo%500==0)

Bad idea. The modulo operator is slow, and millis() is not necessarily going to increase by one every time the clock ticks.

You should keep track of the last time you did something, and if 500 milliseconds has elapsed since then, do whatever needs doing again, and update the last time.

   TachPosition == (2/3 * freq);

Why are you using the equality operator here? The assignment operator (=) is a much better choice.

The same mistake occurs in other places.

ALL variables used in the ISR and in other non-interrupt-driven functions (like loop()) must be volatile.

The H bridge is a temporary thing to verify functionality. These are also small steppers for a gauge cluster (5v 22ma) I will be using a proper stepper motor drive for the final product.

I understand what you mean, I think, it may just be the time after a long day, but i cant seem to figure out how to put it into code.

I am making these changes right now! Thank you everyone. im sure these are not the last questions i will have.

gfvalvo: 'freq', 'tempo' are only used in Interrupt(). They should be local, not global. As @PaulS said, the way you are using 'tempo' is a bad idea.

'Pulse' is only used in Interrupt(). It should be static local, not global.

Accesses to 'TachPosition' in loop() should be protected by a noInterrupts() / interrupts() pairs. Just copy to a local variable and use that to call Tach.runToNewPosition().

Ill have to make those local and static as you suggest, and this is where i think it gets interesting, both need to happen simultaneously, which sort of makes this difficult, im not sure if it will work or not, I know i have to use 2 arduinos for this project, maybe one should be used for strictly measurement and another for the motor drive and communicate between I2c that way one is quick while the other one runs blocking commands?

Edit: If I just looked at the stickies it shows how to work with the timer.

Why would you need two Arduinos?

Delta_G: Why would you need two Arduinos?

There is 8 steppers each being bipolar so requiring 4 pins each, then an lcd. then each of the steppers will need atleast 1 input pin (some have more). I was planning on using 2 to seperate out the 2 key functions that run constantly. the other 6 steppers dont have to be adjusted to terribly often.

i could use shift registers or other means to expand the pins, but would one arduino allow for two essentially pwm inputs(will have 2 square waves of 2 different frequencies as inputs and based on the frequency 2 of the steppers rotate, hopefully at a fluid and constant rate) the other less crucial inputs will mainly be resistance measurements.

There is 8 steppers each being bipolar so requiring 4 pins each

A proper stepper drives uses two pins - one to define the direction to step and one to trigger a step.