magnetic reed switch module

I am trying to build a speedometer using magnetic reed switch module and magnet assembly. To test the code I want to print the frequency of rotation on serial monitor using the below code

unsigned int rpmilli; unsigned long timeold;

void setup() { Serial.begin(9600); digitalWrite(2,HIGH); attachInterrupt(0, rpm_fun, FALLING); rpmilli = 0; timeold = 0; }

void loop() { Serial.print(rpmilli); Serial.print('\n'); delay(600);

} void rpm_fun() {if((millis() - timeold)>50); rpmilli=1/(millis() - timeold); timeold = millis();

}

When magnet is not detected by the sensor, the output is 0, however, when the magnet is detected no matter how fast the magnet is rotating all i get as output is 65535 on serial monitor. Could someone please point out the mistake if any in the code or post any other way of fixing this problem.

I am trying to build a speedometer using magnetic reed switch module and magnet assembly. To test the code I want to print the frequency of rotation on serial monitor using the below code

unsigned int rpmilli; unsigned long timeold;

void setup() { Serial.begin(9600); digitalWrite(2,HIGH); attachInterrupt(0, rpm_fun, FALLING); rpmilli = 0; timeold = 0; }

void loop() { Serial.print(rpmilli); Serial.print('\n'); delay(600);

} void rpm_fun() {if((millis() - timeold)>50); rpmilli=1/(millis() - timeold); timeold = millis();

}

When magnet is not detected by the sensor, the output is 0, however, when the magnet is detected no matter how fast the magnet is rotating all i get as output is 65535 on serial monitor. Could someone please point out the mistake if any in the code or post any other way of fixing this problem.

all i get as output is 65535 on serial monitor.

I'm not sure why that's happening but rpmilli should be a type float because it should always work-out to less than 1. (I would expect you to get zero.)

And, the semicolon ends your if-statement so it doesn't do anything.

  void rpm_fun()
{if((millis() - timeold)>50)
     {
      rpmilli=1/(millis() - timeold);
      timeold = millis();   
     }

How many bits is: unsigned int rpmilli; ? How does integer division work?

(1) This will not work as expected

{if((millis() - timeold)>50);

because of the superfluous semicolon.

(2) Variables that are shared between an Interrupt Service Routine (ISR) and other code should be declared volatile to avoid unexpected optimization by the compiler.

volatile unsigned int rpmilli;

Not sure about timeold. Probably not [because timeold is in setup(), not in loop()], but could not hurt.

(3) It is a good thing that some types of variables are initialized to zero at the start of the program. Variables used by an ISR should be initialized before that interrupt can occur.

  attachInterrupt(0, rpm_fun, FALLING);
  rpmilli = 0;
  timeold = 0;

(4) What LarryD wrote.

If you hit CTRL-T in the Arduino editor, it will nicely fix the indentation, making it more readable and easier to spot mistakes like the semicolon at the end of the if() statement.

unsigned int rpmilli;
unsigned long timeold;

void setup()
{
  Serial.begin(9600);
  digitalWrite(2, HIGH);
  attachInterrupt(0, rpm_fun, FALLING);
  rpmilli = 0;
  timeold = 0;
}

void loop()
{
  Serial.print(rpmilli);
  Serial.print('\n');
  delay(600);

}
void rpm_fun()
{
  if ((millis() - timeold) > 50);
  rpmilli = 1 / (millis() - timeold);
  timeold = millis();
}

as you can see the indentation level of the division statement is the same as the if() statement.

Also variables used in an ISR should be declared as volatile.

Hi, Welcome to the Forum

Please read the first post in any forum entitled how to use this forum. http://forum.arduino.cc/index.php/topic,148850.0.html then look down to item #7 about how to post your code. It will be formatted in a scrolling window that makes it easier to read.

Thanks.. Tom.. :)

What are your design parameters? What is the maximum RPM you will ever have? How often do you need to compute and display the RPM?

I think if you just use the interrupt code to count the interrupts from the sensor, you will have an easier program. then in the main loop, decide when you need to display the RPM, then determine how much time has passed since you last displayed, and turn off interrupts, save the count in a working integer. Turn interrupts back on and compute the RPM based on time and count. You are dealing with RPM, so use one minute as your time base.

Paul

Hi, How have you got the read switch wired? What pin is it connected to, you do not appear to have declared it as an input.

Tom... :)

@winchestr, do not cross-post. Threads merged.

I guess the reed switch is bouncing and millis() - timeOld is zero (it is all happening in the same millisecond) which results in a divide by zero problem. Put a test in your interrupt service routine to reject a pulse if the previous pulse was less than say 50mS ago.

@Tom: Pin 2 low side switch with internal pull-up can be assumed from the code

Hi,

Pin 2 low side switch with internal pull-up can be assumed from the code

Yes I see the shorthand for it, but if its got to go through a compiler, use the proper long term so most people will understand it. It won't make any diff to the Hex file will it.

Does setting an interrupt, automatically set the pin concerned as input?

Tom... :)

TomGeorge: Hi,Yes I see the shorthand for it, but if its got to go through a compiler, use the proper long term so most people will understand it. It won't make any diff to the Hex file will it.

Does setting an interrupt, automatically set the pin concerned as input?

Tom... :)

Of course you are right that he should have used the recommended syntax for declaring the interrupt so it would have been clear.

attachInterrupt(digitalPinToInterrupt(pin), ISR, mode);

Then he could have used something like:

const byte reedSwitchPin = 2 ;
pinMode( reedSwitchPin , INPUT_PULLUP) ;

instead of relying on the default input definition for a pin and writing it high to invoke the pullup resistor.