Program not working as intended

I am trying to create a project that will slowly start a motor, hold the maximum speed until an interrupt triggers, slows down and stops for a while before beginning again. The Arduino Nano circuit works as intended but the code does not. Accelerate seems to run 3 times before the interrupt is triggered, slowdown runs too quickly before Stationary is called and that does not pause before Accelerate runs again. This time the speed terminates at about 148. What have I done wrong? I apologise if any code etc is posted in the wrong format.

const int interruptPin = 3;
#define MOTOR_EN_1_2  10
#define MOTOR_IN1     9
#define MOTOR_IN2     8
int cspeed = 150;// Constant Speed
int i;// General count

//See Version1 for other functions


//*************************************************
void setup()
{
  Serial.begin(9600);
  Serial.println("Motor Speed Up Hold Slow Down with interrupt");
  pinMode(interruptPin, INPUT_PULLUP);
  pinMode(MOTOR_EN_1_2, OUTPUT);
  pinMode(MOTOR_IN1, OUTPUT);
  pinMode(MOTOR_IN2, OUTPUT);
  const unsigned long interval = 200;
  unsigned long previousTime = 0;
  unsigned long currentTime = millis();
  attachInterrupt(digitalPinToInterrupt(3), SlowDown, CHANGE);
   Accelerate();
  }
 
//*********************************************
void Stationary()//Motor stopped for interval time
{
  digitalWrite(MOTOR_IN1, LOW);
  digitalWrite(MOTOR_IN2, LOW);
  const unsigned long interval = 5000;
  unsigned long previousTime = 0;
  unsigned long currentTime = millis();
  if (currentTime - previousTime >= interval)
  {
    // digitalWrite(MOTOR_IN1, LOW);
    // digitalWrite(MOTOR_IN2, LOW);
    Serial.println("Stopped ");
    previousTime = currentTime;
  }
  Accelerate();//Restart after interval
}
//*********************************************
void SlowDown()// triggered by INTERRUPT
{
  digitalWrite(MOTOR_IN1, HIGH);
  digitalWrite(MOTOR_IN2, LOW);
  const unsigned long interval = 400;
  unsigned long previousTime = 0;
  unsigned long currentTime = millis();
  if (currentTime - previousTime >= interval)
  {
    for (int i = cspeed; i >= 0; i--)
    {
      analogWrite(MOTOR_EN_1_2, i);
      Serial.print("Slow Down ");
      Serial.println(i);
    }
    previousTime = currentTime;
  }
  Serial.print(i); Serial.println(" Stopping Motor");
  Stationary();//Motor stopped for a period defined by INTERVAL
}
//*********************************************
void Accelerate()
{
  //int i;
  digitalWrite(MOTOR_IN1, HIGH);
  digitalWrite(MOTOR_IN2, LOW);
  const unsigned long interval = 400;
  unsigned long previousTime = 0;
  unsigned long currentTime = millis();
  if (currentTime - previousTime >= interval)

    while (i <= cspeed)

      for (int i = 50; i <= cspeed; i++)
      {
        analogWrite(MOTOR_EN_1_2, i);
        Serial.print("Speed up ");
        Serial.println(i);
        previousTime = currentTime;
        {
          Coast();
        }
      }
}
//**********************************************
void Coast()
{
  const unsigned long interval = 5000;
  unsigned long previousTime = 0;
  unsigned long currentTime = millis();
  if (currentTime - previousTime >= interval)
  {
    digitalWrite(MOTOR_IN1, HIGH);
    digitalWrite(MOTOR_IN2, LOW);
    {
      analogWrite(MOTOR_EN_1_2, cspeed);
      Serial.print("Maintain Speed ");
      Serial.println("150");
      previousTime = currentTime;
    }
    {
      analogWrite(MOTOR_EN_1_2, cspeed);
      Serial.print("Constant Speed ");
    }
    while (1)//Continues at max speed until INTERRUPT triggered
      delay(1000);
  }
}
//***********************************************
void loop()
{
 Accelerate();
  //Coast();
}
//************************************************

In all cases that you have coded, Current time will ALWAYS be greater than 400. Use serial print to show that actual numbers you are working with.

Paul

Perhaps you meant:
static unsigned long previousTime = 0;
so the value of 'previousTime' doesn't get reset to 0 every time you enter the function.

There are several things to change in your code

variables that are defined inside a function are local to this function
anywhere outside the function the variable is unknown.

everytime you leave such a function the variable is thrown away and created new at the next call of the function.

This means things like

if (currentTime - previousTime >= interval)

can not work as each time the function gets called new previoustime is resetted to zero new

You are assigning the function slowdown to the interrupt

This means each time the interrupt is triggered and only if an interrupt occurs your function slowdown is called.

This is a real mess with a lot of misunderstandings how the code works.

As you have a lot of misconceptions about the code
please write a description in normal words what you want the code to do

I beg you from the bottom of my heart: avoid all programming-words.
use normal words to describe the functionality you want to have

best regards Stefan

I thought I had used Serial.print at various points to show the motor speed and print messages at various points in the program.
Do you have a suggestion for adjusting the "current time"? Because I can't see it, woods and trees stc.

IF you print the values used when you compute the time, you can eliminate the forest and trees.
Paul

OK. Thanks for your time I will have a go in a little while.

What is connected to pin3 and triggers the interrupt? Why have you chosen CHANGE mode?

The interrupt service routine function has many issues, and will not run as you intend. The timed routines depend on millis() changing during the interrupt and the value will not change inside the isr. A review of Nick Gammons tutorial on interrupts will be helpful.

Your best option is to set a volatile boolean flag variable =true in the isr and react to the flag being set in loop() with code like

if(flag==true)
{ //execute all the stuff you want, then set the flag = false}

There is an example of that process in the Gammon tutorial.

You have some recursion going on. You call Accelerate() which calls Coast() which blocks forever. You get an interrupt which calls SlowDown() which calls Stationary() which calls Accelerate() which calls Coast().

And since you called this from an interrupt, interrupts are disabled so you never get them re-enabled and your code will never see another interrupt.

You need to re-design your code as more of a state machine and just have your interrupt routine set a flag to indicate it is time to change states. You should also get rid of those for() loops in your ramps and just do them one step at a time

const int interruptPin = 3;
#define MOTOR_EN_1_2  10
#define MOTOR_IN1     9
#define MOTOR_IN2     8

const int cspeed = 150;// Constant Speed
int currentSpeed;


enum { STATIONARY, SLOWDOWN, ACCELERATE, COAST };
int currentState;

volatile byte flag;

unsigned long currentTime = 0;
unsigned long previousTime = 0;


//*************************************************
void setup()
{
  Serial.begin(9600);
  Serial.println("Motor Speed Up Hold Slow Down with interrupt");
  pinMode(interruptPin, INPUT_PULLUP);
  pinMode(MOTOR_EN_1_2, OUTPUT);
  pinMode(MOTOR_IN1, OUTPUT);
  pinMode(MOTOR_IN2, OUTPUT);
  attachInterrupt(digitalPinToInterrupt(interruptPin), change, CHANGE);
  Accelerate();
}

void change() {
  // interrupt service routine
  flag = 1;
}

//*********************************************
void Stationary()//Motor stopped for interval time
{
  const unsigned long interval = 5000;

  if (currentTime - previousTime >= interval)
  {
    currentState = ACCELERATE;
  }
}

//*********************************************
void SlowDown()
{
  const unsigned long interval = 400;
  static int idx = cspeed;

  digitalWrite(MOTOR_IN1, HIGH);
  digitalWrite(MOTOR_IN2, LOW);

  if (currentTime - previousTime >= interval)
  {
    previousTime = currentTime;
    analogWrite(MOTOR_EN_1_2, currentSpeed);
    Serial.println(currentSpeed);
    currentSpeed--;
    if ( currentSpeed == 0) {
      // we are done decelerating so transition to STATIONARY
      currentState = STATIONARY;
    }
  }
}

//*********************************************
void Accelerate()
{
  const unsigned long interval = 400;

  if (currentTime - previousTime >= interval)
  {
    analogWrite(MOTOR_EN_1_2, currentSpeed);
    Serial.println(currentSpeed);
    currentSpeed++;
    if ( currentSpeed >= cspeed ) {
      // we are done accelerating so transition to COAST
      currentState = COAST;
    }
  }
}

//**********************************************
void Coast()
{
  // nothing to do since we are up to speed
}

//***********************************************
void loop()
{
  currentTime = millis();

  int oldState = currentState;
  
  if ( flag ) {
    currentState = SLOWDOWN;
    flag = 0;
  }

  if ( oldState != currentState ) {
    beginState( currentState );
  }

  checkState( currentState );
}

void beginState( int state ) {

  currentState = state;
  previousTime = currentTime;

  // do whatever you need to start this state
  switch (state) {
    case STATIONARY:
      digitalWrite(MOTOR_IN1, LOW);
      digitalWrite(MOTOR_IN2, LOW);
      Serial.println("Stopped");
      break;

    case SLOWDOWN:
      digitalWrite(MOTOR_IN1, HIGH);
      digitalWrite(MOTOR_IN2, LOW);
      Serial.println("Slowing down");
      break;

    case ACCELERATE:
      digitalWrite(MOTOR_IN1, HIGH);
      digitalWrite(MOTOR_IN2, LOW);
      Serial.println("Speeding up");
      if ( currentSpeed < 50 ) currentSpeed = 50;
      break;

    case COAST:
      Serial.println("at speed");
      break;
  }
}

void checkState( int state ) {

  // do whatever you need to during this state
  switch (state) {
    case STATIONARY:
      Stationary();
      break;

    case SLOWDOWN:
      SlowDown();
      break;

    case ACCELERATE:
      Accelerate();
      break;

    case COAST:
      Coast();
      break;
  }
}

Thank you all for your time. There are some great ideas there and I will spend time evaluating them and see which bear fruit.
Thank you all again.
Tony

Pin3 is connected to a Hall IC and a magnet triggers the interrupt. I chose CHANGE simply because it works. Does it make a difference if I used the other modes? I like the idea of setting a flag and will try and use it. Thank you.

Each side of a square wave will give you and interrupt, so TWO per pulse.
Paul

OK - That makes sense, I hadn't thought of that. I will rethink and use a different method. Thank you for pointing that out.
Tony

OK. The program simply needs to accelerate the motor, hold the speed until the Hall IC is triggered, slow down, stop for a set period of time and then start all over again.

This is much more than I expected. Thank you very much.

very classical case for a software-tecnique called state-machine

a fixed sequence of actions where starting the next action is initiated by one or multiple conditions
best regards Stefan

I wonder if this application actually needs an interrupt, or if it would be enough to just poll the input pin for the appropriate change, ala the state change detect example in the ide?

Thank you. This project originally started out as a way to learn about various coding techniques, interrupts being one of them. I am learning a little as I go on, especially from all of the feedback I am receiving. So much to learn.
Thank you all of you for your time and contributions. Now to try and put it all into practice.

Definitely not if the code is properly structured with no blocking delay()s, etc.

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.