Pages: [1] 2   Go Down
Author Topic: Run if (or similar) statement once  (Read 1296 times)
0 Members and 1 Guest are viewing this topic.
UK
Offline Offline
God Member
*****
Karma: 1
Posts: 530
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi guys,

I've got a bit of a problem, I'm working on a shaft speed and position device, and I seem to be running into problems... Please see the code below - hardware wise I've got a pull down button running into the interrupt pin (which is pulled up to 5v via 10k), hardware debounced using an RC circuit into a hex Schmitt inverter. Pressing this button simulates a 0 - 5v input from a hall sensor and toothed wheel, and works without bouncing, no problems here.

So, pressing my button, timer_pulsecount increments as expected. When timer_pulsecount reaches 3, another button press resets this value to 1, good - just what I wanted.

When timer_pulsecount is 1, I'm saving current micros to a value called timer_p_pulsemicros, same way the blinkwithoutdelay example is, and how I'm printing to serial without delay. Then, when timer_pulsecount is 3, the time for those three pulses to take place is calculated by subtracting previous from current and thus degrees per micro and rpm are calculated.

Questions - this method is causing me problems as the if statesments have time to execute more than once, how can I execute these if statements only once? So that when timer_pulsecount = 3, the if statement runs once, and not until it cycles round until 3 again?

I've seen something about a break; statement, but it seems these don't work in if statements?

All the best


Code:
// program will run for 70 minutes until currentmicro overflows
// check what will happen at overflow time
// changed millis to micro, thought of floating point, but too slow

volatile int timer_pulsecount = 0;
volatile int angle_pulsecount = 0;
unsigned long timer_c_pulsemicros = 0;
unsigned long timer_p_pulsemicros = 0;
unsigned long threepulsetime = 0;
int rpm = 0;
int tooth_spacing = 10;
unsigned long dpm = 0;
unsigned long debug_p_millis = 0;
unsigned long debug_c_millis = 0;

void setup()
{
  Serial.begin(9600);
  attachInterrupt(0, crank, RISING); // din D2
}

void crank()
{
  timer_pulsecount++; //
}

void loop()
{

  ////////////////// SPEED AND POSITION TRACKING ///////////////////

 timer_c_pulsemicros = micros();
  
  if(timer_pulsecount == 1)
  {
    timer_p_pulsemicros = timer_c_pulsemicros;
  }

  if(timer_pulsecount == 3) // maybe change to an elseif statement?
  {
    threepulsetime = timer_c_pulsemicros - timer_p_pulsemicros;
    dpm = (timer_pulsecount * tooth_spacing) / threepulsetime;
    rpm = (dpm * 60000000) / 360; // converts degrees per micro to rpm
  }
  
  if(timer_pulsecount > 3)
  {
    timer_pulsecount = 1;
  }

  ////////////////// DEBUGGING SERIAL STREAM ///////////////////

  debug_c_millis = millis(); // starts counting currentMillis
  if(debug_c_millis - debug_p_millis >= 500) // if current - 0 is > 1000, run if script
  {
    debug_p_millis = debug_c_millis; // set previous to current
    Serial.println(timer_pulsecount);
  }
}
« Last Edit: October 26, 2012, 06:31:11 am by jtw11 » Logged

Norfolk UK
Offline Offline
Edison Member
*
Karma: 66
Posts: 2475
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

You could use switch/case statements instead of if/then and have a variable (static or global) that stores the last timer_pulsecount.
Maybe something like this (untested) code

Code:
////////////////// SPEED AND POSITION TRACKING ///////////////////

timer_c_pulsemicros = micros();

if (lastCommand != timer_pulsecount){
    lastCommand = timer_pulsecount;
    switch (lastCommand){
        case 1:
        timer_p_pulsemicros = timer_c_pulsemicros;
        break;
        case 3:
        threepulsetime = timer_c_pulsemicros - timer_p_pulsemicros;
        dpm = (timer_pulsecount * tooth_spacing) / threepulsetime;
        rpm = (dpm * 60000000UL) / 360; // converts degrees per micro to rpm
        break;
    }
}

if(timer_pulsecount > 3)
{
    timer_pulsecount = 1;
}
I wonder if all the above code should maybe be part of the interrupt routine if missing a state would cause problems. You could then stick to using the if/then as there is no chance of running the loop with same values more than once
Logged

There is no such thing as a stupid question but there are a lot of inquisitive idiots.

UK
Offline Offline
God Member
*****
Karma: 1
Posts: 530
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thankyou - i'll take a look at this, see what I can do. What is the added UL all about now? I'm assuming it stands for unsigned long, but this was already declared unsigned long?

Code:
(dpm * 60000000UL)

I think it is safe to perform the speed calculations within loop, as the next piece of code (I didn't include) which does the position tracking, increments a position counter through another interrupt, with the position between reference pulses calculated on a time basis - but the speed would need to be more than slightly out for this to have a large effect.
Logged

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12562
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Questions - this method is causing me problems as the if statesments have time to execute more than once, how can I execute these if statements only once? So that when timer_pulsecount = 3, the if statement runs once, and not until it cycles round until 3 again?


How about setting timer_pulsecount back to zero in that block of code, after you've finished processing it?
Logged

I only provide help via the forum - please do not contact me for private consultancy.

UK
Offline Offline
God Member
*****
Karma: 1
Posts: 530
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Questions - this method is causing me problems as the if statesments have time to execute more than once, how can I execute these if statements only once? So that when timer_pulsecount = 3, the if statement runs once, and not until it cycles round until 3 again?


How about setting timer_pulsecount back to zero in that block of code, after you've finished processing it?

That's fine, I am indeed resetting to 0 (well, resetting to 1 actually, you will see why 1 not 0 in later code) that's not the problem. The problem is, after the third pulse and speed calculations begin, because loop runs that statement again, and the timer is continuing to count, at low speeds, the rpm output would creep down between pulses.

So, I'm at a point where all my variables are behaving as expected - but the dpm and rpm calculations simply arn't doing anything?

Another question;

Am I going to run into trouble if I try to do math operations with different datatypes, i.e. long * int? I fear this is a problem, as fast button presses do not make the RPM value move from 0 at all.

Here is said code, with things jiggled around somewhat. See notes at the bottom of code. Everything seems good, yet RPM does not change from 0, must be a problem multiplying datatypes?

Code:
// program will run for 70 minutes until currentmicro overflows
// check what will happen at overflow time
// changed millis to micro, thought of floating point, but too slow
// attempty change back to milli

volatile int timer_pulsecount = 0;
volatile int angle_pulsecount = 0;
unsigned long timer_c_pulsemillis = 0;
unsigned long timer_p_pulsemillis = 0;
unsigned long threepulsetime = 0;
unsigned long threepulsetimecopy = 0;
int rpm = 0;
int tooth_spacing = 10;
int lastCommand = 0;
unsigned long dpm = 0;
unsigned long debug_p_millis = 0;
unsigned long debug_c_millis = 0;

void setup()
{
  Serial.begin(9600);
  attachInterrupt(0, crank, RISING); // din D2
}

void crank()
{
  timer_pulsecount++; //
}

void loop()
{

  ////////////////// SPEED AND POSITION TRACKING ///////////////////

  timer_c_pulsemillis = millis();
  dpm = 3 * tooth_spacing / threepulsetimecopy; // causing problem, due to multiplying diff types
  rpm = (dpm * 60000) / 360; // converts degrees per micro to rpm

  if (lastCommand != timer_pulsecount){
    lastCommand = timer_pulsecount;
    switch (lastCommand){
    case 1:
      timer_p_pulsemillis = timer_c_pulsemillis;
      break;
    case 3:
      threepulsetime = timer_c_pulsemillis - timer_p_pulsemillis; // needs to be reset to 0
      threepulsetimecopy = threepulsetime;
      break;
    }
  }

  if(timer_pulsecount > 3)
  {
    timer_pulsecount = 1;
    threepulsetime = 0;
  }

  ////////////////// DEBUGGING SERIAL STREAM ///////////////////

  debug_c_millis = millis(); // starts counting currentMillis
  if(debug_c_millis - debug_p_millis >= 500) // if current - 0 is > 1000, run if script
  {
    debug_p_millis = debug_c_millis; // set previous to current
    Serial.println(timer_pulsecount);
    Serial.println(tooth_spacing);
    Serial.println(threepulsetimecopy);
    Serial.println(dpm);
    Serial.println("");
  }
}

// print the above data, timer_pulsecount increments correctly
// timer_p_pulsemicros is set to current when timer_pulsecount = 1
// previous and current pulsemicros latch as they should
// three pulse time saves on 3, resets when timer_pulsecount loops to 1
« Last Edit: October 26, 2012, 12:07:37 pm by jtw11 » Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Looks to me like this statement at the top of the loop()

Code:
dpm = 3 * tooth_spacing / threepulsetimecopy; // causing problem, due to multiplying diff types

will divide by zero (at least on the first loop)?

What is the output of your program - the debug statements?
« Last Edit: October 27, 2012, 01:02:51 am by marco_c » Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

UK
Offline Offline
God Member
*****
Karma: 1
Posts: 530
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

- timer_pulsecount starts at 0, then with button presses increments 1,2,3,1,2,3 correctly.
- tooth_spacing outputs 10
- three_pulsetimecopy outputs the time between pulses 1 & 3, and does not change again until the next three pulses
- dpm starts at some -number, then goes to 0 with three button presses
Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

dpm goes to zero because 10 divided by anything greater than 10 is zero (in integer maths). Even if the multiplication happens first you get the problem with denominator greater than 30.

I am assuming that the key presses will be at least 100ms if they are being done manually, so this calculation will probably never work as you expect.

I would consider combining the rpm & dpm calculations into one and use brackets () to control the flow of calculation so that you do not lose significant digits in the integer maths.
Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

UK
Offline Offline
God Member
*****
Karma: 1
Posts: 530
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Okay, then, with integer math - for example, 10/15 would not equal 0.6667, but 0?

So actually, with massively fast inputs, this code would actually work? But only if the denominator is less than 30.

Would using floats solve this problem, as opposed to integers (haven't got my board or anything with me right now)? As I would have decimal places.

The reasoning left dpm seperate, is I am using it later on in more code to calculate instantaneous position, rpm simply wouldn't be accurate enough.
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 601
Posts: 48543
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Would using floats solve this problem, as opposed to integers
Yes. The tradeoff is that float division is a lot slower than integer division.
Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
So actually, with massively fast inputs, this code would actually work? But only if the denominator is less than 30.

Yes, but you will lose a lot of precision. Using integer maths:
30/1 = 30
30/2 = 15
30/3 = 10
30/4 = 7
...
30/11 = 2 in fact everthing up to /15 = 2
30/16 thru 30 = 1

This is probably no good for you! Floats will work but are slower (as mentioned) and also reqire libraries that take valuable memory.
Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

UK
Offline Offline
God Member
*****
Karma: 1
Posts: 530
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Then, I must use floats. However, what are the libraries you speak of to support floats? Memory and/or RAM are not particularly a problem as this code will eventually run on a 1284P.

Also, in the final application - I was considering running two processors, one a dedicated timer chip that does all of this work, and then sends position via i2c or something every 0.25 or 0.5deg.
Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Libraries are automatically linked in if you use float, so no need to do anything special the.
Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12562
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Then, I must use floats.

Not necessarily. You could use fixed point arithmetic instead. For example, multiply your integer value by 1000 to get three decimal places of precision.

When you're dealing with arithmetic you need to be aware of the range and resolution of your values all the way through your calculation, so that you can avoid overflow or rounding errors at any stage. The numbers shown in your example are small enough for overflow not to be an issue, but you need to consider the full range of real-world values to make sure that you types are capable of holding them.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

UK
Offline Offline
God Member
*****
Karma: 1
Posts: 530
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

So, I've changed the relevant terms to floats - and now, the code works as expected. Great! See the code below. Thank you for all help offered thus far!

Code:
// program will run for 70 minutes until currentmicro overflows
// check what will happen at overflow time
// changed millis to micro, thought of floating point, but too slow
// attempt change back to milli

volatile int timer_pulsecount = 0;
volatile int angle_pulsecount = 0;
float timer_c_pulsemillis = 0;
float timer_p_pulsemillis = 0;
float threepulsetime = 0;
float threepulsetimecopy = 0;
int rpm = 0;
int tooth_spacing = 10;
int lastCommand = 0;
float dpm = 0;
unsigned long debug_p_millis = 0;
unsigned long debug_c_millis = 0;

void setup()
{
  Serial.begin(9600);
  attachInterrupt(0, crank, RISING); // din D2
}

void crank()
{
  timer_pulsecount++; //
}

void loop()
{

  ////////////////// SPEED AND POSITION TRACKING ///////////////////

  timer_c_pulsemillis = millis();
  dpm = 3 * tooth_spacing / threepulsetimecopy; // causing problem, due to multiplying diff types
  rpm = (dpm * 60000) / 360; // converts degrees per micro to rpm

  if (lastCommand != timer_pulsecount){
    lastCommand = timer_pulsecount;
    switch (lastCommand){
    case 1:
      timer_p_pulsemillis = timer_c_pulsemillis;
      break;
    case 3:
      threepulsetime = timer_c_pulsemillis - timer_p_pulsemillis; // needs to be reset to 0
      threepulsetimecopy = threepulsetime;
      break;
    }
  }

  if(timer_pulsecount > 3)
  {
    timer_pulsecount = 1;
    threepulsetime = 0;
  }

  ////////////////// DEBUGGING SERIAL STREAM ///////////////////

  debug_c_millis = millis(); // starts counting currentMillis
  if(debug_c_millis - debug_p_millis >= 500) // if current - 0 is > 1000, run if script
  {
    debug_p_millis = debug_c_millis; // set previous to current
    Serial.println(timer_pulsecount);
    Serial.println(tooth_spacing);
    Serial.println(threepulsetimecopy);
    Serial.println(dpm);
    Serial.println(rpm);
    Serial.println("");
  }
}

// print the above data, timer_pulsecount increments correctly
// timer_p_pulsemicros is set to current when timer_pulsecount = 1
// previous and current pulsemicros latch as they should
// three pulse time saves on 3, resets when timer_pulsecount loops to 1
// current program WORKS!!!! One slight issue, if engine stalls, rpm will still be reported

I'm just about to have a play trying to get it to work using scaled up integer math as suggested. However, one issue I didn't anticipate... if interrupt pulses stop after the rpm calculation has been performed once, rpm will continue to output the last value until another 3 pulses are received and calculated.

Any ideas as to how I would create a piece of code that says something along the lines of the following?

if(no pulses are received in the next 100ms)
{
dpm = 0
}
« Last Edit: October 29, 2012, 07:39:22 am by jtw11 » Logged

Pages: [1] 2   Go Up
Jump to: