1th project : DCmotor rpm control, need help constraining output

Ok,

so this is my first real project after getting started with arduino. Up to certain level it is already working as expected. I want to control the rpm of a dcmotor ;dunker gr63*25, 40V controlled with BUZ11 mosfet connected to pwm output of the arduino. It’s got an encoder from wich I count the pulses using an isr interupt function and recalculate the pulses to an actual rpm of the outputshaft coming from the mounted gearbox. That bit works fine.

I’ve written this bit of code wich probably sucks but works very well (slow control) just untill the output variable overflows and the pwm output drops and starts building up again. For instance when i put a brake on the shaft while the output is very close to 255. The rpm goes down and the controller increments the output value but goes over 255 and overflows.

Then I tought; let’s include a constrain within the loop so that it doesn’t increment highter than 255. That doesn’t work at all :confused: I can’t get my head around it. Anybody kind enough to explain me why that doesn’t work and even better; what the best approach for my problem is?

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


LiquidCrystal_I2C lcd(0x27,16,2); //set the LCD address to 0x27 for a 16 chars and 2 line display

int analogPin = 1;
int control;
int setP = 1800;
int pulse = 0;
volatile unsigned long firstPulseTime;
volatile unsigned long lastPulseTime;
volatile unsigned long numPulses;
volatile byte output = 0;

void isr()
{
  unsigned long now = millis();
  if (numPulses == 2)
  {
    firstPulseTime = now;
  }
  else
  {
    lastPulseTime = now;
  }
  ++numPulses;
}
byte controlFunction(int x, int y)
{
  int controller;
  if (x < y)
  {
    controller = 1;
  }
  else 
  {
    controller = -1;
  }
  return controller;
}

void setup() 
{
    TCCR2B = TCCR2B & B11111000 | B00000001; // change pwmfreq to cancel motornoise
    Serial.begin(9600);                   
    lcd.init(); 
    lcd.backlight();
    lcd.setCursor(0, 0);
    lcd.print("PwmRpmcontrol");
    lcd.setCursor(0, 1);
    lcd.print("starting...");
    delay(1000);
    lcd.clear();
    lcd.print("rpm      out");
    pinMode(11, OUTPUT);
}

float pulseCounter(unsigned int sampleTime)
{
  numPulses = 0;                     
  attachInterrupt(0, isr, FALLING); 
  delay(sampleTime);
  detachInterrupt(0);
  return (numPulses < 100) ? 0 : (numPulses);
}

void loop() 
{
   pulse = pulseCounter(100);
   control = controlFunction(pulse, setP);
   output = output + control;
   output = constrain(output, 0, 255);
   analogWrite(11, map(output, 0, 255, 35, 255));
   lcd.setCursor(9, 1);
   lcd.print(output);
   float rpm = pulse/8;
   lcd.setCursor(0, 1);
   lcd.print(rpm);
}

Then I tought; let's include a constrain within the loop so that it doesn't increment highter than 255. That doesn't work at all

What does not working at all mean ? What does it do that it shouldn't and/or what doesn't it do that it should ?

byte controlFunction(int x, int y)
{
  int controller;
  if (x < y)
  {
    controller = 1;
  }
  else 
  {
    controller = -1;
  }
  return controller;
}

The function says that it returns a byte. The return statement says that it returns an int. Which is it?

This is really a useless function. There is no advantage to calling a function to compare two values over just comparing them where needed.

The whole function just screams for the use of the ternary operator.

int controlFunction(int x, int y)
{
   return (x < y) ? +1 : -1;
}
float pulseCounter(unsigned int sampleTime)
{
  numPulses = 0;                     
  attachInterrupt(0, isr, FALLING); 
  delay(sampleTime);
  detachInterrupt(0);
  return (numPulses < 100) ? 0 : (numPulses);
}

Using an interrupt and delay() just looks stupid. Returning an int as a float doesn’t look too smart, either.

   pulse = pulseCounter(100);

Storing that float in an int then really makes me wonder.

Thanks for pointing out my stupid mistakes there PaulS. Bought this arduino about three weeks ago and though I have very limited knowledge on analog electronics, programming this are really my first steps in this field.

I’ve corrected them according to your tips and now everything works. Output doesn’t overflow anymore. The fault was probably in the whole int-byte-float variable conversion messup… :-*

Now I can start adding functionality for updating the rpm setpoint.

This the working code, thanks again!;

#include <Wire.h>
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27,16,2); //set the LCD address to 0x27 for a 16 chars and 2 line display


int setP = 500;
int pulse = 0;
int output = 0;
volatile unsigned long firstPulseTime;
volatile unsigned long lastPulseTime;
volatile unsigned long numPulses;

void isr()
{
  unsigned long now = millis();
  if (numPulses == 2)
  {
    firstPulseTime = now;
  }
  else
  {
    lastPulseTime = now;
  }
  ++numPulses;
}

void setup() 
{
  TCCR2B = TCCR2B & B11111000 | B00000001;
  Serial.begin(9600);  
  lcd.init(); 
  lcd.backlight();
  lcd.setCursor(0, 0);
  lcd.print("PwmRpmcontrol");
  lcd.setCursor(0, 1);
  lcd.print("starting...");
  delay(1000);
  lcd.clear();
  lcd.print("rpm      out");
  pinMode(11, OUTPUT);
}

int pulseCounter(unsigned int sampleTime)
{
  numPulses = 0;                    
  attachInterrupt(0, isr, FALLING);
  delay(sampleTime);
  detachInterrupt(0);
  return (numPulses < 100) ? 0 : (numPulses);
}

int controlFunction(int x, int y)
{
  return (x < y) ? +1: -1;
}

void loop() 
{
  pulse = pulseCounter(50);
  int control = controlFunction(pulse, setP);
  output = output + control;
  output = constrain(output, 0, 255); 
  analogWrite(11, map(output, 0, 255, 35, 255));
  lcd.setCursor(9, 1);
  lcd.print(output);
  int rpm = pulse/4;
  lcd.setCursor(0, 1);
  lcd.print(rpm);
}