Separating control and measurement

I had almost no touch with arduino uno and programming and I’m supposed to make some a project. I decided to make a simple station to measure rotation speed of DC engine. I’m controlling its speed and direction with joystick and L293D. Moreover I’m lighting up 7 diodes LED which depends on speed and direction. In next step I want to display calculated value of rotation speed on LCD display.

To this point everything was pretty clear but I have to measure rotation speed with hall effect sensor. At first I tried to use attachInterrupt function and I was able to measure total number of impulses from magnets placed on wheel but I didn’t know how to change it into RPM. I asked google for help and I found some working code. It measures RPM every 1 second but it also causes some issue with control of engine. This part of code makes that I can change speed and direction of engine every second aswell. I tried to use millis() function to separate parts of code responsible for control and measurement but with no effect.

A project kind of meets the assumptions but I’d rather to change rotation speed and its direction with much smaller delay.

The main code:

int speedPin = 5;
int kier1 = 4; //direction1
int kier2 = 3; //direction2
int mSpeed;
int mRPM;
int jPin = A1;
int Yjoy;

int D1 = 13;
int D2 = 12;
int D3 = 11;
int D4 = 10;
int D5 = 9;
int D6 = 8;
int D7 = 7;

const int hall = 2;
const unsigned long sampleTime = 1000;

#include <Wire.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd = LiquidCrystal_I2C(0x27, 16, 2);

void setup() {
  // put your setup code here, to run once:
  pinMode(speedPin, OUTPUT);
  pinMode(kier1, OUTPUT);
  pinMode(kier2, OUTPUT);
  pinMode(jPin, INPUT);
  pinMode(13, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(2, INPUT_PULLUP);

  lcd.init();
  lcd.backlight();
  Serial.begin(9600);

}

void loop() {
  // put your main code here, to run repeatedly:
  digitalWrite(D4, HIGH);

  Yjoy = analogRead(jPin);

  //STEROWANIE SILNIKIEM / Control of engine
  //Serial.println(Yjoy);
  if (Yjoy < 487) {
    digitalWrite(kier1, LOW);
    digitalWrite(kier2, HIGH);
    mSpeed = -255. / 478.*Yjoy + ((487.*255.) / 478.);
    analogWrite(speedPin, mSpeed);
  }
  if (Yjoy >= 487) {
    digitalWrite(kier1, HIGH);
    digitalWrite(kier2, LOW);
    mSpeed = (255. / 533.) * Yjoy - ((255.*487.) / 533.);
    analogWrite(speedPin, mSpeed);
  }
  float mRPM = mSpeed * 3000. / 255.;
  //Serial.println(mRPM);
  //Serial.println(mSpeed);


  //DIODY LED
  if (mRPM < 100 && Yjoy >= 487) {
    digitalWrite(D1, LOW);
    digitalWrite(D2, LOW);
    digitalWrite(D3, LOW);
    digitalWrite(D5, LOW);
    digitalWrite(D6, LOW);
    digitalWrite(D7, LOW);
  }
  if (mRPM < 100 && Yjoy < 487) {
    digitalWrite(D1, LOW);
    digitalWrite(D2, LOW);
    digitalWrite(D3, LOW);
    digitalWrite(D5, LOW);
    digitalWrite(D6, LOW);
    digitalWrite(D7, LOW);
  }
  if (mRPM > 100 && Yjoy >= 487) {
    digitalWrite(D1, LOW);
    digitalWrite(D2, LOW);
    digitalWrite(D3, LOW);
    digitalWrite(D5, HIGH);
    digitalWrite(D6, LOW);
    digitalWrite(D7, LOW);
  }
  if (mRPM > 1500 && Yjoy >= 487) {
    digitalWrite(D1, LOW);
    digitalWrite(D2, LOW);
    digitalWrite(D3, LOW);
    digitalWrite(D5, HIGH);
    digitalWrite(D6, HIGH);
    digitalWrite(D7, LOW);
  }
  if (mRPM > 2500 && Yjoy >= 487) {
    digitalWrite(D1, LOW);
    digitalWrite(D2, LOW);
    digitalWrite(D3, LOW);
    digitalWrite(D5, HIGH);
    digitalWrite(D6, HIGH);
    digitalWrite(D7, HIGH);
  }
  if (mRPM > 100 && Yjoy < 487) {
    digitalWrite(D1, LOW);
    digitalWrite(D2, LOW);
    digitalWrite(D3, HIGH);
    digitalWrite(D5, LOW);
    digitalWrite(D6, LOW);
    digitalWrite(D7, LOW);
  }
  if (mRPM > 1500 && Yjoy < 487) {
    digitalWrite(D1, LOW);
    digitalWrite(D2, HIGH);
    digitalWrite(D3, HIGH);
    digitalWrite(D5, LOW);
    digitalWrite(D6, LOW);
    digitalWrite(D7, LOW);
  }
  if (mRPM > 2500 && Yjoy < 487) {
    digitalWrite(D1, HIGH);
    digitalWrite(D2, HIGH);
    digitalWrite(D3, HIGH);
    digitalWrite(D5, LOW);
    digitalWrite(D6, LOW);
    digitalWrite(D7, LOW);
  }
  //WYŚWIETLANIE / LCD display
  lcd.setCursor(0, 0);
  lcd.print("Vobl=");
  lcd.setCursor(5, 0);
  if (Yjoy >= 487) {
    lcd.print(mRPM);
    lcd.setCursor(12, 0);
    lcd.print("P");
  } else {
    lcd.print(mRPM);
    lcd.setCursor(12, 0);
    lcd.print("L");
  }
  lcd.setCursor(13, 0);
  lcd.print("RPM");
  lcd.setCursor(0, 1);
  lcd.print("Vzm =");

//RPM measurement
  int rpm = getRPM();
  //if (rpm > rpmMaximum) rpmMaximum = rpm;
  lcd.clear();
  displayRPM(rpm);
}

int getRPM()
{
  int count = 0;
  bool countFlag = LOW;
  unsigned long currentTime = 0;
  unsigned long startTime = millis();

  while (currentTime <= sampleTime) {
    currentTime = millis() - startTime;
    if (digitalRead(hall) == HIGH)
    {
      countFlag = HIGH;
    }
    if (digitalRead(hall) == LOW && countFlag == HIGH)
    {
      count++;
      countFlag = LOW;
    }

  }
  int countRpm = int(60000 / float(sampleTime)) * count / 3.; //Mniejszy czas próbkowania = mniejsza rozdzielczość
  return countRpm;
}
void displayRPM(int rpm)
{
  Serial.print("RPM = ");
  Serial.println(rpm);
  //Serial.print("  MAX RPM = ");
  //Serial.println(rpmMaximum);
  lcd.setCursor(5, 1);
  lcd.print(rpm);
  lcd.setCursor(13, 1);
  lcd.print("RPM");
}

attachInterrupt attempt:

volatile word steps;
void setup() {
  // put your setup code here, to run once:
int hall=2;

Serial.begin(9600);
pinMode(2,INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(2), step,FALLING);
}

void loop() {
  // put your main code here, to run repeatedly:
Serial.println(steps);
delay(1000);
}
void step()
{
  static unsigned long lastTime;
  unsigned long timeNow = millis();
  if(timeNow-lastTime<10) return;

  steps++;
  lastTime=timeNow;
}

It's not clear to me what you want to do with the RPM derived from the ISR.

I suggest a couple of things in order to make progress.

First, add to the short ISR program the code needed to convert the count into speed.

Second, and quite separate, I suggest you break up the main program into more functions. For example have a function for check the potentiometer, another to update the LEDs and another to control the motor. That way your code in loop can reduce to something like this

void loop() {
   checkPotentiometer();
   checkRPM();
   updateLEDs();
   operateMotor();
}

This allows you to test each part separately and makes it easy to introduce another part, or to swap one part for a different part.

...R
Planning and Implementing a Program

an interrupt-service-routine (in short "isr") should run through as fast as possible

So all that is done inside the isr should be counting up pulses.

I don't understand why you added

  unsigned long timeNow = millis();
  if(timeNow-lastTime<10) return;

to the isr.

variables that are used in an isr should have the additional workd volatile

Then your main-loop should make a copy of variable steps once every second
timed by a timer based on millis() (which is non-blocking)
delay is blocking

here is a demo-code that compiles but I haven't tested it real with real pulses
This code counts steps (hall-pulses) and calculates rpm

volatile word steps;
volatile unsigned long TimeStamp;
volatile unsigned long StartOfNewPeriod;

unsigned long StepsForRpm;
unsigned long TimeStampForRpm;
unsigned long StartOfNewPeriodForRpm;

unsigned long MyTimer;

unsigned long rpm;

const int hall = 2;

void isr_step() {
  // get exact time when pulse occurred for a more precise rpm-calculation
  TimeStamp = millis(); 
  // if your code needs more time than the standard-measuring-period  
  // using the difference of the two timestamps   "TimeStamp" and "StartOfNewPeriod" 
  // will correct this
  
  if (steps == 0) { // if new period starts
    StartOfNewPeriod = TimeStamp;
  }
  steps++;
}

void setup() {
  // use a higher baudrate to make serial print faster
  // make sure the baudrate in the serial monitor matches this value
  Serial.begin(115200); 
  pinMode(hall, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(hall), isr_step, FALLING);
}

boolean TimePeriodIsOver (unsigned long &expireTime, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - expireTime >= TimePeriod )
  {
    expireTime = currentMillis; // set new expireTime
    return true;                // more time than TimePeriod) has elapsed since last time if-condition was true
  }
  else return false;            // not expired
}


void loop() {
  
  // function TimePeriodIsOver returns true only once every second
  // loop is still running at maximum speed to do other things  
  if ( TimePeriodIsOver(MyTimer,1000) ) {

    noInterrupts();               // disable interrupts to avoid missed counting or changing isr-variables   
    StepsForRpm = steps;          // quickly store copy of values
    TimeStampForRpm = TimeStamp;  
    StartOfNewPeriodForRpm = StartOfNewPeriod;
    steps = 0; // reset steps to start new measuring-period
    interrupts(); // enable interrupts

    // do the rest while isr is already counting for new period
    // multiply with 60 for minute multiply with 1000 for milliseconds
    rpm = StepsForRpm * 60 * 1000 / (TimeStampForRpm - StartOfNewPeriod);
    Serial.println(StepsForRpm);
    Serial.println(rpm);    
  }  
}

best regards Stefan

It seems to work perfectly well. I don't know what to say. You've even written comments to make it more clear. I really appreciate the amount of effort both of you put to help me with understanding this.

Best regards :slight_smile: