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

Motor.run_target() raises RuntimeError #786

Closed
dlech opened this issue Nov 10, 2022 · 11 comments
Closed

Motor.run_target() raises RuntimeError #786

dlech opened this issue Nov 10, 2022 · 11 comments
Assignees
Labels
bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: motors Issues involving motors

Comments

@dlech
Copy link
Member

dlech commented Nov 10, 2022

Describe the bug
What is the problem?

After some time, the program below crashes one of the steer.run_target() lines with a RuntimeError. The time is variable but usually less than a minute (pressing buttons on the remote to steer during this time).

Full output:

('technichub', '3.2.0b4', 'v3.2.0b4-34-g51478f08-dirty on 2022-11-10')
steer angle: 102.5
Traceback (most recent call last):
  File "issue785.py", line 53, in <module>
RuntimeError: Unknown error

To reproduce

from pybricks.pupdevices import Motor, Remote
from pybricks.parameters import Port, Direction, Stop, Button, Color
from pybricks.hubs import TechnicHub
from pybricks.tools import wait
from pybricks import version

print(version)

# Initialize the motors.
steer = Motor(Port.C)
front = Motor(Port.D, Direction.COUNTERCLOCKWISE)

# Connect to the remote and set the light on the remote
remote = Remote()
remote.light.on(Color.GREEN)

# Initialize the hub.
hub = TechnicHub()
hub.light.on(Color.GREEN)

# Read the current settings
old_kp, old_ki, old_kd, _, _ = steer.control.pid()

# Set new values
steer.control.pid(kp=old_kp*4, kd=old_kd*0.4)

# Set initial top speed value
top_speed = 100

# Find the steering endpoint on the left and right.
# The middle is in between.
left_end = steer.run_until_stalled(-200, then=Stop.HOLD)
right_end = steer.run_until_stalled(200, then=Stop.HOLD)

# We are now at the right. Reset this angle to be half the difference.
# That puts zero in the middle.
steer.reset_angle((right_end - left_end)/2)
steer.run_target(speed=200, target_angle=0, wait=False)

# Set steering angle for the buggy
steer_angle = (((right_end - left_end)/2)-5)
print('steer angle:',steer_angle)

# Now we can start driving!
while True:
    # Check which buttons are pressed.
    pressed = remote.buttons.pressed()

    # Choose the steer angle based on the right controls.
    if Button.LEFT_PLUS in pressed:
        steer.run_target(1400, -steer_angle, Stop.HOLD, False)
    elif Button.LEFT_MINUS in pressed:
        steer.run_target(1400, steer_angle, Stop.HOLD, False)
    else:
        steer.track_target(0)

    # Top speed controls
    if Button.LEFT in pressed:
        top_speed = 75
    if Button.RIGHT in pressed:
        top_speed = 100   
    if ((Button.RIGHT in pressed) and (Button.LEFT in pressed)):
        top_speed = 40 

    # Choose the drive speed based on the left controls.
    drive_speed = 0
    if Button.RIGHT_PLUS in pressed:
        drive_speed -= top_speed
    if Button.RIGHT_MINUS in pressed:
        drive_speed += top_speed         

    # Apply the selected speed.
    front.dc(drive_speed)

    # Wait.
    wait(10)

Expected behavior
What did you expect to happen instead?

Program should run forever.

@dlech dlech added bug Something isn't working topic: motors Issues involving motors software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) labels Nov 10, 2022
@laurensvalk
Copy link
Member

laurensvalk commented Nov 10, 2022

Nice find 👍 , I'm happy to look into it.

@laurensvalk
Copy link
Member

Which motors are connected here? They all have different acceleration profiles, which could be relevant for reproduction.

@dlech
Copy link
Member Author

dlech commented Nov 10, 2022

I used the gray medium motors from Robot Inventor for this test.

@laurensvalk laurensvalk self-assigned this Nov 10, 2022
@laurensvalk
Copy link
Member

laurensvalk commented Nov 10, 2022

This probably fails in pbio/trajectory.c, where PBIO_ERROR_FAILED indicates an invalid trajectory.

We should be able to do something like:

old_trajectory = steer.control.trajectory()
try:
    steer.run_target(1400, -steer_angle, Stop.HOLD, False)
except:
    print(steer_angle)
    print(old_trajectory)
    print(steer.control.limits())
    print(steer.control.trajectory())

Then we can add the failed case to test/trajectory.c and statically analyse and debug it.

@laurensvalk
Copy link
Member

For what it's worth, a contributing factor is this. This is an old hack that should no longer be used.

steer.control.pid(kp=old_kp*4, kd=old_kd*0.4)

@laurensvalk
Copy link
Member

laurensvalk commented Nov 11, 2022

ThisHub variant with debug:

from pybricks.pupdevices import Motor, Remote
from pybricks.parameters import Port, Direction, Stop, Button, Color
from pybricks.hubs import ThisHub
from pybricks.tools import wait
from pybricks import version

print(version)

# Initialize the motors.
steer = Motor(Port.A)
front = Motor(Port.B, Direction.COUNTERCLOCKWISE)

# Connect to the remote and set the light on the remote
remote = Remote()
remote.light.on(Color.GREEN)

# Initialize the hub.
hub = ThisHub()
hub.light.on(Color.GREEN)

# Read the current settings
old_kp, old_ki, old_kd, _, _ = steer.control.pid()

# Set new values
steer.control.pid(kp=old_kp*4, kd=old_kd*0.4)

# Set initial top speed value
top_speed = 100

# Find the steering endpoint on the left and right.
# The middle is in between.
# left_end = steer.run_until_stalled(-200, then=Stop.HOLD)
# right_end = steer.run_until_stalled(200, then=Stop.HOLD)

# We are now at the right. Reset this angle to be half the difference.
# That puts zero in the middle.
# steer.reset_angle((right_end - left_end)/2)
# steer.run_target(speed=200, target_angle=0, wait=False)

# Set steering angle for the buggy
# steer_angle = (((right_end - left_end)/2)-5)
# print('steer angle:',steer_angle)
steer_angle = 167.5

# Now we can start driving!
while True:
    # Check which buttons are pressed.
    pressed = remote.buttons.pressed()


    old_trajectory = steer.control.trajectory()
    try:
        # Choose the steer angle based on the right controls.
        if Button.LEFT_PLUS in pressed:
            steer.run_target(1400, -steer_angle, Stop.HOLD, False)
        elif Button.LEFT_MINUS in pressed:
            steer.run_target(1400, steer_angle, Stop.HOLD, False)
        else:
            steer.track_target(0)
    except RuntimeError:
        print(steer_angle)
        print(old_trajectory)
        print(steer.control.limits())
        print(steer.control.trajectory())
        hub.speaker.beep()

    # Top speed controls
    if Button.LEFT in pressed:
        top_speed = 75
    if Button.RIGHT in pressed:
        top_speed = 100   
    if ((Button.RIGHT in pressed) and (Button.LEFT in pressed)):
        top_speed = 40 

    # Choose the drive speed based on the left controls.
    drive_speed = 0
    if Button.RIGHT_PLUS in pressed:
        drive_speed -= top_speed
    if Button.RIGHT_MINUS in pressed:
        drive_speed += top_speed         

    # Apply the selected speed.
    front.dc(drive_speed)

    # Wait.
    wait(10)

@laurensvalk
Copy link
Member

Here are two cases:

steer angle: 167.5
167.5
(0, 0, 0, 12, 0, 0, 0, 0, -24, -24, 0, 2000, 2000)
(1080, 2000, 199)
(0, 0, 0, 1, 0, 0, 0, 0, 3, 3, 0, 2000, -2000)
167.5
(0, 0, 0, 12, 0, 0, 0, 0, 24, 24, 0, -2000, -2000)
(1080, 2000, 199)
(0, 0, 0, 1, 0, 0, 0, 0, 3, 3, 0, 2000, -2000)

@laurensvalk
Copy link
Member

laurensvalk commented Nov 11, 2022

Running with this diff gives a bit more detail:

diff --git a/pybricks/common/pb_type_control.c b/pybricks/common/pb_type_control.c
index b96e5edc..620f9649 100644
--- a/pybricks/common/pb_type_control.c
+++ b/pybricks/common/pb_type_control.c
@@ -213,16 +213,16 @@ STATIC mp_obj_t common_Control_trajectory(mp_obj_t self_in) {
 
     if (pbio_control_is_active(self->control)) {
         parms[0] = mp_obj_new_int(0);
-        parms[1] = mp_obj_new_int(trj->t1 / 10);
-        parms[2] = mp_obj_new_int(trj->t2 / 10);
-        parms[3] = mp_obj_new_int(trj->t3 / 10);
+        parms[1] = mp_obj_new_int(trj->t1);
+        parms[2] = mp_obj_new_int(trj->t2);
+        parms[3] = mp_obj_new_int(trj->t3);
         parms[4] = mp_obj_new_int(0);
-        parms[5] = mp_obj_new_int(trj->th1 / 1000);
-        parms[6] = mp_obj_new_int(trj->th2 / 1000);
-        parms[7] = mp_obj_new_int(trj->th3 / 1000);
-        parms[8] = mp_obj_new_int(trj->w0 / 10);
-        parms[9] = mp_obj_new_int(trj->w1 / 10);
-        parms[10] = mp_obj_new_int(trj->w3 / 10);
+        parms[5] = mp_obj_new_int(trj->th1);
+        parms[6] = mp_obj_new_int(trj->th2);
+        parms[7] = mp_obj_new_int(trj->th3);
+        parms[8] = mp_obj_new_int(trj->w0);
+        parms[9] = mp_obj_new_int(trj->w1);
+        parms[10] = mp_obj_new_int(trj->w3);
         parms[11] = mp_obj_new_int(trj->a0);
         parms[12] = mp_obj_new_int(trj->a2);
         return mp_obj_new_tuple(13, parms);

@laurensvalk
Copy link
Member

laurensvalk commented Nov 11, 2022

steer angle: 136.0
136.0
(0, 0, 0, 110, 0, 0, 0, 121, 220, 220, 0, -2000, -2000)
(1080, 2000, 199)
(0, -9, -9, -9, 0, 0, 0, 1, 18, 0, 0, 2000, -2000)
0   = 0,
t1  =-9,
t2  =-9,
t3  =-9,
0   = 0,
th1 = 0,
th2 = 0,
th3 = 1,
w0  = 18,
w1  = 0,
w3  = 0,
a0  = 2000,
a2  =-2000

The error is nagative time t1 of -9 (or 0 when rounded) so it fails at:

https://github.com/pybricks/pybricks-micropython/blob/51478f08b392110bc0ea6cc9c5940266c254d93c/lib/pbio/src/trajectory.c#L469

laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Nov 11, 2022
Fix incorrect acceleration for rounded speed values, causing negative duration.

Also assert more time values for easier debugging.

See pybricks/support#786
laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Nov 11, 2022
For short maneuvers, t1 can be 0. If time is also 0, it wouldn't get into the first segment of the trajectory.

See pybricks/support#786
laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Nov 11, 2022
While sub-degree maneuvers cannot be entered by users, we can still have sub-degree maneuvers if a new command is started while the trajectory is already near the target.

This happens relatively often when restarting run_target towards the same target in a tight loop.

See pybricks/support#786
laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Nov 11, 2022
If t1=0, the speed skips to w1 right away. Even though w0 is not used in that case, it still makes sense to give it a well defined value. This simplifies debugging and is useful for validation in tests.

See pybricks/support#786
@laurensvalk
Copy link
Member

laurensvalk commented Nov 11, 2022

@dlech Is it better with e.g. pybricks/pybricks-micropython@9af922d?

(Ignore the shaking, that's just excessive PID as mentioned above, but it may help make the actual bug occur more often.)

@dlech
Copy link
Member Author

dlech commented Nov 11, 2022

I can no longer reproduce the problem, so seems to be fixed, thanks!

@dlech dlech closed this as completed Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: motors Issues involving motors
Projects
None yet
Development

No branches or pull requests

2 participants