Make accelerometer and compass updates safe for interrupts.

Author: markshannon

inc/microbit/microbitobj.h

@@ -35,6 +35,12 @@ extern "C" {
 MicroBitPin *microbit_obj_get_pin(mp_obj_t o);
 MicroBitImage *microbit_obj_get_image(mp_obj_t o);
 
+extern volatile bool compass_up_to_date;
+extern volatile bool compass_updating;
+
+extern volatile bool accelerometer_up_to_date;
+extern volatile bool accelerometer_updating;
+
 }
 
 #endif // __MICROPY_INCLUDED_MICROBIT_MICROBITOBJ_H__

source/microbit/main.cpp

@@ -1,4 +1,5 @@
 #include "MicroBit.h"
+#include "microbitobj.h"
 #include "microbitdisplay.h"
 
 extern "C" {
@@ -30,15 +31,15 @@ void app_main() {
     }
 }
 
-extern bool compass_up_to_date;
-extern bool accelerometer_up_to_date;
-
 static void ticker(void) {
 
     // increment our real-time counter.
     ticks += FIBER_TICK_PERIOD_MS;
 
-    if (uBit.compass.isCalibrating()) {
+    /** Update compass if it is calibrating, but not if it is still
+     *  updating as compass.idleTick() is not reentrant.
+     */
+    if (uBit.compass.isCalibrating() && !compass_updating) {
         uBit.compass.idleTick();
     }
 

source/microbit/microbitaccelerometer.cpp

@@ -36,45 +36,49 @@ typedef struct _microbit_accelerometer_obj_t {
     MicroBitAccelerometer *accelerometer;
 } microbit_accelerometer_obj_t;
 
-bool accelerometer_up_to_date = false;
+volatile bool accelerometer_up_to_date = false;
+volatile bool accelerometer_updating = false;
 
-mp_obj_t microbit_accelerometer_get_x(mp_obj_t self_in) {
-    microbit_accelerometer_obj_t *self = (microbit_accelerometer_obj_t*)self_in;
-    if (!accelerometer_up_to_date) {
-        self->accelerometer->idleTick();
+static void update(microbit_accelerometer_obj_t *self) {
+    /* The only time it is possible for accelerometer_updating to be true here
+     * is if this is called in an interrupt when it is already updating in
+     * the main execution thread. This is extremely unlikely, so we just
+     * accept that a slightly out-of-date result will be returned
+     */
+    if (!accelerometer_up_to_date && !accelerometer_updating) {
         accelerometer_up_to_date = true;
+        accelerometer_updating = true;
+        self->accelerometer->idleTick();
+        accelerometer_updating = false;
     }
+}
+
+
+mp_obj_t microbit_accelerometer_get_x(mp_obj_t self_in) {
+    microbit_accelerometer_obj_t *self = (microbit_accelerometer_obj_t*)self_in;
+    update(self);
     return mp_obj_new_int(self->accelerometer->getX());
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_accelerometer_get_x_obj, microbit_accelerometer_get_x);
 
 mp_obj_t microbit_accelerometer_get_y(mp_obj_t self_in) {
     microbit_accelerometer_obj_t *self = (microbit_accelerometer_obj_t*)self_in;
-    if (!accelerometer_up_to_date) {
-        self->accelerometer->idleTick();
-        accelerometer_up_to_date = true;
-    }
+    update(self);
     return mp_obj_new_int(self->accelerometer->getY());
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_accelerometer_get_y_obj, microbit_accelerometer_get_y);
 
 mp_obj_t microbit_accelerometer_get_z(mp_obj_t self_in) {
     microbit_accelerometer_obj_t *self = (microbit_accelerometer_obj_t*)self_in;
-    if (!accelerometer_up_to_date) {
-        self->accelerometer->idleTick();
-        accelerometer_up_to_date = true;
-    }
+    update(self);
     return mp_obj_new_int(self->accelerometer->getZ());
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_accelerometer_get_z_obj, microbit_accelerometer_get_z);
 
 mp_obj_t microbit_accelerometer_get_values(mp_obj_t self_in) {
     microbit_accelerometer_obj_t *self = (microbit_accelerometer_obj_t*)self_in;
     mp_obj_tuple_t *tuple = (mp_obj_tuple_t *)mp_obj_new_tuple(3, NULL);
-    if (!accelerometer_up_to_date) {
-        self->accelerometer->idleTick();
-        accelerometer_up_to_date = true;
-    }
+    update(self);
     tuple->items[0] = mp_obj_new_int(self->accelerometer->getX());
     tuple->items[1] = mp_obj_new_int(self->accelerometer->getY());
     tuple->items[2] = mp_obj_new_int(self->accelerometer->getZ());

source/microbit/microbitcompass.cpp

@@ -62,44 +62,47 @@ mp_obj_t microbit_compass_clear_calibration(mp_obj_t self_in) {
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_compass_clear_calibration_obj, microbit_compass_clear_calibration);
 
-bool compass_up_to_date = false;
-
-mp_obj_t microbit_compass_heading(mp_obj_t self_in) {
-    microbit_compass_obj_t *self = (microbit_compass_obj_t*)self_in;
-    if (!compass_up_to_date) {
+volatile bool compass_up_to_date = false;
+volatile bool compass_updating = false;
+
+static void update(microbit_compass_obj_t *self) {
+    /* The only time it is possible for compass_updating to be true here
+     * is if this is called in an interrupt when it is already updating in
+     * the main execution thread. This is extremely unlikely, so we just
+     * accept that a slightly out-of-date result will be returned
+     */
+    if (!compass_up_to_date && !compass_updating) {
+        compass_updating = true;
         self->compass->idleTick();
+        compass_updating = false;
         compass_up_to_date = true;
     }
+}
+
+mp_obj_t microbit_compass_heading(mp_obj_t self_in) {
+    microbit_compass_obj_t *self = (microbit_compass_obj_t*)self_in;
+    update(self);
     return mp_obj_new_int(self->compass->heading());
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_compass_heading_obj, microbit_compass_heading);
 
 mp_obj_t microbit_compass_get_x(mp_obj_t self_in) {
     microbit_compass_obj_t *self = (microbit_compass_obj_t*)self_in;
-    if (!compass_up_to_date) {
-        self->compass->idleTick();
-        compass_up_to_date = true;
-    }
+    update(self);
     return mp_obj_new_int(self->compass->getX());
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_compass_get_x_obj, microbit_compass_get_x);
 
 mp_obj_t microbit_compass_get_y(mp_obj_t self_in) {
     microbit_compass_obj_t *self = (microbit_compass_obj_t*)self_in;
-    if (!compass_up_to_date) {
-        self->compass->idleTick();
-        compass_up_to_date = true;
-    }
+    update(self);
     return mp_obj_new_int(self->compass->getY());
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_compass_get_y_obj, microbit_compass_get_y);
 
 mp_obj_t microbit_compass_get_z(mp_obj_t self_in) {
     microbit_compass_obj_t *self = (microbit_compass_obj_t*)self_in;
-    if (!compass_up_to_date) {
-        self->compass->idleTick();
-        compass_up_to_date = true;
-    }
+    update(self);
     return mp_obj_new_int(self->compass->getZ());
 }
 MP_DEFINE_CONST_FUN_OBJ_1(microbit_compass_get_z_obj, microbit_compass_get_z);