Is it possible to create 2 loops for RPM calculations?

Hi Guys,

I need help from you guys... appreciate if you can help me on this problem :slight_smile:

I'm currently working on a RPM project. The project is quite simple, read the RPM > press and hold on the button switch > relay will keep clicking to cut the power every 25ms.

Version 1 problem:

I put the RPM calculation in the "loop" and I managed to get the RPM numbers to display on 16x2 LCD and it working fine. But when I press and hold on the button switch, the relay only ON and OFF every 2 secs instead of 25ms.

Try posting your code inline using the </> code tags and post COMPLETE programs. More people will read them that way.

I can't make any sense of your second version. You have a function rpmCalculation() which you never use and then you use a variable which never seems to be updated also called rpmCalculation. It's like you're going out of your way to confuse things.

Steve

int buttonstate = digitalRead(7);
...
  if (rpmCalculation >= Speed && digitalRead(buttonstate) == LOW)

You read 0 or 1 from pin 7, then use that to decide whether to read from either pin 0 or 1, which are the serial pins on the UNO.

There are many other serious problems. It seems like you have passed over the basic learning steps.

I've not looked at your code both because of the way you have posted it and because of the comments of those who have. I would think these 2 tutorials would be useful to what you are trying to do, especially the second one:
Using millis for timing
Demonstration for several things at the same time

slipstick:
Try posting your code inline using the </> code tags and post COMPLETE programs. More people will read them that way.

I can't make any sense of your second version. You have a function rpmCalculation() which you never use and then you use a variable which never seems to be updated also called rpmCalculation. It's like you're going out of your way to confuse things.

Steve

oops sorry! I have post out the code :slight_smile:

so do you mean I should put this function "rpmCalculation()" in the loop?

Void loop() {

rpmCalculation()

}

aarg:

int buttonstate = digitalRead(7);

...
 if (rpmCalculation >= Speed && digitalRead(buttonstate) == LOW)




You read 0 or 1 from pin 7, then use that to decide whether to read from either pin 0 or 1, which are the serial pins on the UNO.

There are many other serious problems. It seems like you have passed over the basic learning steps.

Hi! thanks for the reply!

I'm using NANO for this project, and I have posted out the code, can help take a look again? :slight_smile:

PerryBebbington:
I've not looked at your code both because of the way you have posted it and because of the comments of those who have. I would think these 2 tutorials would be useful to what you are trying to do, especially the second one:
Using millis for timing
Demonstration for several things at the same time

Hi Perry! Thanks for sharing!

The guide is very helpful! I will spend time to read them!

Btw, I have posted out the code :slight_smile:

Thanks again!

Right,

Thanks for posting your code properly. When it comes to other people's code I am not the best, there are people here who amaze me at their ability to cut through what others are trying to do and be helpful, I am not as good as them. Hopefully one of them will offer help.

With that said this is what I think:
I agree with slipstick when he said:

It's like you're going out of your way to confuse things.

It seems to me that you are trying to do something with bits and pieces you have picked up without understanding what they are for. You need to learn the basics, which you can do by working through the tutorials on this web site, in the IDE and elsewhere.

The following stood out for me as, well, daft TBH:
You declare:

int buttonPin = 7;

But then use:

pinMode(7, INPUT_PULLUP);

What is the point of giving the pin a name if you don't use it?

  int buttonstate = digitalRead(7);
  switch (buttonstate) {
    case LOW:
      cutpower();
      break;
  }

What is the point of switch with only one case?

void cutpower() {
  if (rpm >= Speed && digitalRead(buttonstate) == LOW)
  {
    digitalWrite(relay, HIGH);
    delay(25);
    digitalWrite(relay, LOW);
    delay(25);
  }
}

The only way you can get to the function cutpower() is if buttonstate is low, so why read it again?

While delay is OK for initial learning and playing about it will not serve you well, it will make your code slow and unresponsive and you need to learn to not use it but use millis() instead. Also, pulseIn() is blocking and as bad as delay.

Learn the basics, look at how others write code and try again.

Have fun.

What kind of turbo relay do you have, being able to switch 40 times a second?

wvmarle:
What kind of turbo relay do you have, being able to switch 40 times a second?

I would think a reed relay would manage that - just. No, I've not tried it!

PerryBebbington:
Right,

Thanks for posting your code properly. When it comes to other people's code I am not the best, there are people here who amaze me at their ability to cut through what others are trying to do and be helpful, I am not as good as them. Hopefully one of them will offer help.

With that said this is what I think:
I agree with slipstick when he said:It seems to me that you are trying to do something with bits and pieces you have picked up without understanding what they are for. You need to learn the basics, which you can do by working through the tutorials on this web site, in the IDE and elsewhere.

The following stood out for me as, well, daft TBH:
You declare:

int buttonPin = 7;

But then use:

pinMode(7, INPUT_PULLUP);

What is the point of giving the pin a name if you don't use it?

  int buttonstate = digitalRead(7);

switch (buttonstate) {
    case LOW:
      cutpower();
      break;
  }



What is the point of switch with only one case?



void cutpower() {
  if (rpm >= Speed && digitalRead(buttonstate) == LOW)
  {
    digitalWrite(relay, HIGH);
    delay(25);
    digitalWrite(relay, LOW);
    delay(25);
  }
}



The only way you can get to the function cutpower() is if buttonstate is low, so why read it again?

While delay is OK for initial learning and playing about it will not serve you well, it will make your code slow and unresponsive and you need to learn to not use it but use millis() instead. Also, pulseIn() is blocking and as bad as delay.

Learn the basics, look at how others write code and try again.

Have fun.

ah.... thanks for pointing this out! I think i should clean up the code... and ya, I realised that the pulseIn is the culprit! But I need to use pulseIn to read the rpm signal.... hmmmmm.....

Diy_World:
But I need to use pulseIn to read the rpm signal.... hmmmmm.....

Why?

State change detection is what you need.

When the signal goes high, record the time. Continue checking, do other things in the meantime. When the signal goes low, record the time again, and you know how long it was. You won't be off by more than a few µs (assuming you get rid of the other delay() calls and so as well, so your loop() loops fast). For a signal of up to a few kHz that'll be quite fine.

For high speed signals (a few kHz or more) interrupts or input capture techniques come in play.

I think i should clean up the code...

Yes, please do, otherwise you attract answers about the mess in the code and not about what you really want to know. Not only that but in the process of cleaning it up you might solve your problem.

wvmarle:
Why?

State change detection is what you need.

When the signal goes high, record the time. Continue checking, do other things in the meantime. When the signal goes low, record the time again, and you know how long it was. You won't be off by more than a few µs (assuming you get rid of the other delay() calls and so as well, so your loop() loops fast). For a signal of up to a few kHz that'll be quite fine.

For high speed signals (a few kHz or more) interrupts or input capture techniques come in play.

Hi Wvmarle!

Just found this code. seems like this is what you referring to? use this code to replace with my pulseIn? :slight_smile:

/*
 * Non-blocking pulseIn(): returns the pulse length in microseconds
 * when the falling edge is detected. Otherwise returns 0.
 */
unsigned long read_pulse(int pin)
{
    static unsigned long rising_time;  // time of the rising edge
    static int last_state;             // previous pin state
    int state = digitalRead(pin);      // current pin state
    unsigned long pulse_length = 0;    // default return value

    // On rising edge: record current time.
    if (last_state == LOW && state == HIGH) {
        rising_time = micros();
    }

    // On falling edge: report pulse length.
    if (last_state == HIGH && state == LOW) {
        unsigned long falling_time = micros();
        pulse_length = falling_time - rising_time;
    }

    last_state = state;
    return pulse_length;
}

PerryBebbington:
Yes, please do, otherwise you attract answers about the mess in the code and not about what you really want to know. Not only that but in the process of cleaning it up you might solve your problem.

haha, I have been spending a few days to try to solve this problem, and until now I finally know that the pulseIn will delay the code........

That looks like it will do the job. Add that to your loop() and make sure it's called frequently, as in thousands of times a second - so no other delay() or so allowed in your other functions!

wvmarle:
That looks like it will do the job. Add that to your loop() and make sure it's called frequently, as in thousands of times a second - so no other delay() or so allowed in your other functions!

Okay! so basically, I only need to change the :

int state = digitalRead(pin); // current pin state

to

int state = digitalRead(RpmPin); // current pin state

and add the function to loop ?

Looks like. Try it out!

I think your code looks OK. Be aware that it will mostly return 0, so you need to check the return value for being non-zero before using it.

Best thing is to try it.

Even if there's something wrong with it, it's a damn sight better than what you had before!

Reading through this post and have to ask, what are you controlling and what is the goal?

It sounds like you are controlling the RPM of something (a motor?) by switching a relay on and off. There might be better ways of accomplishing this if you could explain your goal.

John