motor position control

hi I'm trying to achieve a rotational position control on my DC motor by reading the encoder and using 3 analog inputs to calculate pwm and degree of rotation. The code runs but the positioning is far from being precise. What am I doing wrong here? Would appreciate any and all feedback.

#include <TimerThree.h>
#include <TimerOne.h>
#include <Math.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27, 16, 2);

// Constants
  float Max_Sweep_Angle = 30;          // claw sweep angle that corrspondts to max air volume to be delivered
  float Hold_time = 1000;              // hold time (in ms) between inhale and exhale to maintain air pressure in lungs before exhaling
  float Pinion_Gear_Teeth = 30;        // number of teeth on gear directly attached to motor shaft (pinion)
  float Claw_Gear_Teeth = 48;          // number of teeth on gear driven by the pinion
  float Max_Tidal_Volume = 800;        // maximum volume of air to be delivered to patient

//Variables
  float BPM = A0;               // BPM potentiometer
  float Tidal_Volume = A1;      // Tidal_Volume potentiometer
  float IE = A2;                // inhale/exhale ratio
  float BPM_Value = 0;          // variable to store the value coming from the sensor
  float Tidal_Volume_Value = 0;
  float IE_Value = 0;
  int Pressure_Value = 0;
  int Opto_Interrupt_Value = 0;

  int PWM2;
  int PWM6;

// Inputs
  int motor_home = 6;
  int motor_inhale = 2;
  int motor_dir = 4;
  int opto_interrupt = 7;
  int pressure_sensor = 8;
  int button_1 = 50;
  int button_2 = 48;
  int ENCODER_A = 18;                                // pin 18 is an interrupt pin and is more reliable physical connection on the board
  int ENCODER_B = 12;
  int rpmcount = 0;
  unsigned long enc_inhale_counter = 0;
  unsigned long enc_exhale_counter = 0;

 void setup() {
  // declare potentiometers as inputs
  Serial.begin(9600);
  pinMode(BPM, INPUT);
  pinMode(Tidal_Volume, INPUT);
  pinMode(IE, INPUT);
  // declare OUTPUT PINS FOR INHALE AND EXHALE PWMs:
  pinMode(motor_inhale, OUTPUT);
  pinMode(motor_dir, OUTPUT);
  // declare buttons as input
  pinMode(motor_home, INPUT);
  pinMode(opto_interrupt, INPUT);
  pinMode(pressure_sensor,INPUT);
  pinMode(ENCODER_A, INPUT);                              // may need to change these to input_pulup
  pinMode(ENCODER_B, INPUT);
// LCD setup
  lcd.init();                      // initialize the lcd 
  lcd.init();
  // Print a message to the LCD.
  lcd.backlight();
  }

 void loop() {

  //pinMode(opto_interrupt, INPUT_PULLUP);
  
  // read the value from the pots:
    BPM_Value = analogRead(BPM);
    Tidal_Volume_Value = analogRead(Tidal_Volume);
    IE_Value = analogRead(IE);

    Pressure_Value = digitalRead(pressure_sensor);
    Opto_Interrupt_Value = digitalRead(opto_interrupt);
// map pot readings to actual values defined in the clinical specs
  
    float Actual_BPM_Value = map(BPM_Value, 0, 1023, 6, 40);                              // breaths per minute 6 - 40  (breath period 60/BPM)
    float Actual_Tidal_Volume_Value = map(Tidal_Volume_Value, 0, 1023, 200, 800);         // Tidal volume 200 ml - 800 ml. In terms of encoder pulses: (800 ml/30 degrees)*(3.4 degrees/1 pulse) = 90.7 ml/pulse
    float Actual_IE_Value = map(IE_Value, 0, 1023, 1, 4);                                 // I/E  inhale time/exhale time ratio 1/1  - 1/4 (given as 1- 4)
float Breath_period = (60UL/Actual_BPM_Value);
    float t_inhale = (Breath_period)/(1+Actual_IE_Value);
    float t_exhale = Breath_period - t_inhale;  
    float arm_speed = (radians(Max_Sweep_Angle)/t_inhale)*(Actual_Tidal_Volume_Value/Max_Tidal_Volume);
    float pinion_speed = arm_speed*(Claw_Gear_Teeth/Pinion_Gear_Teeth);
    float RPM = (pinion_speed*60UL)/(2*PI);
    float pos_duty_cycle = (RPM/26)*100;                  // 340 is max NO LOAD RPM of AndyMark NeveRest planetary gear motor which is used here (assumption: 12V - 340 RPM)
    float volume_pulses = Actual_Tidal_Volume_Value/91;
// Convert Actual_BPM_Value, Actual_Tidal Volume_Value, Actual_IE_Value from float to int for LCD
    int LCD_BPM = int(Actual_BPM_Value);
    int LCD_Tidal_Volume = int(Actual_Tidal_Volume_Value);
    int LCD_IE = int(Actual_IE_Value);

    // Display BPM, Tidal Volume, IE to LCD
    lcd.setCursor(0,0);
    lcd.print("BPM ");
    lcd.print(LCD_BPM);
    lcd.setCursor(7,0);
    lcd.print(" Vol ");
    lcd.print(LCD_Tidal_Volume);
    lcd.setCursor(6,1);
    lcd.print("IE ");
    lcd.print(LCD_IE);
if(Pressure_Value == HIGH){

    // Initiate inhale

    

   while(int(enc_inhale_counter) <= int(volume_pulses)){   
    digitalWrite(motor_dir, HIGH);
    analogWrite(motor_inhale, int(pos_duty_cycle));
    Serial.println("duty cycle for inhale = \t");
    Serial.println(pos_duty_cycle);
    if(digitalRead(ENCODER_A) == HIGH){                         //reading both encoder signals results in inaccurate count: && digitalRead(ENCODER_B) == LOW 
      enc_inhale_counter++;
      }
    Serial.println("Pulse count in inhale loop = \t");
    Serial.println(enc_inhale_counter);    
    }
    enc_inhale_counter = 0;
    
    digitalWrite(motor_inhale, LOW);
    digitalWrite(motor_dir, LOW);
    delay(Hold_time);

   while(int(enc_exhale_counter) <= int(volume_pulses)){
    Serial.println("in exhale");
    digitalWrite(motor_dir, LOW);                                                    // 7-30-2020 enters this loop but motor doesn't go back 
    analogWrite(motor_inhale, int(pos_duty_cycle));                                                   // check in actual setup and verify if this is an appropriate duty cycle
    if(digitalRead(ENCODER_A) == LOW){
      enc_exhale_counter++;
      }
    Serial.println("Pulse count in exhale loop = \t");
    Serial.println(enc_exhale_counter); 
      }
      enc_exhale_counter = 0;
      digitalWrite(motor_inhale, LOW);
      digitalWrite(motor_dir, HIGH);
 }
 
 }

here's the serial readout; as you can see the counter is inconsistent

10:00:02.590 -> `f~`f⸮⸮f⸮⸮⸮fx⸮⸮⸮⸮⸮⸮⸮⸮⸮fduty cycle for inhale = 	
10:00:02.590 -> 24.62
10:00:02.590 -> Pulse count in inhale loop = 	
10:00:02.627 -> 0
10:00:02.627 -> duty cycle for inhale = 	
10:00:02.661 -> 24.62
10:00:02.661 -> Pulse count in inhale loop = 	
10:00:02.697 -> 1
10:00:02.697 -> duty cycle for inhale = 	
10:00:02.730 -> 24.62
10:00:02.730 -> Pulse count in inhale loop = 	
10:00:02.766 -> 1
10:00:02.766 -> duty cycle for inhale = 	
10:00:02.802 -> 24.62
10:00:02.802 -> Pulse count in inhale loop = 	
10:00:02.836 -> 2
10:00:02.836 -> duty cycle for inhale = 	
10:00:02.872 -> 24.62
10:00:02.872 -> Pulse count in inhale loop = 	
10:00:02.908 -> 3
10:00:02.908 -> duty cycle for inhale = 	
10:00:02.944 -> 24.62
10:00:02.944 -> Pulse count in inhale loop = 	
10:00:02.980 -> 3
10:00:02.980 -> duty cycle for inhale = 	
10:00:03.016 -> 24.62
10:00:03.052 -> Pulse count in inhale loop = 	
10:00:03.088 -> 3
10:00:03.088 -> duty cycle for inhale = 	
10:00:03.088 -> 24.62
10:00:03.124 -> Pulse count in inhale loop = 	
10:00:03.124 -> 4
10:00:03.124 -> duty cycle for inhale = 	
10:00:03.160 -> 24.62
10:00:03.160 -> Pulse count in inhale loop = 	
10:00:03.196 -> 4
10:00:03.196 -> duty cycle for inhale = 	
10:00:03.233 -> 24.62
10:00:03.233 -> Pulse count in inhale loop = 	
10:00:03.269 -> 5
10:00:04.225 -> in exhale
10:00:04.225 -> Pulse count in exhale loop = 	
10:00:04.261 -> 1
10:00:04.261 -> in exhale
10:00:04.261 -> Pulse count in exhale loop = 	
10:00:04.297 -> 2
10:00:04.297 -> in exhale
10:00:04.334 -> Pulse count in exhale loop = 	
10:00:04.368 -> 2
10:00:04.368 -> in exhale
10:00:04.368 -> Pulse count in exhale loop = 	
10:00:04.405 -> 3
10:00:04.405 -> in exhale
10:00:04.441 -> Pulse count in exhale loop = 	
10:00:04.477 -> 3
10:00:04.477 -> in exhale
10:00:04.477 -> Pulse count in exhale loop = 	
10:00:04.512 -> 4
10:00:04.512 -> in exhale
10:00:04.512 -> Pulse count in exhale loop = 	
10:00:04.548 -> 5

Comment out ALL the LCD calls and see if tat helps.

Paul

Hi Paul,

Just tried; did not work.

12:10:55.993 -> inhale pre-while enc counter  = 	
12:10:56.032 -> 0
12:10:56.032 -> duty cycle for inhale = 	
12:10:56.066 -> 24.62
12:10:56.066 -> Pulse count in inhale loop = 	
12:10:56.102 -> 1
12:10:56.102 -> duty cycle for inhale = 	
12:10:56.138 -> 24.62
12:10:56.138 -> Pulse count in inhale loop = 	
12:10:56.174 -> 1
12:10:56.174 -> duty cycle for inhale = 	
12:10:56.210 -> 24.62
12:10:56.210 -> Pulse count in inhale loop = 	
12:10:56.210 -> 1
12:10:56.210 -> duty cycle for inhale = 	
12:10:56.246 -> 24.62
12:10:56.246 -> Pulse count in inhale loop = 	
12:10:56.283 -> 2
12:10:56.283 -> duty cycle for inhale = 	
12:10:56.318 -> 24.62
12:10:56.318 -> Pulse count in inhale loop = 	
12:10:56.354 -> 2
12:10:56.354 -> duty cycle for inhale = 	
12:10:56.390 -> 24.62
12:10:56.426 -> Pulse count in inhale loop = 	
12:10:56.462 -> 3
12:10:56.462 -> duty cycle for inhale = 	
12:10:56.462 -> 24.62
12:10:56.498 -> Pulse count in inhale loop = 	
12:10:56.538 -> 4
12:10:56.538 -> duty cycle for inhale = 	
12:10:56.573 -> 24.62
12:10:56.573 -> Pulse count in inhale loop = 	
12:10:56.610 -> 4
12:10:56.610 -> duty cycle for inhale = 	
12:10:56.644 -> 24.62
12:10:56.644 -> Pulse count in inhale loop = 	
12:10:56.681 -> 4
12:10:56.681 -> duty cycle for inhale = 	
12:10:56.681 -> 24.62
12:10:56.681 -> Pulse count in inhale loop = 	
12:10:56.717 -> 5
12:10:57.677 -> exhale pre-while enc counter = 	
12:10:57.712 -> 0
12:10:57.712 -> in exhale
12:10:57.712 -> Pulse count in exhale loop = 	
12:10:57.748 -> 1
12:10:57.748 -> in exhale
12:10:57.748 -> Pulse count in exhale loop = 	
12:10:57.784 -> 2
12:10:57.820 -> in exhale
12:10:57.820 -> Pulse count in exhale loop = 	
12:10:57.856 -> 3
12:10:57.856 -> in exhale
12:10:57.856 -> Pulse count in exhale loop = 	
12:10:57.891 -> 4
12:10:57.891 -> in exhale
12:10:57.928 -> Pulse count in exhale loop = 	
12:10:57.928 -> 5

I am noticing that the problem is with the first while loop. The encoder pulse count somehow does not get properly updated.

The second while loop seems to work consistently.

when you read an encoder, you generally need to immediately detect the change in state of one of the inputs and look at the other input to determine the direction of the change. delays such as those LCD calls get in the way of detecting the state change immediately. i use interrupts just when turning a rotary encoder by hand.

it's also not clear what you motor is driving (the encoder?). when I did this on a model railroad turntable i needed a PID algorithm to anticipate reaching the target position and start reducing speed before reaching it.

float BPM = A0;               // BPM potentiometer
float Tidal_Volume = A1;      // Tidal_Volume potentiometer
float IE = A2;                // inhale/exhale ratio

?!? Why are you storing pin numbers in 'float' variables ?!?

Change those to 'const byte'.

Also, put 'const' in front of your other named constants.

The 'map()' function only works on integers so your IE values will be 1, 2, 3, or 4.

      if (digitalRead(ENCODER_A) == HIGH)
      {
        enc_inhale_counter++;
      }

That is NOT how you measure motion with an encoder. If the motor is stopped with channel A HIGH you would be counting a pulse each time through the loop even though ther was no motion! You have to look for a CHANGE in the state of the pin to detect motion. AND you have to look at the state of the OTHER channel to tell which direction the motion is. Perhaps you should use an Encoder library.

there is a ready to buy solution for DC-motors which does all the positioning details. Not cheap 108 Euros) but very precise resolution 0,01 degrees)

SuperModified V3.0 for DC motors

You still have to program communication between this position-controller and your microcontroller.

Standard mechanical-switch encoders have some problems especcially if you want the motor to change between clockwise and counterclockwise rotation after some seconds or even shorter periods.
Even the standard-encoder-libraries fail to count correctly with those mechanical switch-encoders.

Positioning a DC-motor is no self-purpose. Maybe your project can work based on a servo-motor which does the positioning-details for you. JMC has servo-motors of different sizes.

Again depending on the purpose of the positioning a steppermotor might be suitable too.
If you describe what the positioning is used for at the end better solutions canbe suggested.

best regards Stefan

@gcjr thanks for the feedback: I tried checking both encoder signals to determine a change in state before counting and took out all lcd calls as well:

 while(int(enc_inhale_counter) <= int(volume_pulses)){   
    digitalWrite(motor_dir, HIGH);
    analogWrite(motor_inhale, int(pos_duty_cycle));
    
    current_ENCODER_A_STATE = digitalRead(ENCODER_A);
       
    Serial.println("duty cycle for inhale = \t");
    Serial.println(pos_duty_cycle);
    if(current_ENCODER_A_STATE != previous_ENCODER_A_STATE){                         //reading both encoder signals results in inaccurate count: && digitalRead(ENCODER_B) == LOW 
      if(digitalRead(ENCODER_B) != current_ENCODER_A_STATE){
      enc_inhale_counter++;
      previous_ENCODER_A_STATE = current_ENCODER_A_STATE;
      }
      }
    Serial.println("Pulse count in inhale loop = \t");
    Serial.println(enc_inhale_counter);    
    }
    //enc_inhale_counter = 0;
    
    digitalWrite(motor_inhale, LOW);
    digitalWrite(motor_dir, LOW);
    delay(Hold_time);

    Serial.println("exhale pre-while enc counter = \t");
    Serial.println(enc_exhale_counter);

   while(int(enc_exhale_counter) <= int(volume_pulses)){
    Serial.println("in exhale");
    digitalWrite(motor_dir, LOW);                                                    // 7-30-2020 enters this loop but motor doesn't go back 
    analogWrite(motor_inhale, int(pos_duty_cycle));                                                   // check in actual setup and verify if this is an appropriate duty cycle
    
    current_ENCODER_B_STATE = digitalRead(ENCODER_B);
    
    if(current_ENCODER_B_STATE != previous_ENCODER_B_STATE){
      if(digitalRead(ENCODER_A) != current_ENCODER_B_STATE){
      enc_exhale_counter++;
      previous_ENCODER_B_STATE = current_ENCODER_B_STATE;
      }
      }
    Serial.println("Pulse count in exhale loop = \t");
    Serial.println(enc_exhale_counter); 
      }
      enc_exhale_counter = 0;
      digitalWrite(motor_inhale, LOW);
      digitalWrite(motor_dir, HIGH);

it did not help. Looks like the problem is inherent in the way I try to achieve my desired position; I drive the motor which then enables the encoder on the back and use the pulse count to stop and reverse the drive shaft. Will be looking into PID implementation. Thanks @Paul_KD7HB, @johnwasser, @StefanL38