Problems with my motor/sensor control .

Hey guys! I'm new here, and I have a problem with my program. Basically , I've built an RPM counter , using the IR Sensor, LCD to show the value, and one button which has the functionality to save the current RPM when pressed ( using an interrupt function ). My problem is that when I power up the motor , my LCD is showing random readings ( even on the second line where I have my saved RPM , that should go there only when I press my button ) , even tho my sensor is not facing the motor for the reading. I think it has something to do with analogWrite( motor ,120 ) ; and where to place it ..

FYI : im powering the motor with a different power supply and using a simple transistor and diode to control it. For the sensor im using this schematic , the only difference being that my arduino is connected to my laptop

#include<LiquidCrystal.h> //librarie LCD
LiquidCrystal lcd(7, 8, 9, 10, 11, 12); //interfatare pini LCD
volatile unsigned long rev = 0;
volatile unsigned long OK;
unsigned long rpm;
unsigned long time , oldtime = 0 ;
const int buton = 3;
const int motor = 5;
void RPM_Count()
{
  rev++;
}
void button()
{
  OK = rpm;
}
void setup()
{
  lcd.begin(16, 2); // initializare LCD
  pinMode(motor, OUTPUT);
  pinMode(buton, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(2), RPM_Count, RISING);
  attachInterrupt(digitalPinToInterrupt(3), button, FALLING); 
}

void loop()
{
  analogWrite(motor, 120);
  delay(1000); // update RPM la fiecare secunda
  noInterrupts();
  time = millis() - oldtime;    // gasim timpul
  rpm = (rev * 60000 / time);   //calculam RPM ( 60.000 miliseconds = 1 minut)
  oldtime = millis();           //salvam timpul curent;
  rev = 0;
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print(rpm);
  lcd.print(" RPM");
  interrupts();
  lcd.setCursor(0, 1);
  lcd.print("Saved:");
  lcd.print(OK);
}
  • you should disable and re-enable interrupts for as short a period of time as possible. I would disable interrupts, read the value of rev into another variable, reset it to zero and re-enable interrupts.

  • instead of setting oldtime to millis() just set it to time

gcjr:

  • you should disable and re-enable interrupts for as short a period of time as possible. I would disable interrupts, read the value of rev into another variable, reset it to zero and re-enable interrupts.

  • instead of setting oldtime to millis() just set it to time

I dont quite understand what you mean . Can you write it down , please ?

    noInterrupts();
    int rev2 = rev;
    rev = 0;
    interrupts();

    time = millis() - oldtime;
    rpm = (rev2 * 60000 / (time - oldtime);
    oldtime = time;

gcjr:

    rpm = (rev2 * 60000 / (time - oldtime);

Why did you divide by ( time - oldtime ) there ?

wickfm:
Why did you divide by ( time - oldtime ) there ?

did math fail you?

Capture.PNG

Capture.PNG

wickfm:
Why did you divide by ( time - oldtime ) there ?

it made the assumption that "time = millis()". And I got confused because you were calling millis() twice, but oldtime wasn't being used until the next second.

   time = millis() - oldtime;
    // gasim timpul
    rpm = (rev * 60000 / time);
    //calculam RPM ( 60.000 miliseconds = 1 minut)
    oldtime = millis();

for me, "time - oldtime" is clearly the delta between readings. time and oldtime are timestamps

please think it out and post what you think is correct