Merge pull request #10634 from dhalbert/rotaryio-proper-deinit

rotaryio.IncrementalEncoder deinit fix and improvements

Author: tannewt

ports/atmel-samd/common-hal/rotaryio/IncrementalEncoder.c

@@ -17,6 +17,10 @@
 
 void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencoder_obj_t *self,
     const mcu_pin_obj_t *pin_a, const mcu_pin_obj_t *pin_b) {
+
+    // Ensure object starts in its deinit state.
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+
     if (!pin_a->has_extint) {
         raise_ValueError_invalid_pin_name(MP_QSTR_pin_a);
     }
@@ -83,10 +87,13 @@ void common_hal_rotaryio_incrementalencoder_deinit(rotaryio_incrementalencoder_o
     turn_off_eic_channel(self->eic_channel_b);
 
     reset_pin_number(self->pin_a);
-    self->pin_a = NO_PIN;
-
     reset_pin_number(self->pin_b);
-    self->pin_b = NO_PIN;
+
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+}
+
+void common_hal_rotaryio_incrementalencoder_mark_deinit(rotaryio_incrementalencoder_obj_t *self) {
+    self->pin_a = NO_PIN;
 }
 
 void incrementalencoder_interrupt_handler(uint8_t channel) {

ports/espressif/common-hal/rotaryio/IncrementalEncoder.c

@@ -21,6 +21,10 @@ void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencode
     //
     // These routines also implicitly configure the weak internal pull-ups, as expected
     // in CircuitPython.
+
+    // Ensure object starts in its deinit state.
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+
     pcnt_unit_config_t unit_config = {
         // Set counter limit
         .low_limit = INT16_MIN,
@@ -87,6 +91,10 @@ void common_hal_rotaryio_incrementalencoder_deinit(rotaryio_incrementalencoder_o
     pcnt_del_channel(self->channel_a);
     pcnt_del_channel(self->channel_b);
     pcnt_del_unit(self->unit);
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+}
+
+void common_hal_rotaryio_incrementalencoder_mark_deinit(rotaryio_incrementalencoder_obj_t *self) {
     self->unit = NULL;
 }
 

ports/mimxrt10xx/common-hal/rotaryio/IncrementalEncoder.c

@@ -26,6 +26,9 @@ static void encoder_change(void *self_in) {
 void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencoder_obj_t *self,
     const mcu_pin_obj_t *pin_a, const mcu_pin_obj_t *pin_b) {
 
+    // Ensure object starts in its deinit state.
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+
     self->pin_a = pin_a;
     self->pin_b = pin_b;
 
@@ -49,7 +52,7 @@ void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencode
 }
 
 bool common_hal_rotaryio_incrementalencoder_deinited(rotaryio_incrementalencoder_obj_t *self) {
-    return !self->pin_a;
+    return self->pin_a == NULL;
 }
 
 void common_hal_rotaryio_incrementalencoder_deinit(rotaryio_incrementalencoder_obj_t *self) {
@@ -62,6 +65,9 @@ void common_hal_rotaryio_incrementalencoder_deinit(rotaryio_incrementalencoder_o
     common_hal_reset_pin(self->pin_a);
     common_hal_reset_pin(self->pin_b);
 
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+}
+
+void common_hal_rotaryio_incrementalencoder_mark_deinit(rotaryio_incrementalencoder_obj_t *self) {
     self->pin_a = NULL;
-    self->pin_b = NULL;
 }

ports/nordic/common-hal/rotaryio/IncrementalEncoder.c

@@ -32,6 +32,9 @@ static void _intr_handler(nrfx_gpiote_pin_t pin, nrf_gpiote_polarity_t action) {
 void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencoder_obj_t *self,
     const mcu_pin_obj_t *pin_a, const mcu_pin_obj_t *pin_b) {
 
+    // Ensure object starts in its deinit state.
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+
     self->pin_a = pin_a->number;
     self->pin_b = pin_b->number;
 
@@ -78,6 +81,10 @@ void common_hal_rotaryio_incrementalencoder_deinit(rotaryio_incrementalencoder_o
     nrfx_gpiote_in_uninit(self->pin_b);
     reset_pin_number(self->pin_a);
     reset_pin_number(self->pin_b);
+
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+}
+
+void common_hal_rotaryio_incrementalencoder_mark_deinit(rotaryio_incrementalencoder_obj_t *self) {
     self->pin_a = NO_PIN;
-    self->pin_b = NO_PIN;
 }

ports/raspberrypi/bindings/rp2pio/StateMachine.h

@@ -42,6 +42,7 @@ void common_hal_rp2pio_statemachine_construct(rp2pio_statemachine_obj_t *self,
 
 void common_hal_rp2pio_statemachine_deinit(rp2pio_statemachine_obj_t *self);
 bool common_hal_rp2pio_statemachine_deinited(rp2pio_statemachine_obj_t *self);
+void common_hal_rp2pio_statemachine_mark_deinit(rp2pio_statemachine_obj_t *self);
 
 void common_hal_rp2pio_statemachine_never_reset(rp2pio_statemachine_obj_t *self);
 

ports/raspberrypi/common-hal/rotaryio/IncrementalEncoder.c

@@ -43,6 +43,9 @@ void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencode
     const mcu_pin_obj_t *pin_a, const mcu_pin_obj_t *pin_b) {
     const mcu_pin_obj_t *pins[] = { pin_a, pin_b };
 
+    // Ensure object starts in its deinit state.
+    common_hal_rotaryio_incrementalencoder_mark_deinit(self);
+
     // Start out with swapped to match behavior with other ports.
     self->swapped = true;
     if (!common_hal_rp2pio_pins_are_sequential(2, pins)) {
@@ -89,6 +92,7 @@ void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencode
 }
 
 bool common_hal_rotaryio_incrementalencoder_deinited(rotaryio_incrementalencoder_obj_t *self) {
+    // Use the deinit state of the PIO state machine.
     return common_hal_rp2pio_statemachine_deinited(&self->state_machine);
 }
 
@@ -100,6 +104,11 @@ void common_hal_rotaryio_incrementalencoder_deinit(rotaryio_incrementalencoder_o
     common_hal_rp2pio_statemachine_deinit(&self->state_machine);
 }
 
+void common_hal_rotaryio_incrementalencoder_mark_deinit(rotaryio_incrementalencoder_obj_t *self) {
+    // Use the deinit state of the PIO state machine.
+    common_hal_rp2pio_statemachine_mark_deinit(&self->state_machine);
+}
+
 static void incrementalencoder_interrupt_handler(void *self_in) {
     rotaryio_incrementalencoder_obj_t *self = self_in;
 

ports/raspberrypi/common-hal/rp2pio/StateMachine.c

@@ -628,6 +628,9 @@ void common_hal_rp2pio_statemachine_construct(rp2pio_statemachine_obj_t *self,
     int mov_status_type,
     int mov_status_n) {
 
+    // Ensure object starts in its deinit state.
+    common_hal_rp2pio_statemachine_mark_deinit(self);
+
     // First, check that all pins are free OR already in use by any PIO if exclusive_pin_use is false.
     pio_pinmask_t pins_we_use = wait_gpio_mask;
     PIO_PINMASK_MERGE(pins_we_use, _check_pins_free(first_out_pin, out_pin_count, exclusive_pin_use));
@@ -744,7 +747,7 @@ void common_hal_rp2pio_statemachine_construct(rp2pio_statemachine_obj_t *self,
         mov_status_type, mov_status_n);
     if (!ok) {
         // indicate state machine never inited
-        self->state_machine = NUM_PIO_STATE_MACHINES;
+        common_hal_rp2pio_statemachine_mark_deinit(self);
         mp_raise_RuntimeError(MP_ERROR_TEXT("All state machines in use"));
     }
 }
@@ -827,6 +830,10 @@ void common_hal_rp2pio_statemachine_deinit(rp2pio_statemachine_obj_t *self) {
     rp2pio_statemachine_deinit(self, false);
 }
 
+void common_hal_rp2pio_statemachine_mark_deinit(rp2pio_statemachine_obj_t *self) {
+    self->state_machine = NUM_PIO_STATE_MACHINES;
+}
+
 void common_hal_rp2pio_statemachine_never_reset(rp2pio_statemachine_obj_t *self) {
     rp2pio_statemachine_never_reset(self->pio, self->state_machine);
     // TODO: never reset all the pins

shared-bindings/rotaryio/IncrementalEncoder.h

@@ -13,11 +13,15 @@ extern const mp_obj_type_t rotaryio_incrementalencoder_type;
 
 extern void common_hal_rotaryio_incrementalencoder_construct(rotaryio_incrementalencoder_obj_t *self,
     const mcu_pin_obj_t *pin_a, const mcu_pin_obj_t *pin_b);
+
 extern void common_hal_rotaryio_incrementalencoder_deinit(rotaryio_incrementalencoder_obj_t *self);
 extern bool common_hal_rotaryio_incrementalencoder_deinited(rotaryio_incrementalencoder_obj_t *self);
+extern void common_hal_rotaryio_incrementalencoder_mark_deinit(rotaryio_incrementalencoder_obj_t *self);
+
 extern mp_int_t common_hal_rotaryio_incrementalencoder_get_position(rotaryio_incrementalencoder_obj_t *self);
 extern void common_hal_rotaryio_incrementalencoder_set_position(rotaryio_incrementalencoder_obj_t *self,
     mp_int_t new_position);
+
 extern mp_int_t common_hal_rotaryio_incrementalencoder_get_divisor(rotaryio_incrementalencoder_obj_t *self);
 extern void common_hal_rotaryio_incrementalencoder_set_divisor(rotaryio_incrementalencoder_obj_t *self,
     mp_int_t new_divisor);