Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A4988, SyncDriver, not turning CCW #53

Closed
hartogs opened this issue Apr 6, 2018 · 11 comments
Closed

A4988, SyncDriver, not turning CCW #53

hartogs opened this issue Apr 6, 2018 · 11 comments
Labels
Milestone

Comments

@hartogs
Copy link

hartogs commented Apr 6, 2018

Hi,

I am facing the issue that the stepmotor is not turning CCW, when using negative values in SyncDriver.controller.rotate/move. When measuring the DIR pin of the motor, the pin is not set to 0V.

#include <Arduino.h>
#include <Ethernet.h>
#include <ArtNode.h>
#include "SyncDriver.h"
#include "A4988.h"

#define VERSION_HI 0
#define VERSION_LO 1

#define MOTOR_STEPS 200
#define MICROSTEPS 16

#define ENABLE 8

// X motor
#define DIR_X 5
#define STEP_X 2
#define RPM_X 50
#define MOTOR_ACCEL_X 400
#define MOTOR_DECEL_X 400

// Y motor
#define DIR_Y 6
#define STEP_Y 3
#define RPM_Y 70
#define MOTOR_ACCEL_Y 300
#define MOTOR_DECEL_Y 300

#define MS1 10
#define MS2 11
#define MS3 12
A4988 stepperX(MOTOR_STEPS, DIR_X, STEP_X, ENABLE, MS1, MS2, MS3);
A4988 stepperY(MOTOR_STEPS, DIR_Y, STEP_Y, ENABLE, MS1, MS2, MS3);
SyncDriver controller(stepperX, stepperY);

void setup() {
    stepperX.begin(RPM_X, MICROSTEPS);
    stepperX.enable();

    stepperY.begin(RPM_Y, MICROSTEPS);
    stepperY.enable();

    stepperX.setSpeedProfile(stepperX.LINEAR_SPEED, MOTOR_ACCEL_X, MOTOR_DECEL_X);
    stepperY.setSpeedProfile(stepperY.LINEAR_SPEED, MOTOR_ACCEL_Y, MOTOR_DECEL_Y);
}

void loop() {
  controller.rotate(10, 30);
  delay(500);
  controller.rotate(-5, -15);
  delay(500);
}

Anyone who also faced the same issue?

Regards,

Sybren

@MarcoBlulab
Copy link

same issue for me
Marco

@laurb9
Copy link
Owner

laurb9 commented Jun 4, 2018

I see that you are using SyncDriver. I would suggest to try and isolate the problem by running just one of the steppers individually:

stepperX.rotate(360); stepperX.rotate(-360)

The first thing that came to mind though, is a possible wiring issue. If there is a strong pull-up (or short) somewhere, the pin won't be able to go down to 0V.

This is easy to check if you either run without wiring connected or if you try a simple sketch to set the pin to LOW and see what happens.

@laurb9
Copy link
Owner

laurb9 commented Jun 4, 2018

So after some more testing, it looks like SyncDriver has an issue when using acceleration; I do not recommend using it in combination with LINEAR_SPEED. The estimation of time to complete the move is most likely broken because of the integer math. I plan to address it after #40

For now you can either use MultiDriver (which will result in one motor finishing sooner than the other, but you can use acceleration) or SyncDriver with CONSTANT_SPEED mode (which will sync the motors but won't work with high-load or high speeds).

@laurb9 laurb9 added the bug label Jun 4, 2018
@POPOBE97
Copy link

POPOBE97 commented Oct 1, 2018

I think I found the problem. In the BasicStepperDriver.cpp, line 114.

void BasicStepperDriver::startMove(long steps){

    long speed;
    if (steps_remaining){
        alterMove(steps);
    } else {
        dir_state = (steps >= 0) ? HIGH : LOW;
        last_action_end = 0;
        steps_remaining = abs(steps);
        ...
    }
}

So when you pass a native steps to the function and the steps_remaining equals to 0, you first store the absolute value of steps into steps_remaining.
And later on in the next loop, the steps is still negative but steps_remaining is not equal to 0 in most of the circumstances. So it calls alterMove.

void BasicStepperDriver::alterMove(long steps){
    switch (getCurrentState()){
    case ACCELERATING: // this also works but will keep the original speed target
    case CRUISING:
        if ( steps >= 0 ){
            steps_remaining += steps;
        } else {
            steps_remaining = max(steps_to_brake, steps_remaining+steps);
        };
        break;
        ...
    }
}

Here you added steps_remaining(positive) directly to steps(negative) regardless of whether they share the same direction or not. Which caused the problem.

void BasicStepperDriver::alterMove(long steps){
    switch (getCurrentState()){
    case ACCELERATING: // this also works but will keep the original speed target
    case CRUISING:
        if ((steps >= 0 && dir_state == HIGH) || (steps < 0 && dir_state == LOW)){
            steps_remaining += abs(steps);
        } else {
            steps_remaining = max(steps_to_brake, steps_remaining+steps);
        };
        ...
    }
}

Changing code like this did the trick, but I am not sure if it will cause any other problems. So far I tested on the DRV8825 and it works fine.
If any of you have other platforms and could do the test for us, it will be great.

POPOBE97 pushed a commit to POPOBE97/StepperDriver that referenced this issue Oct 1, 2018
POPOBE97 pushed a commit to POPOBE97/StepperDriver that referenced this issue Oct 1, 2018
POPOBE97 pushed a commit to POPOBE97/StepperDriver that referenced this issue Oct 1, 2018
@laurb9
Copy link
Owner

laurb9 commented Oct 2, 2018

Nice find. I think you actually found a new issue (the initial move works fine but altering course has the bug).

I think alterMove() should only accept changes in the direction that it's going anyway. Thanks for the PR, I'll take a look at it later.

@laurb9
Copy link
Owner

laurb9 commented Oct 2, 2018

The fix may have to be more subtle: alterMove() was intended to add/remove steps in the current direction. So if you original move is -500 and you call alterMove(100), it would be like having called move(-600).

This is not what should happen when you call move(100) - I think the expectation is that the value gets combined with the original so in effect you would have move(-400). So in that case passing the same steps value from move to alterMove is indeed incorrect...

@POPOBE97
Copy link

POPOBE97 commented Oct 2, 2018

Oh so you didn't design alterMove() for being able to change the direction in the middle of one operation?

@POPOBE97
Copy link

POPOBE97 commented Oct 2, 2018

And also there is another issue in SyncDriver. It called startMove twice in one action, which doubled the movement. I commited the change along with the fix to issue #53.

@anuragkr16
Copy link

I was also facing the same issue with the sync driver. After applying the fix @laurb9 #53 by @POPOBE97 , I get to move the motors with acceleration .
Now what i found that both the motors are not in sync in the operation that is they are making a hockey stick movement rather than a straight line path.
Here is the code I am uploading to my mega and using it with rams 14 and drv8825

#include <Arduino.h>
#include "DRV8825.h"
#include "MultiDriver.h"
#include "SyncDriver.h"
// Motor steps per revolution. Most steppers are 200 steps or 1.8 degrees/step
#define MOTOR_STEPS 200
// Target RPM for X axis motor
#define MOTOR_X_RPM 180
// Target RPM for Y axis motor
#define MOTOR_Y_RPM 180

// Acceleration and deceleration values are always in FULL steps / s^2
#define X_MOTOR_ACCEL 2000//2000 works perfectly 2500 goes slow 500 also goes slow ....
#define Y_MOTOR_ACCEL 2000//2000 works perfectly 2500 goes slow 500 also goes slow ....
#define Z_MOTOR_ACCEL 2000//2000 works perfectly 2500 goes slow 500 also goes slow ....

#define X_MOTOR_DECEL 2000//no idea what happening in backend will go deep & update later
#define Y_MOTOR_DECEL 2000//no idea what happening in backend will go deep & update later
#define Z_MOTOR_DECEL 2000//no idea what happening in backend will go deep & update later


// X motor
#define DIR_X 55
#define STEP_X 54
#define EN_X 38

// Y motor
#define DIR_Y 61
#define STEP_Y 60
#define EN_Y 56

// If microstepping is set externally, make sure this matches the selected mode
// 1=full step, 2=half step etc.
#define MICROSTEPS 32

// 2-wire basic config, microstepping is hardwired on the driver
// Other drivers can be mixed and matched but must be configured individually
DRV8825 stepperX(MOTOR_STEPS, DIR_X, STEP_X, EN_X);
DRV8825 stepperY(MOTOR_STEPS, DIR_Y, STEP_Y, EN_Y);

// Pick one of the two controllers below
// each motor moves independently, trajectory is a hockey stick
//MultiDriver controller(stepperX, stepperY);
// OR
// synchronized move, trajectory is a straight line
SyncDriver controller(stepperX, stepperY);

void setup() {
  /*
     Set target motors RPM.
  */
  stepperX.begin(MOTOR_X_RPM, MICROSTEPS);
  stepperY.begin(MOTOR_Y_RPM, MICROSTEPS);
  stepperX.setSpeedProfile(stepperX.LINEAR_SPEED, X_MOTOR_ACCEL, X_MOTOR_DECEL);
  stepperY.setSpeedProfile(stepperY.LINEAR_SPEED, Y_MOTOR_ACCEL, Y_MOTOR_DECEL);
}

void loop()
{
  stepperX.enable();// stepper enable and disabled to reduce the heat generate in drv8825 driver during continuous operation
  stepperY.enable();
  controller.rotate(360, 360);
  stepperX.disable();
  stepperY.disable();
  delay(5000);
  stepperX.enable();
  stepperY.enable();
  controller.rotate(-360, -360);
  stepperX.disable();
  stepperY.disable();
  delay(5000);
  stepperX.enable();
  stepperY.enable();
  controller.rotate(720, 360 / 2);
  stepperX.disable();
  stepperY.disable();
  delay(5000);
  stepperX.enable();
  stepperY.enable();
  controller.rotate(-720, -360 / 2);
  stepperX.disable();
  stepperY.disable();
  delay(5000);
}

Here when this code controller.rotate(360, 360); is executed both motors start and stop at the same time but when this line is executed controller.rotate(720, 360 / 2); ,motor y stopped after both motor travel fixed distance and then the rest distance motor x moves individually.

One more thing i noticed that when i turn off acceleration profile by executing this code

 stepperX.setSpeedProfile(stepperX.LINEAR_SPEED, 0, 0);
  stepperY.setSpeedProfile(stepperY.LINEAR_SPEED, 0, 0);

(not included in above code)
still the motor runs slow in SyncDriver than the individual motor control using stepperY.rotate(360);

please look into it.
I have started using dv8825 recently so may be there might be something I am missing currently.
Please help me on that or correct me if i am missing something.

@laurb9
Copy link
Owner

laurb9 commented Jan 27, 2019

The acceleration profile is disabled by simply not calling setSpeedProfile() at all (because it is the default) or explicitly by calling setSpeedProfile(stepperX.CONSTANT_SPEED). The command you used does set acceleration to 0 which makes sense but not to the code, as that would cause some "division by 0" errors with unexpected results where the code expected to see some acceleration :)

I suspect constant speed won't work well with #65 because getTimeForMove() does not call startMove() when in that mode.

laurb9 added a commit that referenced this issue Jan 27, 2019
As @POPOBE97 observed, SyncDriver calls `startMove()` twice and the second call is interpreted as a request to alter course which breaks both positive and negative movements in different ways.
@laurb9 laurb9 added this to the 1.1.4 milestone Jan 27, 2019
@laurb9
Copy link
Owner

laurb9 commented Jan 28, 2019

I think the revert above should resolve this issue.

@laurb9 laurb9 closed this as completed Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants