Clean up interrupt handling for pulseio.PulseIn implementation for adafruit#716 ESP8266

Author: nickzoic

ports/esp8266/common-hal/microcontroller/Pin.c

@@ -24,6 +24,7 @@
  * THE SOFTWARE.
  */
 
+#include "shared-bindings/microcontroller/__init__.h"
 #include "common-hal/microcontroller/__init__.h"
 #include "common-hal/microcontroller/Pin.h"
 #include "shared-bindings/microcontroller/Pin.h"
@@ -35,6 +36,28 @@
 bool adc_in_use;
 bool gpio16_in_use;
 
+typedef struct {
+    void (*func)(void *);
+    void *data;
+} pin_intr_handler_t;
+
+static pin_intr_handler_t _pin_intr_handlers[GPIO_PIN_COUNT];
+
+void microcontroller_pin_call_intr_handlers(uint32_t status) {
+    status &= (1 << GPIO_PIN_COUNT) - 1;
+    for (int p = 0; status; ++p, status >>= 1) {
+        if ((status & 1) && _pin_intr_handlers[p].func) {
+            _pin_intr_handlers[p].func(_pin_intr_handlers[p].data);
+        }
+    }
+}
+
+void microcontroller_pin_register_intr_handler(uint8_t gpio_number, void (*func)(void *), void *data) {
+    common_hal_mcu_disable_interrupts();
+    _pin_intr_handlers[gpio_number] = (pin_intr_handler_t){ func, data };
+    common_hal_mcu_enable_interrupts();
+}
+
 bool common_hal_mcu_pin_is_free(const mcu_pin_obj_t* pin) {
     if (pin == &pin_TOUT) {
         return !adc_in_use;
@@ -92,4 +115,4 @@ void reset_pins(void) {
 
     adc_in_use = false;
     gpio16_in_use = false;
-}
+}

ports/esp8266/common-hal/microcontroller/Pin.h

@@ -45,4 +45,7 @@ void claim_pin(const mcu_pin_obj_t* pin);
 void reset_pin(const mcu_pin_obj_t* pin);
 void reset_pins(void);
 
+void microcontroller_pin_register_intr_handler(uint8_t gpio_number, void (*func)(void *), void *data);
+void microcontroller_pin_call_intr_handlers(uint32_t status);
+
 #endif  // MICROPY_INCLUDED_ESP8266_COMMON_HAL_MICROCONTROLLER_PIN_H

ports/esp8266/common-hal/pulseio/PulseIn.c

@@ -37,9 +37,6 @@
 #include "shared-bindings/pulseio/PulseIn.h"
 #include "common-hal/microcontroller/__init__.h"
 
-// XXX map gpio pins to pulsein objects: kinda clumsy.
-static pulseio_pulsein_obj_t *pulseio_pulsein_objs[GPIO_PIN_COUNT] = {0};
-
 static void pulsein_set_interrupt(pulseio_pulsein_obj_t *self, bool rising, bool falling) {
     ETS_GPIO_INTR_DISABLE();
     // Set interrupt mode
@@ -55,7 +52,9 @@ static void pulsein_set_interrupt(pulseio_pulsein_obj_t *self, bool rising, bool
     ETS_GPIO_INTR_ENABLE();
 }
 
-void pulseio_pulsein_interrupt_handler(pulseio_pulsein_obj_t *self, uint32_t time_us) {
+void pulseio_pulsein_interrupt_handler(void *data) {
+    pulseio_pulsein_obj_t *self = data;
+    uint32_t time_us = system_get_time();
     if (self->first_edge) {
         self->first_edge = false;
         pulsein_set_interrupt(self, true, true);
@@ -72,18 +71,6 @@ void pulseio_pulsein_interrupt_handler(pulseio_pulsein_obj_t *self, uint32_t tim
     self->last_us = time_us;
 }
 
-// XXX needs a better name, or a better abstraction
-// XXX called from intr.c:pin_intr_handler_iram ... inelegantly
-void pulsein_interrupt_handler(uint32_t status) {
-    uint32_t time_us = system_get_time();
-    for (int i=0; i<GPIO_PIN_COUNT; i++) {
-        if (status & 1<<i) {
-            pulseio_pulsein_obj_t *self = pulseio_pulsein_objs[i];
-            if (self) pulseio_pulsein_interrupt_handler(self, time_us);
-        }
-    }
-}
-
 void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t* self,
         const mcu_pin_obj_t* pin, uint16_t maxlen, bool idle_state) {
     if (pin->gpio_number == NO_GPIO || pin->gpio_function == SPECIAL_CASE) {
@@ -104,19 +91,22 @@ void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t* self,
     self->len = 0;
     self->first_edge = true;
     self->last_us = 0;
-    pulseio_pulsein_objs[self->pin->gpio_number] = self;
+
+    microcontroller_pin_register_intr_handler(self->pin->gpio_number,
+            pulseio_pulsein_interrupt_handler, (void *)self);
     pulsein_set_interrupt(self, !idle_state, idle_state);
 }
 
 bool common_hal_pulseio_pulsein_deinited(pulseio_pulsein_obj_t* self) {
-    return pulseio_pulsein_objs[self->pin->gpio_number] == NULL;
+    return self->buffer == NULL;
 }
 
 void common_hal_pulseio_pulsein_deinit(pulseio_pulsein_obj_t* self) {
     pulsein_set_interrupt(self, false, false);
-    pulseio_pulsein_objs[self->pin->gpio_number] = NULL;
+    microcontroller_pin_register_intr_handler(self->pin->gpio_number, NULL, NULL);
     PIN_FUNC_SELECT(self->pin->peripheral, 0);
     m_free(self->buffer);
+    self->buffer = NULL;
 }
 
 void common_hal_pulseio_pulsein_pause(pulseio_pulsein_obj_t* self) {

ports/esp8266/intr.c

@@ -34,8 +34,10 @@
 void pin_intr_handler_iram(void *arg) {
     uint32_t status = GPIO_REG_READ(GPIO_STATUS_ADDRESS);
     GPIO_REG_WRITE(GPIO_STATUS_W1TC_ADDRESS, status);
+
+    // machine.Pin handlers
     pin_intr_handler(status);
 
-    // XXX bit of a hack
-    pulsein_interrupt_handler(status);
+    // microcontroller.Pin handlers
+    microcontroller_pin_call_intr_handlers(status);
 }