PWM motor speed question

Hi,
I have 4 LMD 18200 motor driver boards that I am using to control 3 pumps and a stir motor. The inputs are PWM, Direction and Brake. As only one pump is running at any given time all of the pumps share a PWM signal. Individually I am controlling the direction and then releasing the brake. Then the PWM signal will ramp up the speed of the pump to max then the timer counts up to a set point and then the PWM signal ramps down the speed and the moves on to the next state. I have the basic ramp up and down working but I am having trouble integrating it into the existing program. Can you look at this and give me some guidance?

//////////////////////////////////////////
void HandleFILLING1State()
{         
  TSread(); // Check Touch screen for abort request
  counter = counter + 1; //Display print counter
  if (counter <2) // print screen once
  {
    ///
    tft.fillRect(10, 10, 320, 90, BLACK);
    tft.drawRect(15, 20, 282, 90, GREEN);
    tft.fillRect(15, 112, 282, 95, BLACK);
    tft.setCursor(0, 35);
    tft.setTextColor(GREEN);   
    tft.setTextSize(4);
    tft.println("  Press to");
    tft.println(" Abort Cycle");
    tft.setTextColor(RED);
    tft.setTextSize(4);
    tft.setCursor(70, 140);
    tft.println("Filling");
    tft.setCursor(71, 170);
    tft.println("Cleaner");
  } 
  ///////////////////////
  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH); //Fill so leave direction pin high 
  digitalWrite(pump1_brake,     LOW); //Release brake on pump
  digitalWrite(pump2_direction,HIGH);
  digitalWrite(pump2_brake,    HIGH);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    HIGH);
  for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
    analogWrite(44, motorspeed);    // ramp up motor speed PWM pin
    Serial.println("ramping up ");
    delay(pump_ramp); //
  } //motor now running max speed
  static unsigned long lastTick = 0; // set up a local variable to hold the last time we moved forward one second
  // (static variables are initialized once and keep their values between function calls)
  if (millis() - lastTick >= 1000) {

    lastTick = millis();
    second++; // add second

    Serial.print("Second = ");
    Serial.println(second);

    if (second > FILLING1_TIME){ //time out
      for (int motorspeed = pump_speed; motorspeed >= 0; motorspeed--) {
        analogWrite(44, motorspeed);
        Serial.println("ramping down ");
        delay(pump_ramp);
      } //motor is now stopped
      clearcenter();
      counter = 0 ;
      second = 0 ;
      state=STIRRING1;
    }  
  }

  if (counter > 5){
    counter = 3 ;
  }
}

you should provide informations about other variables and their values before calling this void ... but let's try to gues these things and presume some .... from the description of "pump cycle" you wrote i gues that pump should run for FILLING1_TIME (in seconds) at full speed from the time it is reached .... then you should set up lastTick variable for the first time at actual millis(), not zero

  } //motor now running max speed
  static unsigned long lastTick = millis(); // set up a local variable to hold the last time we moved forward one second

then the interval for full run should be measured well up to FILLING_TIME

second - this is only question, it should not affect void itself .... why do you reset display counter to 3 if it is >5?

  if (counter > 5){
    counter = 3 ;
  }

by the way, on the closer look i must ask, in what interval you are calling this void from ouside code? because if it is less than one second, you never step inside first "if" and do not print seconds and do not test FILLING_TIME and do not ramp down the pump ....

in what interval you are calling this void from ouside code?

It is not a void, it is a function
void is the type of variable that the function returns, ie none

UKHeliBob:

in what interval you are calling this void from ouside code?

It is not a void, it is a function
void is the type of variable that the function returns, ie none

... yes, you'r right, let's call it subroutine .... but it does not matter from point of what i asked

by the way, on the closer look i must ask, in what interval you are calling this void from ouside code? because if it is less than one second, you never step inside first "if" and do not print seconds and do not test FILLING_TIME and do not ramp down the pump ....

This just keeps the display from flickering. It prints the screen if counter is less than 2 then keeps the counter less tan 5. Without this the counter overflows and the display begins to flicker

The function is one of many states in a state machine. I had relays driving the pumps and the timer stuff worked well. But when I added the pwm routine the ramp up loop runs several times in the function while running.

This is the offending code

for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
    analogWrite(44, motorspeed);

int motorspeed = 0;

Every time thru the loop it initializes motorspeed to 0. I need a for next loop that will let me use a variable that has already been initialized in setup maybe.

Here is the stand alone PWM program this works but when I add it to the program I have the above problem. Maybe there is another approach.

////////////////////////////////////
int stir_speed = 80;
int stir_ramp = 20;
int pump_speed = 250;
int pump_ramp = 4;
////////////////////////////////////


#define stir_brake 25
#define pump1_brake 27
#define pump2_brake 29
#define pump3_brake 31
#define pump1_direction 33
#define pump2_direction 35
#define pump3_direction 37
void setup() {

  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    HIGH);
  digitalWrite(pump2_direction,HIGH);
  digitalWrite(pump2_brake,    HIGH);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    HIGH);
  
  
  
    pinMode(42,              OUTPUT); //Analog pin
    pinMode(44,              OUTPUT); //PWM Output common to all motor drivers
    pinMode(46,              OUTPUT); //Analog pin
    pinMode(stir_brake,      OUTPUT); 
    pinMode(pump1_brake,     OUTPUT);
    pinMode(pump1_direction, OUTPUT); 
    pinMode(pump2_brake,     OUTPUT);
    pinMode(pump2_direction, OUTPUT); 
    pinMode(pump3_brake,     OUTPUT);    
    pinMode(pump3_direction, OUTPUT);
 
 
 int motorspeed = 0;
}

void loop() {
  
 
  digitalWrite(stir_brake,     LOW);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    HIGH);
  digitalWrite(pump2_direction,HIGH);
  digitalWrite(pump2_brake,    HIGH);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    HIGH);
  
  
  
    for ( int motorspeed ; motorspeed < stir_speed; motorspeed++) {
      analogWrite(44, motorspeed);
      delay(stir_ramp);
    } 
    
     delay(2000);
    
    for (int motorspeed = stir_speed; motorspeed >= 0; motorspeed--) {
      analogWrite(44, motorspeed);
      delay(stir_ramp);
    } 
   
    delay(20);  
  /////////////////////
  
  
  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    LOW);
  digitalWrite(pump2_direction,HIGH);
  digitalWrite(pump2_brake,    HIGH);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    HIGH);
  

  
    for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    
     delay(2000);
    
    for (int motorspeed = pump_speed; motorspeed >= 0; motorspeed--) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    // pause between LEDs:
    delay(20);  
  

  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    LOW);
  digitalWrite(pump2_direction,LOW);
  digitalWrite(pump2_brake,    HIGH);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    HIGH);
  // iterate over the pins:
  
    for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    
     delay(2000);
   
    for (int motorspeed = pump_speed; motorspeed >= 0; motorspeed--) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    delay(20);  
    /////////////////////
  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    HIGH);
  digitalWrite(pump2_direction,HIGH);
  digitalWrite(pump2_brake,    LOW);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    HIGH);
  
    for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    
     delay(2000);
    for (int motorspeed = pump_speed; motorspeed >= 0; motorspeed--) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    delay(20);
  

      /////////////////////
  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    HIGH);
  digitalWrite(pump2_direction,LOW);
  digitalWrite(pump2_brake,    LOW);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    HIGH);
  
    for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    
     delay(2000);
    for (int motorspeed = pump_speed; motorspeed >= 0; motorspeed--) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    delay(20);  
     /////////////////////
  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    HIGH);
  digitalWrite(pump2_direction,HIGH);
  digitalWrite(pump2_brake,    HIGH);
  digitalWrite(pump3_direction,HIGH);  
  digitalWrite(pump3_brake,    LOW);
  
    for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    
     delay(2000);
    for (int motorspeed = pump_speed; motorspeed >= 0; motorspeed--) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    delay(20);  
     /////////////////////
  digitalWrite(stir_brake,     HIGH);
  digitalWrite(pump1_direction,HIGH);
  digitalWrite(pump1_brake,    HIGH);
  digitalWrite(pump2_direction,HIGH);
  digitalWrite(pump2_brake,    HIGH);
  digitalWrite(pump3_direction,LOW);  
  digitalWrite(pump3_brake,    LOW);
  
    for (int motorspeed = 0; motorspeed < pump_speed; motorspeed++) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    
     delay(2000);
    for (int motorspeed = pump_speed; motorspeed >= 0; motorspeed--) {
      analogWrite(44, motorspeed);
      delay(pump_ramp);
    } 
    delay(20);  

  
}

urthlight:
The function is one of many states in a state machine. I had relays driving the pumps and the timer stuff worked well. But when I added the pwm routine the ramp up loop runs several times in the function while running.

maybe i am suspecting what is going on .... let me first clarify one thing .... is the function supposed to do whole cycle of pump during one call (it means ramp-up, run max speed until FILLING_TIME and then ramp-down) and then exit?

... yes, you'r right, let's call it subroutine .... but it does not matter from point of what i asked

Sorry, but I think that it does matter. Sloppy use of language leads to misunderstandings, but let's agree to differ and move on.

lumptom:
... yes, you'r right, let's call it subroutine .... but it does not matter from point of what i asked

No, let's call it a function since that is the only correct name for it.

The implementation has blocking code to ramp the speed up and down but does not have any blocking code to wait for the elapsed time to pass. It also has a couple of global/static variables that it is incrementing and using to trigger display output. The only way that makes sense is for HandleFILLING1State() to be called repeatedly, so I assume that's what is happening. In that case, the logic to turn the motor on and ramp the speed up should only be called on entry to the state. I would guess that corresponds to (counter == 1). You could add an if(counter==1){} statement and move the digital writes and acceleration ramp inside it.

My suggestion, for what it's worth, is that you break this state and this function up into three separates states corresponding to the accelerating, constant speed and decelerating phases. Each state could then be implemented with a trivial non-blocking piece of code and the use of global data would almost disappear.

Given that the FILLING1_TIME is defined separately from the pump acceleration characteristics, I wonder whether it's conceivable that the timeout could occur before the acceleration had completed, and what you would want to happen in that case.

is the function supposed to do whole cycle of pump during one call (it means ramp-up, run max speed until FILLING_TIME and then ramp-down) and then exit?

Yes exactly.

Being self-taught I did not have this verbage.

The implementation has blocking code to ramp the speed up and down but does not have any blocking code to wait for the elapsed time to pass.

This is the problem.

It also has a couple of global/static variables that it is incrementing and using to trigger display output.

Actually it inhibits repainting the screen and causing a flicker.

The only way that makes sense is for HandleFILLING1State() to be called repeatedly, so I assume that's what is happening.

Correct. The main loop is just a big switch case tree (correct verbage?) that sends the program to the proper Function continuously until the conditions dictate a state change.

the logic to turn the motor on and ramp the speed up should only be called on entry to the state. I would guess that corresponds to (counter == 1).

Correct. And the ramp down is the last thing to do other than resetting the variables.

You could add an if(counter==1){} statement and move the digital writes and acceleration ramp inside it.

This was the best I could come up with but it seemed inelegant.

My suggestion, for what it's worth, is that you break this state and this function up into three separates states corresponding to the accelerating, constant speed and decelerating phases. Each state could then be implemented with a trivial non-blocking piece of code and the use of global data would almost disappear.

This Would be better but I have 29 states right now so I would have to break up and render 21 of them up to do so.

Given that the FILLING1_TIME is defined separately from the pump acceleration characteristics, I wonder whether it's conceivable that the timeout could occur before the acceleration had completed, and what you would want to happen in that case.

If I understand what you are asking-
I am ramping up to max speed on the pumps then running for 10 minutes and then ramping down to fill . The stirrer ramps up to a preset speed runs for 10 minutes and then ramps down and then reversing the pump for drain. I want to Start the timer after the motor reaches speed and then decel after counter time-out.

urthlight:
Maybe there is another approach.

    for ( int motorspeed ; motorspeed < stir_speed; motorspeed++) {

analogWrite(44, motorspeed);
      delay(stir_ramp);
}

Another approach:

if (motorspeedA >= stirspeed){ //?? reached desired speed?
// next step
// Rest of your program
//
}
else {
   motorspeedA++;
   analogWrite(44, motorspeedA);
   delay(stir_ramp);
}

This will loop until the motor has reached the desired speed, then go onto your next step.
You'll need to use different variable names for each of your motorspeeds within the program.

urthlight:
This was the best I could come up with but it seemed inelegant.

As far as I can see, you're using the global counter variable as a sub-state which controls actions within the current state. It's not the most elegant FSM design I've ever seen but it seems viable, and with that scheme in place it seems appropriate to use that to handle startup processing for this state. I don't see that this is any less elegant than the one-off display output.

Just out of curiosity, are any of the other states similar to this one? I noticed that several identifiers associated with this state contain a number, which always makes me suspicious. The mixed use of of blocking and non-blocking code, widespread use of global data (which now requires duplicate handling for each state) and sheer number of states would make me want to rationalise the state machine.

if (motorspeedA >= stirspeed){ //?? reached desired speed?
// next step
// Rest of your program
//
}
else {
   motorspeedA++;
   analogWrite(44, motorspeedA);
   delay(stir_ramp);
}

I can wrap my head around this.

As far as I can see, you're using the global counter variable as a sub-state which controls actions within the current state. It's not the most elegant FSM design I've ever seen but it seems viable, and with that scheme in place it seems appropriate to use that to handle startup processing for this state. I don't see that this is any less elegant than the one-off display output.

LOL. I am sure it is not the most elegant ever it is the only one I have ever written and example code was hard to come by. I really did not like what I came up with for the display but it worked so I moved on.

Just out of curiosity, are any of the other states similar to this one? I noticed that several identifiers associated with this state contain a number, which always makes me suspicious. Yes there is 3 of every thing (fill, stir and drain). Suspicious of what?

The mixed use of of blocking and non-blocking code, widespread use of global data (which now requires duplicate handling for each state) and sheer number of states would make me want to rationalise the state machine.
Post

And, you lost me. I will look into rationalising.

This is the Prototype and there is another version coming so I am really trying to wrap this up for the beta testers. So right now function is more important than style.

urthlight:
Suspicious of what?

Redundant states. If you have a lot of states representing similar activities in different contexts, perhaps you would be better to consolidate them into one set of states which has parameters which specify the context.

There are also ways to structure the state machine that reduce the code and data replication between states and make each state more self contained. One that works well is to have the handler function for each state return the new state, and have your state machine framework compare old/new states to detect state transitions. You can use that to trigger exit processing for the old state and entry processing for the new state. For example actions such as initialising sub-state variables such as motor speed and run duration, and starting the motor, could be handled by state entry code. This is a good place to put logging, too. Then the processing for each state would be reduced to 'if the motor speed has reached the target, move to the next state' and so on. It may not apply here, but this approach means that entry/exit processing for each state is always done reliably, without any code replication, regardless of how complex the sequences of state transitions can become. If you need to increase the complexity even further, it gives you a hook which you can use to manage compound finite state machines.

This sounds good for rev 2. It will take a lot of consideration. And yeah the original plan called for 12 states but it evolved. There are redundant states that could have values passed to a single state, but you know how it is, "Its working move on to the next phase" pressure.Still I am only using 20 % of the mega's program space. Do you have any simple FSM code that you are willing to share. I found the example for the remote car starter and learned a lot from that. Bad habits included. I would be done by now but I had to upgrade the motor controls from relays to an H-Bridge.

OK, I am looking at using a function to spool up and spool down the motors and handle the duration timer. Here is what I have that does not work. Please Help!

//////////////////////////////

#define STIRRING1_TIME 20  //   1200
#define Srelay    51
int stir_speed = 180;
int time = 0;  
int pump_pwm_del = 1;
int second = 0;
void setup() {
  Serial.begin(57600);
  Serial.println("setup");

  digitalWrite(23,HIGH);
  
  /////////////////////////////
  pinMode(Srelay, OUTPUT); // Provides/Removes Power to motor controllers
  pinMode(44, OUTPUT); //Shared PWM Signal to all 4 motor controllers
  pinMode(23, OUTPUT);   //Stir motor
  digitalWrite(Srelay,LOW);//Turn off power to motor controllers
}

void loop() {
  digitalWrite(Srelay,HIGH);//Turn off power to motor controllers   
  Serial.println("23");
  stir(10);
 
}

/////////////////////////////////////////////////////////
//Stir
int stir(int time){
  digitalWrite(23,LOW); // Release Brake Stir Motor
     Serial.print("Brake On ");
  for (int motorspeed = 0; motorspeed < stir_speed; motorspeed++) {
    analogWrite(44, motorspeed);// Ramp up speed
     Serial.print("motorspeed = ");
    Serial.println(motorspeed);
    delay(4);
  } 
  //motor at speed start counting
  Serial.print("motorspeed = MAX ");
  static unsigned long lastTick = 0; // set up a local variable to hold the last time we moved forward one second
  // (static variables are initialized once and keep their values between function calls)
  if (millis() - lastTick >= 1000) {
    
    lastTick = millis();
    second++;
   
    
    Serial.print("Second = ");
    Serial.println(second);

   
    if (second > time){   // Count over decel motor
      Serial.print("Time Out");
       for (int motorspeed = stir_speed; motorspeed >= 0; motorspeed--) {
    analogWrite(44, motorspeed);  //Ramp down
     Serial.print("motorspeed = ");
    Serial.println(motorspeed);
    delay(4);
  } 
    Serial.print("motorspeed = MIN ");
  digitalWrite(23, HIGH);// Set Brake stir Motor
   Serial.print("Brake On ");
    }
  } 
 
  delay(1000);
}

Here is what I have that does not work.

Does it do nothing, or not just not what you want ?

Every time the stir() function is called the stir speed will return to zero then ramp up. Is that what you want ? Do you want the program to stay in the stir() function for 10 seconds or for it to check each time that it is called whether it should ramp up the speed or whether it is time to ramp down the stir speed if it is running at full speed and has been for 10 seconds ?

What does it do? What would you like it to do? What serial output do you get?

Since you're calling it in loop, I assume you're expecting it to spool up for a 10 second stir and then wind down and stop and then do it all again repeatedly. If that's the case but it only works once, the likely cause is that you never set you second variable back to zero. After the wind down would be a good place.