Assigning a "previous value" variable

I am working on some code for a tachometer and am running into some issues.

I want to structure it as the follow:

  1. Detect hall effects sensor sending a high
  2. Interrupt loop
  3. Assign tick1 to current time using millis()
  4. Next time hall effects sensor sends a high, assign prevtick1 to the value that tick1 initially had
  5. Reassign tick1 to the current time again

I have the following code:

volatile long tick1 = 0;
volatile long prevtick1 = 0;
volatile long rpm1 = 0;

void setup(){
  pinMode(2,INPUT);
  attachInterrupt(0,tacho1,RISING);
  Serial.begin(9600);
}

void loop(){
  Serial.print("tick1: ");
  Serial.println(tick1);
  Serial.print("prevtick1: ");
  Serial.println(prevtick1);
  delay(1000);
}

void tacho1(){
  prevtick1 = tick1;
  tick1 = millis();
}

The output shows that prevtick1 and tick1 always have the same value, and I believe that they shouldn't

Show the output.

I am not sure that it is the issue but I believe most programmers will tell you to ditch the delay(1000) and convert it to a timer;

#define INTERVAL 1000;

Loop
{
currentMillis = millis();       
  
    if (currentMillis - previousMillis > INTERVAL)      // only update every X (INTERVAL) milliseconds
    {
      previousMillis = currentMillis;

//DO SOME STUFF
}
}

I am not sure that it is the issue but I believe most programmers will tell you to ditch the delay(1000) and convert it to a timer;

Ignore the above. And forget about "timers" No competent programmer would tell you to use a "timer" on an Arduino (pointless complication and overhead)

@op - how many pulses per second do you expect?

Mark

micros/mills return unsgined ints !

Mark

holmes4:
Ignore the above. And forget about "timers" No competent programmer would tell you to use a "timer"

Maybe my terminology is not correct when I say "timer". My point was that every proficient programmer I have every conversed with has said that the delay function is not to be used. My suggestion above relates to the OP having a desire to only Serial Print every 1000ms whether the overhead and complication is bad or not and is the advice I have previously received.

Change

void loop(){
  Serial.print("tick1: ");
  Serial.println(tick1);
  Serial.print("prevtick1: ");
  Serial.println(prevtick1);
  delay(1000);
}

to

void loop(){
  Serial.print("tick1: ");
  Serial.print(tick1);// ln removed
  Serial.print("  prevtick1: ");// spaces added
  Serial.println(prevtick1);
  delay(1000);
}

I think it's just a case of you miss reading the output of the program!

Mark

delay() is OK in this case!

Mark

The delay() function certainly has its uses, and in this particular case, use of delay(1000) is perfectly appropriate.

Why are you using an interrupt (something that is made to be very quickly) and a delay (something that is made to pass time, in slowly actions) in the same program? You can use a millis() timer insteed delay.
After the millis() type is unsigned long, not long

holmes4:
micros/mills return unsgined ints !

Mark

Close, but no cigar.

Try something like this. Untested, but it compiles:

volatile unsigned long tick1 = 0;
volatile unsigned long prevTick1 = 0;
volatile unsigned long rpm1 = 0;

void setup() {
  pinMode(2, INPUT);
  attachInterrupt(0, tacho1, RISING);
  Serial.begin(9600);
}

void loop() {
  unsigned long localTick1, localPrevTick1;
  static unsigned long lastTick1 = 0;
  noInterrupts();
  localTick1 = tick1;
  localPrevTick1 = prevTick1;
  interrupts();
  if (localTick1 != lastTick1) {
    Serial.print("tick1: ");
    Serial.println(localTick1);
    Serial.print("prevtick1: ");
    Serial.println(localPrevTick1);
    lastTick1 = localTick1;
  }
}

void tacho1() {
  prevTick1 = tick1;
  tick1 = millis();
}

EDIT:
OP - You never answered the question in Reply #3. If it's too fast, your print buffer might overflow and the printouts look wrong.

Or try out this version (guess I’m late to the game) :

volatile unsigned long tick1 = 0;
volatile unsigned long prevtick1 = 0;
const byte InterruptPin = 2;

void setup()
{
  pinMode(InterruptPin, INPUT);
  attachInterrupt(digitalPinToInterrupt(InterruptPin), tacho1, RISING);
  Serial.begin(9600);
}

void loop()
{
  // Make local copies of multi-byte volatile variables with
  // interrupts disabled so the value can't change between
  // byte fetches.
  noInterrupts();
  unsigned long tickTime = tick1;
  unsigned long prevTickTime = prevtick1;
  interrupts();
  Serial.print("tick1: ");
  Serial.print(tickTime);
  Serial.print("   prevtick1: ");
  Serial.println(prevTickTime);

  unsigned long tickInterval = tickTime - prevTickTime;
  unsigned ticksPerMinute = 0;
  // Only show the ticks per minute if there has been at least 1
  // millisecond per tick and there has been a tick within the last second.
  if (tickInterval > 0 && (millis() - tickTime < 1000))
  {
    ticksPerMinute = 60000 / tickInterval;
    Serial.println(ticksPerMinute);
  }
  delay(1000);
}

void tacho1()
{
  prevtick1 = tick1;
  tick1 = millis();
}

If it's too fast, your print buffer might overflow and the printouts look wrong.

@gfvalvo do the math!

Mark

holmes4:
@gfvalvo do the math!

OK:

9600 Baud gives you 960 characters / second. Printing an unsigned long can require up to 10 characters and the code is printing 2 of them. Add in the other characters and let's say the message length is 30. So, at 960 characters / second, that's 32 messages / second -- Maximum.

The OP appears to be measuring the angular speed of a rotating object. Assuming a single Hall device, that's 1 pulse per revolution and the code prints 1 message per pulse. So, the maximum rate of 32 messages / second corresponds to 1920 RPM - not at all outside the realm of possibility.

Admittedly, this is only an issue with the code I posted as it prints one message per pulse. The OP's code (if it worked) prints at a rate determined by the delay function, so not an issue.

gfvalvo:
The OP appears to be measuring the angular speed of a rotating object. Assuming a single Hall device, that's 1 pulse per revolution and the code prints 1 message per pulse. So, the maximum rate of 32 messages / second corresponds to 1920 RPM - not at all outside the realm of possibility.

The sketch contains a "delay(1000);" so it sends messages less than once per second.
Since the output is based on milliseconds between the last two pulses it can only calculate a rate if that value is greater than zero. That would allow pulse rates up to 60,000 per minute. If it is one pulse per revolution that would be 60000 RPM. Of course the next slowest value would be 2 milliseconds between pulses (30,000 RPM), then 3 (20,000 RPM). To get a resolution of 10% full scale the minimum value would be 10 milliseconds between pulses (6,000 RPM). For 1% resolution you would need a minimum of 100 milliseconds between pulses (600 RPM).
I would switch over to micros() to get 1000 times the resolution.

Bit to ve helped He has to answere our questions:
What He Walt to do (because se can only immagine it)
How many Times every second the valve may chance? 1? 100? 10000?
Way do es He pur that strange and useless delay (1000)?

Silente:
Way do es He pur that strange and useless delay (1000)?

It's not useless if he doesn't need updates faster than once a second. It's not like his sketch is doing anything other than showing the time between pulses about once a second.