[Bug] Occasional bad byte in gyro value on Technic Hub

[laurensvalk]

On Technic Hub, it never becomes stationary with the default settings (...)

Originally posted by @laurensvalk in #1022 (comment)

---

[laurensvalk]

I think I have identified the issue.

When we become nonstationary, it is normally because the (raw) sensor values change beyond a threshold. For example, if I touch the hub with the configuration described below, I get an expected result such as:

# gyro xyz, accl xyz, 16 bit int
-2      11      3       39      3       4308  # Stationary value we compare to
-3      9       3       60      46      4189  # This exceeds the 100 threshold on Z, so nonstationary

But sometimes when it is sitting perfectly still, we see:

-2      5       3       41      0       4248
253     9       0       34      -16     4244  # <---- The 253 should be approximately -2

Could we be reading the frame while it is being updated?

Reproduce:

Run a program like this on the Technic hub while it sits flat on the table:

from pybricks.hubs import ThisHub
from pybricks.tools import wait
from pybricks.experimental import hello_world

hub = ThisHub()

wait(2000)
hello_world(5)

while hub.imu.stationary():

    wait(10)

Run a modified firmware to add a debug print when the hub first becomes non stationary:

+#include <stdio.h>
+
+bool debug_imu_print = false;
+
 static void pbdrv_imu_lsm6ds3tr_c_stm32_update_stationary_status(pbdrv_imu_dev_t *imu_dev) {
 
     // Check whether still stationary compared to constant start sample.
@@ -264,6 +268,14 @@ static void pbdrv_imu_lsm6ds3tr_c_stm32_update_stationary_status(pbdrv_imu_dev_t
         imu_dev->stationary_accel_data_sum[1] += imu_dev->data[4];
         imu_dev->stationary_accel_data_sum[2] += imu_dev->data[5];
     } else {
+
+        if (debug_imu_print) {
+            debug_imu_print = false;
+            printf("%d\t%d\t%d\t%d\t%d\t%d\n", imu_dev->stationary_data_start[0], imu_dev->stationary_data_start[1], imu_dev->stationary_data_start[2], imu_dev->stationary_data_start[3], imu_dev->stationary_data_start[4], imu_dev->stationary_data_start[5]);
+            printf("%d\t%d\t%d\t%d\t%d\t%d\n", imu_dev->data[0], imu_dev->data[1], imu_dev->data[2], imu_dev->data[3], imu_dev->data[4], imu_dev->data[5]);
+            
+        }
+
         // Not stationary anymore, so reset. Current sample becomes new start.
         memcpy(&imu_dev->stationary_data_start[0], &imu_dev->data[0], NUM_DATA_BYTES);
         imu_dev->stationary_now = false;
diff --git a/pybricks/experimental/pb_module_experimental.c b/pybricks/experimental/pb_module_experimental.c
index c8b43f453..f705a02e3 100644
--- a/pybricks/experimental/pb_module_experimental.c
+++ b/pybricks/experimental/pb_module_experimental.c
@@ -54,6 +54,8 @@ STATIC mp_obj_t mod_experimental_pthread_raise(mp_obj_t thread_id_in, mp_obj_t e
 STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_experimental_pthread_raise_obj, mod_experimental_pthread_raise);
 #endif // PYBRICKS_HUB_EV3BRICK
 
+extern bool debug_imu_print;
+
 // pybricks.experimental.hello_world
 STATIC mp_obj_t experimental_hello_world(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     PB_PARSE_ARGS_FUNCTION(n_args, pos_args, kw_args,
@@ -76,6 +78,8 @@ STATIC mp_obj_t experimental_hello_world(size_t n_args, const mp_obj_t *pos_args
     // y = hello_world(5, 6)
     // y = hello_world(foo=5, bar=6)
 
+    debug_imu_print = true;
+
     // All input arguments are available as MicroPython objects with _in post-fixed to the name.
     // Use MicroPython functions or Pybricks helper functions to unpack them into C types:
     mp_int_t foo = pb_obj_get_int(foo_in);

[laurensvalk]

Could we be reading the frame while it is being updated?

Reverting pybricks/pybricks-micropython@121c4db seems to fix it.

The data sheet does recommend to keep this on.

Is there a downside to keeping this on? Could we occasionally miss an update this way?