Change pinmode implementation to use array and improve API usability. Makes releasing pins safe in interrupts. Fix music module's pin releasing.

Author: markshannon

inc/microbit/microbitobj.h

@@ -29,14 +29,9 @@
 extern "C" {
 
 #include "py/obj.h"
+#include "microbitpin.h"
 #include "PinNames.h"
 
-typedef struct _microbit_pin_obj_t {
-    mp_obj_base_t base;
-    uint8_t number; // The pin number on microbit board
-    PinName name; // The pin number in the GPIO port.
-} microbit_pin_obj_t;
-
 const microbit_pin_obj_t *microbit_obj_get_pin(mp_obj_t o);
 PinName microbit_obj_get_pin_name(mp_obj_t o);
 

inc/microbit/microbitpin.h

@@ -26,39 +26,68 @@
 #ifndef __MICROPY_INCLUDED_MICROBIT_MICROBITPIN_H__
 #define __MICROPY_INCLUDED_MICROBIT_MICROBITPIN_H__
 
-#ifndef MICROBIT_PIN_P0
+#include "py/obj.h"
 
-#define MICROBIT_PIN_P0     (P0_3)
-#define MICROBIT_PIN_P1     (P0_2)
-#define MICROBIT_PIN_P13    (P0_23)
-#define MICROBIT_PIN_P14    (P0_22)
-#define MICROBIT_PIN_P15    (P0_21)
+typedef struct _microbit_pin_obj_t {
+    mp_obj_base_t base;
+    uint8_t number; // The pin number on microbit board
+    uint8_t name; // The pin number in the GPIO port.
+    uint8_t initial_mode;
+} microbit_pin_obj_t;
 
-#endif
-
-#include "microbit/microbitobj.h"
-
-mp_obj_t microbit_pin_write_digital(mp_obj_t self_in, mp_obj_t value_in);
-mp_obj_t microbit_pin_read_digital(mp_obj_t self_in);
 typedef void(*release_func)(const microbit_pin_obj_t *pin);
 
 typedef struct _pinmode {
     qstr name;
     release_func release; /* Call this function to release pin */
 } microbit_pinmode_t;
 
+extern const microbit_pinmode_t microbit_pinmodes[];
+
+/* Leave 0 to mean default mode. */
+#define MODE_UNUSED 1
+#define MODE_READ_DIGITAL 2
+#define MODE_WRITE_DIGITAL 3
+#define MODE_DISPLAY 4
+#define MODE_BUTTON 5
+#define MODE_MUSIC 6
+#define MODE_AUDIO_PLAY 7
+#define MODE_TOUCH 8
+#define MODE_I2C 9
+#define MODE_SPI 10
+#define MODE_WRITE_ANALOG 11
 
+#define microbit_pin_mode_unused        (&microbit_pinmodes[MODE_UNUSED])
+#define microbit_pin_mode_write_analog  (&microbit_pinmodes[MODE_WRITE_ANALOG])
+#define microbit_pin_mode_read_digital  (&microbit_pinmodes[MODE_READ_DIGITAL])
+#define microbit_pin_mode_write_digital (&microbit_pinmodes[MODE_WRITE_DIGITAL])
+#define microbit_pin_mode_display       (&microbit_pinmodes[MODE_DISPLAY])
+#define microbit_pin_mode_button        (&microbit_pinmodes[MODE_BUTTON])
+#define microbit_pin_mode_music         (&microbit_pinmodes[MODE_MUSIC])
+#define microbit_pin_mode_audio_play    (&microbit_pinmodes[MODE_AUDIO_PLAY])
+#define microbit_pin_mode_touch         (&microbit_pinmodes[MODE_TOUCH])
+#define microbit_pin_mode_i2c           (&microbit_pinmodes[MODE_I2C])
+#define microbit_pin_mode_spi           (&microbit_pinmodes[MODE_SPI])
+
+/** Can this pin be acquired? Safe to call in an interrupt. Not safe to call in an interrupt. */
 void microbit_obj_pin_fail_if_cant_acquire(const microbit_pin_obj_t *pin);
 
+/** Release pin for use by other modes. Safe to call in an interrupt.
+ * If pin is NULL or pin already unused, then this is a no-op
+ */
 void microbit_obj_pin_free(const microbit_pin_obj_t *pin);
 
-void microbit_obj_pin_acquire(const microbit_pin_obj_t *pin, qstr name);
+/** Acquire pin (causing analog/digital modes to release) for mode.
+ * If pin is already in specified mode, this is a no-op.
+ * Not safe to call in an interrupt as it may raise. */
+void microbit_obj_pin_acquire(const microbit_pin_obj_t *pin, const microbit_pinmode_t *mode);
 
 bool microbit_pin_high_debounced(microbit_pin_obj_t *pin);
 
-qstr microbit_obj_pin_get_mode(const microbit_pin_obj_t *pin);
+const microbit_pinmode_t *microbit_pin_get_mode(const microbit_pin_obj_t *pin);
 
 bool microbit_obj_pin_can_be_acquired(const microbit_pin_obj_t *pin);
 
+void pinmode_error(const microbit_pin_obj_t *pin);
 
 #endif // __MICROPY_INCLUDED_MICROBIT_MICROBITPIN_H__

source/microbit/microbitbutton.cpp

@@ -109,8 +109,6 @@ const microbit_button_obj_t microbit_button_b_obj = {
     .index = 1,
 };
 
-extern uint8_t microbit_pinmodes[];
-
 enum PinTransition
 {
     LOW_LOW = 0,
@@ -162,11 +160,11 @@ void microbit_button_tick(void) {
         pressed[microbit_button_a_obj.index] = (pressed[microbit_button_a_obj.index] + 2) | 1;
     if (update(microbit_button_b_obj.pin) == HIGH_LOW)
         pressed[microbit_button_b_obj.index] = (pressed[microbit_button_b_obj.index] + 2) | 1;
-    if (microbit_obj_pin_get_mode(&microbit_p0_obj) == MP_QSTR_touch)
+    if (microbit_pin_get_mode(&microbit_p0_obj) == microbit_pin_mode_touch)
         update(&microbit_p0_obj);
-    if (microbit_obj_pin_get_mode(&microbit_p1_obj) == MP_QSTR_touch)
+    if (microbit_pin_get_mode(&microbit_p1_obj) == microbit_pin_mode_touch)
         update(&microbit_p1_obj);
-    if (microbit_obj_pin_get_mode(&microbit_p2_obj) == MP_QSTR_touch)
+    if (microbit_pin_get_mode(&microbit_p2_obj) == microbit_pin_mode_touch)
         update(&microbit_p2_obj);
 }
 

source/microbit/microbitdisplay.cpp

@@ -431,12 +431,12 @@ mp_obj_t microbit_display_on_func(mp_obj_t obj) {
     microbit_obj_pin_fail_if_cant_acquire(&microbit_p7_obj);
     microbit_obj_pin_fail_if_cant_acquire(&microbit_p9_obj);
     microbit_obj_pin_fail_if_cant_acquire(&microbit_p10_obj);
-    microbit_obj_pin_acquire(&microbit_p3_obj, MP_QSTR_display);
-    microbit_obj_pin_acquire(&microbit_p4_obj, MP_QSTR_display);
-    microbit_obj_pin_acquire(&microbit_p6_obj, MP_QSTR_display);
-    microbit_obj_pin_acquire(&microbit_p7_obj, MP_QSTR_display);
-    microbit_obj_pin_acquire(&microbit_p9_obj, MP_QSTR_display);
-    microbit_obj_pin_acquire(&microbit_p10_obj, MP_QSTR_display);
+    microbit_obj_pin_acquire(&microbit_p3_obj, microbit_pin_mode_display);
+    microbit_obj_pin_acquire(&microbit_p4_obj, microbit_pin_mode_display);
+    microbit_obj_pin_acquire(&microbit_p6_obj, microbit_pin_mode_display);
+    microbit_obj_pin_acquire(&microbit_p7_obj, microbit_pin_mode_display);
+    microbit_obj_pin_acquire(&microbit_p9_obj, microbit_pin_mode_display);
+    microbit_obj_pin_acquire(&microbit_p10_obj, microbit_pin_mode_display);
     /* Make sure all pins are in the correct state */
     microbit_display_init();
     /* Re-enable the display loop.  This will resume any animations in

source/microbit/microbitmusic.cpp

@@ -70,7 +70,7 @@ static uint32_t async_music_wait_ticks;
 static bool async_music_loop;
 static uint16_t async_music_notes_len;
 static uint16_t async_music_notes_index;
-static const microbit_pin_obj_t *async_music_pin;
+static const microbit_pin_obj_t *async_music_pin = NULL;
 
 extern uint32_t ticks;
 
@@ -100,6 +100,7 @@ void microbit_music_tick(void) {
             } else {
                 async_music_state = ASYNC_MUSIC_STATE_IDLE;
                 microbit_obj_pin_free(async_music_pin);
+                async_music_pin = NULL;
                 return;
             }
         }
@@ -275,9 +276,11 @@ STATIC mp_obj_t microbit_music_stop(mp_uint_t n_args, const mp_obj_t *args) {
     } else {
         pin = microbit_obj_get_pin(args[0]);
     }
+    // Raise exception if the pin we are trying to stop is not in a compatible mode.
+    microbit_obj_pin_acquire(pin, microbit_pin_mode_music);
     pwm_set_duty_cycle(pin->name, 0);
     microbit_obj_pin_free(pin);
-
+    async_music_pin = NULL;
     async_music_state = ASYNC_MUSIC_STATE_IDLE;
 
     return mp_const_none;
@@ -296,6 +299,10 @@ STATIC mp_obj_t microbit_music_play(mp_uint_t n_args, const mp_obj_t *pos_args,
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
+    // reset octave and duration so tunes always play the same
+    music_state.last_octave = DEFAULT_OCTAVE;
+    music_state.last_duration = DEFAULT_DURATION;
+
     // get either a single note or a list of notes
     mp_uint_t len;
     mp_obj_t *items;
@@ -306,21 +313,13 @@ STATIC mp_obj_t microbit_music_play(mp_uint_t n_args, const mp_obj_t *pos_args,
         mp_obj_get_array(args[0].u_obj, &len, &items);
     }
 
+    // Release the previous pin
+    microbit_obj_pin_free(async_music_pin);
+    async_music_pin = NULL;
+
     // get the pin to play on
     const microbit_pin_obj_t *pin = microbit_obj_get_pin(args[1].u_obj);
-
-    if (async_music_state != ASYNC_MUSIC_STATE_IDLE) {
-      // Stop the current music before starting new music.
-      async_music_state = ASYNC_MUSIC_STATE_IDLE;
-      pwm_set_duty_cycle(pin->name, 0);
-      microbit_obj_pin_free(pin);
-    }
-
-    // reset octave and duration so tunes always play the same
-    music_state.last_octave = DEFAULT_OCTAVE;
-    music_state.last_duration = DEFAULT_DURATION;
-
-    microbit_obj_pin_acquire(pin, MP_QSTR_music);
+    microbit_obj_pin_acquire(pin, microbit_pin_mode_music);
 
     // start the tune running in the background
     async_music_state = ASYNC_MUSIC_STATE_IDLE;
@@ -364,11 +363,19 @@ STATIC mp_obj_t microbit_music_pitch(mp_uint_t n_args, const mp_obj_t *pos_args,
     mp_uint_t frequency = args[0].u_int;
     mp_int_t duration = args[1].u_int;
     const microbit_pin_obj_t *pin = microbit_obj_get_pin(args[2].u_obj);
-    microbit_obj_pin_acquire(pin, MP_QSTR_music);
+
+    // Update pin modes
+    microbit_obj_pin_free(async_music_pin);
+    async_music_pin = NULL;
+    microbit_obj_pin_acquire(pin, microbit_pin_mode_music);
     bool wait = args[3].u_bool;
     pwm_set_duty_cycle(pin->name, 128);
-    if (pwm_set_period_us(1000000/frequency))
-        nlr_raise(mp_obj_new_exception_msg(&mp_type_ValueError, "pitch too extreme"));
+    if (frequency == 0) {
+        pwm_release(pin->name);
+    } else if (pwm_set_period_us(1000000/frequency)) {
+        pwm_release(pin->name);
+        nlr_raise(mp_obj_new_exception_msg(&mp_type_ValueError, "invalid pitch"));
+    }
     if (duration >= 0) {
         // use async machinery to stop the pitch after the duration
         async_music_state = ASYNC_MUSIC_STATE_IDLE;