[Bug] unexpected motor oscillations due use of Icon in a loop

[Xylar52]

Describe the bug
The motor stutter to an extreme level when i use the DriveBase.drive, Motor.run and all infintie movement code (on spike)

To reproduce
Steps to reproduce the behavior:

1. Have a spike
2. run the motors forever with drivebase.drive or motor.run

Expected behavior
a motor that is turning smooth

Screenshots
this is with DriveBase.drive(100,0) :
https://user-images.githubusercontent.com/90038318/224097846-27abb1a0-5f2e-4c2f-8f07-fcdecb083f48.mp4

this is with Motor.run(100) :
https://user-images.githubusercontent.com/90038318/224098182-314478f0-7da0-4416-bf45-0a226a3484bc.mp4

[dlech]

Which firmware version are you using? We just released a new version yesterday at https://beta.pybricks.com that should improve running motors at low speeds.

100 deg/sec is near the low limit of where the motors can actually be controlled. If you need to run at low speeds like that, you can also consider gearing down the wheels.

[Xylar52]

I am using firmware v3.2.3 i think.
the code is very simple just lot of motors and sensors.
and it isn't just for low speed i can put it at 1000 deg/s it will also heavily stutter

[laurensvalk]

This is indeed unusual behavior. I will try to reproduce it.

For future bug reports, it would be great if you want to paste the code here. Makes it a bit easier for us to try it out. Thanks!

[laurensvalk]

The following minimal program reproduces it:

from pybricks.hubs import PrimeHub
from pybricks.pupdevices import Motor
from pybricks.parameters import Port, Icon

hub = PrimeHub()
motor = Motor(Port.C)

while True:
    hub.display.icon(Icon.HAPPY)
    motor.run(100)

It goes away when removing the icon display step. It also goes away when adding a wait(10).

That's definitely a bug - the user shouldn't have to know about these things.

Thanks @Xylar52! We'll do our best to fix it as soon as possible.

[laurensvalk]

Sometimes, the motor call fails with EAGAIN: Try again later, which points towards the layer below the controls. Indeed, the motor position produces bad data:

[laurensvalk]

OK - the problem appears to be repeatedly loading the Icon.

This program does not have the problem:

from pybricks.hubs import PrimeHub
from pybricks.pupdevices import Motor
from pybricks.parameters import Port, Icon

hub = PrimeHub()
motor = Motor(Port.C)

happy = Icon.HAPPY

while True:
    hub.display.icon(happy)
    motor.run(100)

I thought we wouldn't block during garbage collect, so maybe there is something else going on.

[laurensvalk]

This is definitely worth fixing, but probably doesn't require an emergency patch. Phew 😅.

[Xylar52]

well that's strange, how would defining icon.happy as happy change things ?

[laurensvalk]

Icon.HAPPY (and the other icons) are represented as Matrices. This allows you to easily manipulate or compose images, but it means they are stored as 5x5 floating point values. Calling Icon.HAPPY in a fast loop allocates a new Matrix object again and again, at a really fast rate, so the hub runs out of memory.

When storing one matrix in a variable outside of the loop, it creates only one of those matrices so there is no memory error.

Still, the hub is supposed to automatically clean up unused memory behind the scenes without interrupting everything else, but you've helped us find a case where that doesn't happen correctly :)

[laurensvalk]

Here's an even smaller program to reproduce the issue:

from pybricks.pupdevices import Motor
from pybricks.parameters import Port, Icon

motor = Motor(Port.C)

motor.run(100)

while True:
    Icon.HAPPY  # Makes the hub unhappy

[laurensvalk]

So this is probably not related to motors per se.

Still @Xylar52 --- we did improve the motor performance at low speeds, so try https://beta.pybricks.com/ if the motors are still not smooth enough after removing the icons from your program.

[dlech]

I think we've stumbled on a really strange MicroPython GC bug. This seems to be triggered specifically by the pb_type_Matrix_make_bitmap() function (and also pb_type_Matrix_make_vector() if you call it with a value >> 3, not something that is possible currently without modifying C code).

If we add an extra allocation in the function, the problem goes away. This essentially has the effect of allocating one extra GC block.

diff --git a/pybricks/geometry/pb_type_matrix.c b/pybricks/geometry/pb_type_matrix.c
index 005c047d..e9a6a648 100644
--- a/pybricks/geometry/pb_type_matrix.c
+++ b/pybricks/geometry/pb_type_matrix.c
@@ -564,6 +564,7 @@ mp_obj_t pb_type_Matrix_make_bitmap(size_t m, size_t n, float scale, uint32_t sr
     mat->n = n;
     mat->scale = scale;
     mat->data = m_new(float, m * n);
+    m_new(float, 1);
 
     for (size_t i = 0; i < m * n; i++) {
         mat->data[m * n - i - 1] = (src & (1 << i)) != 0;

Alternately, if we change the size of m and n in pb_type_Matrix_obj_t, the problem also goes away. This reduces the size of the struct by 8 bytes which allows it to fit in one GC block instead of 2.

diff --git a/pybricks/geometry.h b/pybricks/geometry.h
index 865f7276..41c33c25 100644
--- a/pybricks/geometry.h
+++ b/pybricks/geometry.h
@@ -18,8 +18,8 @@ typedef struct _pb_type_Matrix_obj_t {
     mp_obj_base_t base;
     float *data;
     float scale;
-    size_t m;
-    size_t n;
+    uint8_t m;
+    uint8_t n;
     bool transposed;
 } pb_type_Matrix_obj_t;

Also, interestingly, trying to reproduce the issue using Matrix([[0] * 5] * 5]) instead of Icon(0) doesn't seem to reproduce the issue even though it creates essentially the same Matrix object. The only difference is that it uses pb_type_Matrix_make_new() instead of pb_type_Matrix_make_bitmap().

[laurensvalk]

Thanks for looking into this. One other thing I've tried is to allocate both the Matrix object and its data in a continuous single m_alloc instead of two, but this did not make a difference.

[dlech]

I think the bug should be fixed now (when the heap got full, allocation took a long time and interfered with motor control).

The hypothesis that the UART connection to the motor timed out and reset because it wasn't being serviced fast enough, causing bad position data seems plausible.

[dlech]

You can grab the latest firmware from https://nightly.link/pybricks/pybricks-micropython/workflows/build/master for testing.