[Feature] Make drivebase turns counterclockwise for positive turn_rate when reversing

[JDwyer009]

Issue with drive_base.drive() function While working on the really easy to follow "Hub to Hub" example (which seems to be really robust and reliable :)), I experienced what I suspect is a bug in the drive_base.drive() function.

Context When using the above function, steering left and right while driving Forward works as expected. However, when reversing the steering seems to be reversed.

The issue was easily fixed by inverting the steering variable (multiply by -1) if the speed is negative, but I just thought I'd post it here so the original function can be corrected if it is incorrect.

        if nSpeed < 0:
            nSteering *= -1

in greater context

while True:
    # Receive tilt data.
    data = hub.ble.observe(1)

    if data is not None:
        # If we received data, start driving.
        nSteering, nSpeed, nClaw = data
        
        # Fix a seeming anomaly in drive_base() causing steering to be inverted when reversing 
        if nSpeed < 0:
            nSteering *= -1
        
        drive_base.drive(speed=nSpeed * 20, turn_rate=nSteering * 3)
        claw_motor.run(nClaw)

[laurensvalk]

Thanks @JDwyer009! Can you elaborate on the direction you expected, versus what you are seeing?

[JDwyer009]

When driving fwd, if you steer left on the controller, the car/robot turns left (or more correctly arcs to the left - As it should) If you continue to steer left, but put it into reverse, I would expect it to reverse along the same left arc. But instead, it reverses along the right arc or to the right. This is not how a car steers in reverse and seems counter-intuitive.

[laurensvalk]

Thanks for the follow up! I agree with your intuition.

We'll have to review what has been the standard in previous releases to see if this has changed, or just that nobody noticed this before :)

[JDwyer009]

Thanks Laurens. Much appreciated.

[laurensvalk]

We'll have to review what has been the standard in previous releases to see if this has changed, or just that nobody noticed this before :)

It is looking like it has been this way since at least EV3 version 2.0. While I agree with @JDwyer009 's interpretation of what should be intuitively correct, we probably shouldn't change it at this point.

[JDwyer009]

Hi Laurens,

I'd politely like to disagree with leaving this bug in based on existing legacy programs. Despite this issue being around since the EV3, my suggestion would still be to correct the issue. Particularly if you are in the process of releasing Pybricks block code. While it took me a little while to identify the issue, (going forward, I and your existing python coders, can quickly identify and fix the issue) this will likely confuse and frustrate your new and younger coders (as they increasingly come on board with Pybricks block code). It is such an easy problem to fix, it seems crazy not to do so.

A very common project (with visual Pybricks) moving forward will be to experiment using one hub to Bluetooth control another vehicle which relies on the drive_base.drive() function. It seems important to ensure it works like a vehicle in the real world. Please reconsider.

[laurensvalk]

It's a tricky one - this isn't a bug, just a different definition, so changing it after several years is going to change people's programs.

We're usually quite strict about that. But it looks like we have @dlech 's upvote already, so I guess that makes me the swing vote, so I'll reconsider it before the release :smile:

[JDwyer009]

Thank you very much for reconsidering this. PS your existing user base will be more than smart enough to fix their old programs if necessary. ;) AND your new user base will appreciate the more intuitive functionality. PS @dlech Thanks David for chiming in. Fingers crossed!

[laurensvalk]

PS your existing user base will be more than smart enough to fix their old programs if necessary. ;)

With the official apps changing completely every couple of years (EV3-G --> EV3 classroom, SPIKE2 ---> SPIKE3, MINDSTORMS ---> :candle: , we try to remain a constant source of reliability, but exceptions can be made :smile:

---

For what it's worth, the current definition is technically consistent --- positive turn rates are always clockwise, even in reverse.

We just need to expand the definition to say that a positive turn rate will be counterclockwise when you reverse.

We'll also want to double check the final result with the newer curve method so that it is technically and intuitively consistent with drive. See also https://github.com/orgs/pybricks/discussions/1240

---

Would you be interested to help with parts of this change? One useful chore would be to make a useful test program that will verify all the directions for various permutations. We can run it on a real robot and in our automated tests so we will fix it once and for all.

It could be something like this

#imports

#set up the left/right motors, drivebase, etc. 

# and then the tests:

robot.drive(500, 0)
wait(1000) # Give it some time to get going
assert left.speed() > 100, "When driving straight forward, left speed should be positive"
assert right.speed() > 100, "When driving straight forward, right speed should be positive"

robot.drive(-500, 0)
# and so on, making sure to test useful combinations of forward, reverse, large turns, tight turns, etc.

---

If you make such a test, feel free to just post your script in this issue. Or if you want to, create a pull request to add it here: https://github.com/pybricks/pybricks-micropython/tree/master/tests/virtualhub/motor

We could get it merged in before we make the change to the drive method. This means the test should reflect the old behavior for now.

Then we update the drive method in the firmware and update the tests to match, so it's super clear what got changed.

[JDwyer009]

Thanks Laurens,

Yes, I'm happy to help as much as possible. I'll put a test program together.

[JDwyer009]

@laurensvalk Here is a program to test and explain most of the functionality of the DriveBase() class.

Is this along the lines of what you were asking?

from pybricks.pupdevices import Motor
from pybricks.parameters import Direction, Port
from pybricks.robotics import DriveBase
from pybricks.tools import wait

# Initialize the drive base. 
motorL = Motor(Port.C, Direction.COUNTERCLOCKWISE) 
motorR = Motor(Port.D) 
drive_base = DriveBase(motorL, motorR, wheel_diameter=56, axle_track=97) 
 
# User's can override the default speeds with .setting()  (defaults are 40% of it's max speed) 
# .settings(straight_speed, straight_acceleration, turn_rate, turn_acceleration)

# Demo 1 - Using .straight() to move Fwd & Rev in mm
# .straight(distance, then=Stop.HOLD, wait=True)
drive_base.straight(200)
wait(250)
drive_base.straight(-200)
wait(250)

# Demo 2 - Using .turn() to 'Point Turn' by angle (+ve = CW, -ve = CCW)
# .turn(angle, then=Stop.HOLD, wait=True)
drive_base.turn(90)
wait(500)
drive_base.turn(180)
wait(500)
drive_base.turn(-270)
wait(500)
drive_base.turn(1080)
wait(500)
drive_base.turn(-1440)

# Demo 3 - Using .curve() drives the 'center point' between the wheels a full circle (360 degrees or part thereof) around a circle of 12cm radius
# .curve(radius, angle, then=Stop.HOLD, wait=True)
drive_base.curve(120, 360)   # Drives Clockwise Fwd
drive_base.curve(-120, 360)   # Drives CounterClockwise in Rev
drive_base.curve(120, -360)  # Drives Counterclockwise Fwd
drive_base.curve(-120, -360)  # Drives Clockwise in Rev

# Demo 4 - Using .drive() to control the robot in various ways
# .drive(speed, turn_rate)
drive_base.drive(200, 0)   # Drive Fwd
wait(1000)
drive_base.stop()
wait(250)

drive_base.drive(-200, 0)   # Drive Rev
wait(1000)
drive_base.stop()
wait(250)

drive_base.drive(200, 90)   # Drives Clockwise Fwd
wait(4000)   # 4 seconds x 90 deg/s = full circle
drive_base.stop()
wait(250)

drive_base.drive(-200, 90)   # Drives Clockwise in Rev
wait(4000)
drive_base.stop()
wait(250)

drive_base.drive(200, -90)   # Drives Counter-CW Fwd
wait(4000)
drive_base.stop()
wait(250)

drive_base.drive(-200, -90)   # Drives Counter-CW in Rev
wait(4000)
drive_base.stop()
wait(250) 

[dlech]

Having seen the discussion comparing drive() and curve() now, I think I have a better perspective. I think maybe perhaps rather than changing the drive() method, it could be more useful to introduce a new "Drive forever" method that has speed and radius as parameters and the semantics of radius are the same as in curve().

[JDwyer009]

That works for me. :)

[laurensvalk]

Thanks for the test program. Can you have a look at the example snippet above with the assert statements?

It's very useful if a program can test itself so we don't have to visually confirm it (or get confused about what is clockwise :smile:)

[laurensvalk]

I have gone ahead and modified this in our drivers:

diff --git a/lib/pbio/src/drivebase.c b/lib/pbio/src/drivebase.c
index ae1d94a17..2e29052f0 100644
--- a/lib/pbio/src/drivebase.c
+++ b/lib/pbio/src/drivebase.c
@@ -641,6 +641,12 @@ static pbio_error_t pbio_drivebase_drive_time_common(pbio_drivebase_t *db, int32
         return err;
     }
 
+    // When reversing, also reverse steering. This way, driving backwards without
+    // changing the steering sign means driving along the same circle, backwards.
+    if (drive_speed < 0) {
+        turn_speed  = -turn_speed;
+    }
+
     err = pbio_control_start_timed_control(&db->control_heading, time_now, &state_heading, duration, turn_speed, on_completion);
     if (err != PBIO_SUCCESS) {
         return err;

So instead of making clockwise/counterclockwise consistent as we used to, we now flip things as needed to instead make the "circle along which you drive" consistent.

So now for both drive and curve, a positive angle means driving along a circle on the robot's right, where the speed/radius may be made negative to reverse on the same circle.

So now for both drive and curve, a negative angle means driving along a circle on the robot's left, where the speed/radius may be made negative to reverse on the same circle.

from pybricks.pupdevices import Motor
from pybricks.parameters import Direction, Port
from pybricks.robotics import DriveBase
from pybricks.tools import wait

# Set up all devices.
left = Motor(Port.A, Direction.COUNTERCLOCKWISE)
right = Motor(Port.B, Direction.CLOCKWISE)
drive_base = DriveBase(left, right, 56, 114)


# both of these do forward along a circle placed to its right
drive_base.curve(100, 90)
drive_base.drive(150, 75)
wait(1000)

# both of these do reverse along a circle placed to its right
drive_base.curve(-100, 90)  # it was already that way for curve
drive_base.drive(-150, 75)   # for drive, it used to be flipped
wait(1000)

# both of these do forward along a circle placed to its left
drive_base.curve(100, -90)
drive_base.drive(150, -75)
wait(1000)

# both of these do reverse along a circle placed to its left
drive_base.curve(-100, -90)
drive_base.drive(-150, -75)
wait(1000)

[laurensvalk]

This introduces another interesting problem. Should we also reverse the reported turn_rate when reporting the robot state?

[dlech]

Should we also reverse the reported turn_rate when reporting the robot state?

~~I would say yes, what you put in should be the same as what you get out (assuming the drive base has reached steady state and the request rate is within the physical limits)~~

EDIT: I changed my mind. I think this inconsistency is another reason to not change the drive() method.

[JDwyer009]

Thanks for the test program. Can you have a look at the example snippet above with the assert statements?

It's very useful if a program can test itself so we don't have to visually confirm it (or get confused about what is clockwise 😄)

Apologies for thew delay in responding. Here is a test program that includes the assert statements. Hopefully it is still useful. At present, based on the beta code Test 15 and Test 17 throw errors as they behave with the old behaviours but test for consistency with the new bahaviours ie getting .drive() to behave consistently with .curve().

@laurensvalk Is this consistent with what you wanted?

from pybricks.pupdevices import Motor
from pybricks.parameters import Direction, Port
from pybricks.robotics import DriveBase
from pybricks.tools import wait

# Initialize the drive base. 
motorL = Motor(Port.C, Direction.COUNTERCLOCKWISE) 
motorR = Motor(Port.D) 
drive_base = DriveBase(motorL, motorR, wheel_diameter=56, axle_track=80) 
 
# User's can override the default speeds with .setting()  (Defaults are 40% of it's max speed) 
# .settings(straight_speed, straight_acceleration, turn_rate, turn_acceleration)

# Demo 1 - Using .straight() to move Fwd & Rev in mm
# .straight(distance, then=Stop.HOLD, wait=True)
drive_base.straight(200, wait=False)
wait(500)
print("Test 1 - Drive Fwd - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "When driving Fwd in straight line, left speed should be +ve"
assert motorR.speed() > 0,  "When driving Fwd in straight line, right speed should be +ve"
while not drive_base.done():
    wait(10)
wait(250)

drive_base.straight(-200, wait=False)
wait(500)
print("Test 2 -Drive Rev - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "When driving Rev in straight line, left speed should be -ve"
assert motorR.speed() < 0,  "When driving Rev in straight line, right speed should be -ve"
while not drive_base.done():
    wait(10)
wait(250)

# Demo 2 - Using .turn() to 'Point Turn' by angle (+ve = CW, -ve = CCW)
# .turn(angle, then=Stop.HOLD, wait=True)
drive_base.turn(90, wait=False)
wait(500)
print("Test 3 - CW Point Turn 90 degrees - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "For CW Point Turn 90 Degrees, left speed should be +ve"
assert motorR.speed() < 0,  "For CW Point Turn 90 Degrees, right speed should be -ve"
while not drive_base.done():
    wait(10)
wait(500)

drive_base.turn(180, wait=False)
wait(500)
print("Test 4 - CW Point Turn 180 degrees - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "For CW Point Turn 180 Degrees, left speed should be +ve"
assert motorR.speed() < 0,  "For CW Point Turn 180 Degrees, right speed should be -ve"
while not drive_base.done():
    wait(10)
wait(500)

drive_base.turn(-270, wait=False)
wait(500)
print("Test 5 - CCW Point Turn 270 degrees - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "For CCW Point Turn 270 Degrees, left speed should be -ve"
assert motorR.speed() > 0,  "For CCW Point Turn 270 Degrees, right speed should be +ve"
while not drive_base.done():
    wait(10)
wait(500)

drive_base.turn(1080, wait=False)
wait(500)
print("Test 6 - CW Point Turn 1080 degrees - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "For CW Point Turn 1080 Degrees, left speed should be +ve"
assert motorR.speed() < 0,  "For CW Point Turn 1080 Degrees, right speed should be -ve"
while not drive_base.done():
    wait(10)
wait(500)

drive_base.turn(-1440, wait=False)
wait(500)
print("Test 7 - CCW Point Turn 1440 degrees - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "For CCW Point Turn 270 Degrees, left speed should be -ve"
assert motorR.speed() > 0,  "For CCW Point Turn 270 Degrees, right speed should be +ve"
while not drive_base.done():
    wait(10)
wait(500)


# Demo 3 - Using .curve() drives the 'center point' between the wheels a full circle (360 degrees or part thereof) around a circle of 12cm radius
# .curve(radius, angle, then=Stop.HOLD, wait=True)
drive_base.curve(120, 360, wait=False)   # Drives Clockwise Fwd
wait(500)
print("Test 8 - CW Curve Fwd (radius 12cm) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "When driving Fwd in CW Curve, left speed should be +ve"
assert motorR.speed() > 0,  "When driving Fwd in CW Curve, right speed should be +ve"
assert motorL.speed() > motorR.speed(), "When driving Fwd in CW Curve, motorL should be greater than motorR"
while not drive_base.done():
    wait(10)
wait(500)

drive_base.curve(-140, 360, wait=False)   # Drives CounterClockwise in Rev
wait(500)
print("Test 9 - CCW Curve Rev (radius 12cm) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "When driving Rev in CCW Curve, left speed should be -ve"
assert motorR.speed() < 0,  "When driving Rev in CCW Curve, right speed should be -ve"
assert motorL.speed() < motorR.speed(), "When driving Rev in CCW Curve, motorL should be less than motorR"
while not drive_base.done():
    wait(10)
wait(500)

drive_base.curve(120, -360, wait=False)  # Drives Counterclockwise Fwd
wait(500)
print("Test 10 - CCW Curve Fwd (radius 12cm) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "When driving Fwd in CCW Curve, left speed should be +ve"
assert motorR.speed() > 0,  "When driving Fwd in CCW Curve, right speed should be +ve"
assert motorL.speed() < motorR.speed(), "When driving Fwd in CCW Curve, motorL should be less than motorR"
while not drive_base.done():
    wait(10)
wait(500)

drive_base.curve(-120, -360, wait=False)  # Drives Clockwise in Rev
wait(500)
print("Test 11 - CW Curve Rev (radius 12cm) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "When driving Rev in CW Curve, left speed should be -ve"
assert motorR.speed() < 0,  "When driving Rev in CW Curve, right speed should be -ve"
assert motorL.speed() > motorR.speed(), "When driving Rev in CW Curve, motorL should be greater than motorR"
while not drive_base.done():
    wait(10)
wait(500)

# Demo 4 - Using .drive() to control the robot in various ways
# .drive(speed, turn_rate)
drive_base.drive(200, 0)   # Drive Fwd
wait(1000)
print("Test 12 - .drive() - Drive Fwd - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "When driving Fwd in straight line, left speed should be +ve"
assert motorR.speed() > 0,  "When driving Fwd in straight line, right speed should be +ve"
wait(500)
drive_base.stop()
wait(250)

drive_base.drive(-200, 0)   # Drive Rev
wait(1000)
print("Test 13 - .drive() - Drive Rev - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "When driving Rev in straight line, left speed should be -ve"
assert motorR.speed() < 0,  "When driving Rev in straight line, right speed should be -ve"
wait(500)
drive_base.stop()
wait(250)

drive_base.drive(200, 90)   # Drives CW Fwd
wait (1000)
print("Test 14 - .drive() - CW Curve Fwd (turning 90/sec) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "When driving CW Curve Fwd, left speed should be +ve"
assert motorR.speed() > 0,  "When driving CW Curve Fwd, right speed should be +ve"
assert motorL.speed() > motorR.speed(), "When driving Fwd in CW Curve, motorL should be greater than motorR"
wait(3000)   # 4 seconds x 90 deg/s = full circle
drive_base.stop()
wait(250)

drive_base.drive(-200, 90)   # In corrected version it should Drive CCW in Rev
wait (1000)
print("Test 15 - .drive() - CCW Curve Rev (turning 90/sec) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "When driving CCW Curve Rev, left speed should be -ve"
assert motorR.speed() < 0,  "When driving CCW Curve Rev, right speed should be -ve"
assert motorL.speed() < motorR.speed(), "When driving Rev in CCW Curve, motorL should be less than motorR"
wait(3000)   # 4 seconds x 90 deg/s = full circle
drive_base.stop()
wait(250)

drive_base.drive(200, -90)   # In corrected version it should Drive CCW Fwd
wait (1000)
print("Test 16 - .drive() - CCW Curve Fwd (turning 90/sec) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() > 0,  "When driving CCW Curve Fwd, left speed should be +ve"
assert motorR.speed() > 0,  "When driving CCW Curve Fwd, right speed should be +ve"
assert motorL.speed() < motorR.speed(), "When driving Fwd in CCW Curve, motorL should be less than motorR"
wait(3000)   # 4 seconds x 90 deg/s = full circle
drive_base.stop()
wait(250)

drive_base.drive(-200, -90)   # In corrected version it should drive CW in Rev
wait (1000)
print("Test 17 - .drive() - CW Curve Rev (turning 90/sec) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "When driving CW Curve Rev, left speed should be +ve"
assert motorR.speed() < 0,  "When driving CW Curve Rev, right speed should be +ve"
assert motorL.speed() > motorR.speed(), "When driving Rev in CW Curve, motorL should be less than motorR"
wait(3000)   # 4 seconds x 90 deg/s = full circle
drive_base.stop()
wait(250)

[laurensvalk]

Thank you, that looks great!

And you could try https://github.com/pybricks/pybricks-micropython/pull/212 to try the alternate behavior.

[JDwyer009]

Thank you, that looks great!

And you could try pybricks/pybricks-micropython#212 to try the alternate behavior.

Thanks @laurensvalk. I tested the newest firmware and the program worked as expected. .curve() and .drive() now behave consistently which is great.

Question: When I run the test program on the robot with the wheels raise off the ground, the program executes correctly. However, when I have it move on the ground it throws up erratic assert errors. Is this just due to inertia and momentum preventing the motors being in the correct state at the time of checking/measuring the 'assert' statements?

[laurensvalk]

Re: question about tests. Have a look at variants like these:

drive_base.curve(-120, -360)  # Drives Clockwise in Rev
print("Test 11 - CW Curve Rev (radius 12cm) - motorL.speed() = {}, motorR.speed() = {}".format(motorL.speed(), motorR.speed()))
assert motorL.speed() < 0,  "When driving Rev in CW Curve, left speed should be -ve"
assert motorR.speed() < 0,  "When driving Rev in CW Curve, right speed should be -ve"
assert motorL.speed() > motorR.speed(), "When driving Rev in CW Curve, motorL should be greater than motorR"

drive_base.curve(-120, -360) will run until the end, and only then the asserts will run.

Perhaps you could use wait=False, like this:

drive_base.curve(-120, -360, wait=False)

# wait briefly to get going

# do the asserts here

# Wait for the rest of the move to finish
while not drive_base.done():
    wait(10)

# and then the next tests.
 

[JDwyer009]

Thanks @laurensvalk

I've edited my original code post to reflect the changes you suggested above and the results now appear to be reliably correct. :)