Respond to review comments

Thanks @tannewt!

ports/atmel-samd/common-hal/_canio/Listener.c

@@ -375,7 +375,6 @@ bool common_hal_canio_listener_readinto(canio_listener_obj_t *self, canio_messag
 }
 
 void common_hal_canio_listener_deinit(canio_listener_obj_t *self) {
-    // free our FIFO, clear our matches, SOMETHING
     if (self->can) {
         clear_filters(self);
         if (self->fifo_idx == 0) {

shared-bindings/_canio/CAN.c

@@ -53,8 +53,8 @@
 //|         connected to a transceiver which controls the H and L pins on a shared
 //|         bus.
 //|
-//|         :param ~microcontrller.Pin rx: the pin to receive with.
-//|         :param ~microcontrller.Pin tx: the pin to transmit with, or None if the peripheral should operate in "silent" mode.
+//|         :param ~microcontroller.Pin rx: the pin to receive with.
+//|         :param ~microcontroller.Pin tx: the pin to transmit with, or None if the peripheral should operate in "silent" mode.
 //|         :param int baudrate: The bit rate of the bus in Hz.  All devices on the bus must agree on this value.
 //|         :param bool loopback: True if the peripheral will be operated in loopback mode.
 //|         :param bool auto_restart: If True, will restart communications after entering bus-off state
@@ -76,13 +76,8 @@ STATIC mp_obj_t canio_can_make_new(const mp_obj_type_t *type, size_t n_args, con
 
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
-mp_printf(&mp_plat_print, "ARG_rx=%d args[ARG_rx].u_obj=%p\n", ARG_rx, args[ARG_rx].u_obj);
-mp_printf(&mp_plat_print, "ARG_tx=%d args[ARG_tx].u_obj=%p\n", ARG_tx, args[ARG_tx].u_obj);
-
     mcu_pin_obj_t *rx_pin = validate_obj_is_free_pin(args[ARG_rx].u_obj);
-mp_printf(&mp_plat_print, "rx_pin=%p\n", rx_pin);
     mcu_pin_obj_t *tx_pin = validate_obj_is_free_pin_or_none(args[ARG_tx].u_obj);
-mp_printf(&mp_plat_print, "tx_pin=%p\n", tx_pin);
 
     canio_can_obj_t *self = m_new_obj(canio_can_obj_t);
     self->base.type = &canio_can_type;
@@ -94,7 +89,7 @@ mp_printf(&mp_plat_print, "tx_pin=%p\n", tx_pin);
 }
 
 
-//|     auto_restart: int
+//|     auto_restart: bool
 //|     """If True, will restart communications after entering bus-off state"""
 //|
 STATIC mp_obj_t canio_can_auto_restart_get(mp_obj_t self_in) {
@@ -240,12 +235,6 @@ STATIC const mp_obj_property_t canio_can_state_obj = {
 };
 
 
-#if 0
-//|     # pending_tx_count: int
-//|     # """The number of messages waiting to be transmitted. (read-only)"""
-//|
-#endif
-
 //|     def restart(self) -> None:
 //|         """If the device is in the bus off state, restart it."""
 //|         ...

shared-bindings/_canio/Listener.c

@@ -39,7 +39,7 @@
 //|
 
 //|     def read(self) -> Optional[Message]:
-//|         """Returns a message, after waiting up to self.timeout seconds
+//|         """Reads a message, after waiting up to self.timeout seconds
 //|
 //|         If no message is received in time, None is returned.  Otherwise,
 //|         a Message is returned."""
@@ -62,10 +62,8 @@ STATIC mp_obj_t canio_listener_read(mp_obj_t self_in) {
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(canio_listener_read_obj, canio_listener_read);
 
 //|     def readinto(self, message: Message) -> bool:
-//|         """Returns a message, after waiting up to self.timeout seconds
-//|
-//|         Returns True (and modifies message) if a message was received,
-//|         False otherwise."""
+//|         """Returns True (and modifies message) if a message was received
+//|         before ``timeout`` seconds elapsed, False otherwise."""
 //|         ...
 //|
 STATIC mp_obj_t canio_listener_readinto(mp_obj_t self_in, mp_obj_t message) {
@@ -115,7 +113,7 @@ STATIC mp_obj_t canio_listener_next(mp_obj_t self_in) {
     if (common_hal_canio_listener_in_waiting(self)) {
         return canio_listener_read(self_in);
     }
-    return self;
+    return MP_OBJ_STOP_ITERATION;
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(canio_listener_next_obj, canio_listener_next);
 

shared-bindings/_canio/Match.c

@@ -25,7 +25,6 @@
  */
 
 #include "shared-bindings/_canio/Match.h"
-#include "shared-module/_canio/Match.h"
 
 #include "py/objproperty.h"
 #include "py/runtime.h"
@@ -46,7 +45,7 @@
 STATIC mp_obj_t canio_match_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     enum { ARG_address, ARG_mask, ARG_extended, NUM_ARGS };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_address, MP_ARG_INT | MP_ARG_REQUIRED, {.u_int = 0} },
+        { MP_QSTR_address, MP_ARG_INT | MP_ARG_REQUIRED },
         { MP_QSTR_mask, MP_ARG_INT, {.u_int = 0} },
         { MP_QSTR_extended, MP_ARG_BOOL, {.u_bool = false} },
     };
@@ -79,7 +78,7 @@ STATIC mp_obj_t canio_match_make_new(const mp_obj_type_t *type, size_t n_args, c
 
 STATIC mp_obj_t canio_match_address_get(mp_obj_t self_in) {
     canio_match_obj_t *self = self_in;
-    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_address_get(self));
+    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_get_address(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_match_address_get_obj, canio_match_address_get);
 
@@ -97,7 +96,7 @@ const mp_obj_property_t canio_match_address_obj = {
 
 STATIC mp_obj_t canio_match_mask_get(mp_obj_t self_in) {
     canio_match_obj_t *self = self_in;
-    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_mask_get(self));
+    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_get_mask(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_match_mask_get_obj, canio_match_mask_get);
 
@@ -114,7 +113,7 @@ const mp_obj_property_t canio_match_mask_obj = {
 
 STATIC mp_obj_t canio_match_extended_get(mp_obj_t self_in) {
     canio_match_obj_t *self = self_in;
-    return mp_obj_new_bool(common_hal_canio_match_extended_get(self));
+    return mp_obj_new_bool(common_hal_canio_match_get_extended(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_match_extended_get_obj, canio_match_extended_get);
 

shared-bindings/_canio/Match.h

@@ -27,7 +27,11 @@
 #pragma once
 
 #include "py/obj.h"
+#include "shared-module/_canio/Match.h"
 
 extern const mp_obj_type_t canio_match_type;
 
-// Nothing now.
+void common_hal_canio_match_construct(canio_match_obj_t *self, int address, int mask, bool extended);
+int common_hal_canio_match_get_address(const canio_match_obj_t *self);
+int common_hal_canio_match_get_mask(const canio_match_obj_t *self);
+bool common_hal_canio_match_get_extended(const canio_match_obj_t *self);

shared-bindings/_canio/Message.c

@@ -25,7 +25,6 @@
  */
 
 #include "shared-bindings/_canio/Message.h"
-#include "shared-module/_canio/Message.h"
 
 #include "py/obj.h"
 #include "py/objproperty.h"
@@ -33,7 +32,7 @@
 
 //| class Message:
 //|     def __init__(self, id: int=0, data: Optional[bytes] = None, *, size: Optional[int] = None, rtr: bool = False, extended: bool = False):
-//|         """Construct a Message to send on a CAN bus
+//|         """Construct a Message to use with a CAN bus.  Provide arguments to create a message to send.  Otherwise, use Listener.readinto() to read a message.
 //|
 //|         :param int id: The numeric ID of the message
 //|         :param bytes data: The content of the message
@@ -97,13 +96,13 @@ STATIC mp_obj_t canio_message_make_new(const mp_obj_type_t *type, size_t n_args,
 //|
 STATIC mp_obj_t canio_message_id_get(const mp_obj_t self_in) {
     canio_message_obj_t *self = self_in;
-    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_id_get(self));
+    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_get_id(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_message_id_get_obj, canio_message_id_get);
 
 STATIC mp_obj_t canio_message_id_set(const mp_obj_t self_in, const mp_obj_t id) {
     canio_message_obj_t *self = self_in;
-    common_hal_canio_message_id_set(self, mp_obj_get_int(id));
+    common_hal_canio_message_set_id(self, mp_obj_get_int(id));
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(canio_message_id_set_obj, canio_message_id_set);
@@ -122,7 +121,7 @@ STATIC const mp_obj_property_t canio_message_id_obj = {
 //|
 STATIC mp_obj_t canio_message_data_get(const mp_obj_t self_in) {
     canio_message_obj_t *self = self_in;
-    return mp_obj_new_bytes((const byte*)common_hal_canio_message_data_get(self), common_hal_canio_message_size_get(self));
+    return mp_obj_new_bytes((const byte*)common_hal_canio_message_get_data(self), common_hal_canio_message_get_size(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_message_data_get_obj, canio_message_data_get);
 
@@ -133,7 +132,7 @@ STATIC mp_obj_t canio_message_data_set(const mp_obj_t self_in, const mp_obj_t da
     if (data.len > 8) {
         mp_raise_ValueError(translate("Messages limited to 8 bytes"));
     }
-    common_hal_canio_message_data_set(self, data.buf, data.len);
+    common_hal_canio_message_set_data(self, data.buf, data.len);
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(canio_message_data_set_obj, canio_message_data_set);
@@ -154,7 +153,7 @@ STATIC const mp_obj_property_t canio_message_data_obj = {
 //|
 STATIC mp_obj_t canio_message_size_get(const mp_obj_t self_in) {
     canio_message_obj_t *self = self_in;
-    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_size_get(self));
+    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_get_size(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_message_size_get_obj, canio_message_size_get);
 
@@ -164,7 +163,7 @@ STATIC mp_obj_t canio_message_size_set(const mp_obj_t self_in, const mp_obj_t si
     if (size > 8) {
         mp_raise_ValueError(translate("Messages limited to 8 bytes"));
     }
-    common_hal_canio_message_size_set(self, size);
+    common_hal_canio_message_set_size(self, size);
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(canio_message_size_set_obj, canio_message_size_set);
@@ -182,13 +181,13 @@ STATIC const mp_obj_property_t canio_message_size_obj = {
 //|
 STATIC mp_obj_t canio_message_extended_get(const mp_obj_t self_in) {
     canio_message_obj_t *self = self_in;
-    return mp_obj_new_bool(common_hal_canio_message_extended_get(self));
+    return mp_obj_new_bool(common_hal_canio_message_get_extended(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_message_extended_get_obj, canio_message_extended_get);
 
 STATIC mp_obj_t canio_message_extended_set(const mp_obj_t self_in, const mp_obj_t extended) {
     canio_message_obj_t *self = self_in;
-    common_hal_canio_message_size_set(self, mp_obj_is_true(extended));
+    common_hal_canio_message_set_extended(self, mp_obj_is_true(extended));
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(canio_message_extended_set_obj, canio_message_extended_set);
@@ -207,13 +206,13 @@ STATIC const mp_obj_property_t canio_message_extended_obj = {
 //|
 STATIC mp_obj_t canio_message_rtr_get(const mp_obj_t self_in) {
     canio_message_obj_t *self = self_in;
-    return mp_obj_new_bool(common_hal_canio_message_rtr_get(self));
+    return mp_obj_new_bool(common_hal_canio_message_get_rtr(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_message_rtr_get_obj, canio_message_rtr_get);
 
 STATIC mp_obj_t canio_message_rtr_set(const mp_obj_t self_in, const mp_obj_t rtr) {
     canio_message_obj_t *self = self_in;
-    common_hal_canio_message_size_set(self, mp_obj_is_true(rtr));
+    common_hal_canio_message_set_rtr(self, mp_obj_is_true(rtr));
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(canio_message_rtr_set_obj, canio_message_rtr_set);

shared-bindings/_canio/Message.h

@@ -27,5 +27,18 @@
 #pragma once
 
 #include "py/obj.h"
+#include "shared-module/_canio/Message.h"
 
 extern const mp_obj_type_t canio_message_type;
+
+void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void *data, size_t size, bool rtr, bool extended);
+const void *common_hal_canio_message_get_data(const canio_message_obj_t *self);
+void common_hal_canio_message_set_data(canio_message_obj_t *self, const void *data, size_t size);
+bool common_hal_canio_message_get_extended(const canio_message_obj_t *self);
+void common_hal_canio_message_set_extended(canio_message_obj_t *self, bool extended);
+int common_hal_canio_message_get_id(const canio_message_obj_t *self);
+void common_hal_canio_message_set_id(canio_message_obj_t *self, int id);
+bool common_hal_canio_message_get_rtr(const canio_message_obj_t *self);
+void common_hal_canio_message_set_rtr(canio_message_obj_t *self, bool rtr);
+size_t common_hal_canio_message_get_size(const canio_message_obj_t *self);
+void common_hal_canio_message_set_size(canio_message_obj_t *self, size_t size);

shared-module/_canio/Match.c

@@ -32,12 +32,12 @@ void common_hal_canio_match_construct(canio_match_obj_t *self, int address, int
     self->extended = extended;
 }
 
-int common_hal_canio_match_address_get(const canio_match_obj_t *self) {
+int common_hal_canio_match_get_address(const canio_match_obj_t *self) {
     return self->address;
 }
-int common_hal_canio_match_mask_get(const canio_match_obj_t *self) {
+int common_hal_canio_match_get_mask(const canio_match_obj_t *self) {
     return self->mask;
 }
-bool common_hal_canio_match_extended_get(const canio_match_obj_t *self) {
+bool common_hal_canio_match_get_extended(const canio_match_obj_t *self) {
     return self->extended;
 }

shared-module/_canio/Match.h

@@ -34,8 +34,3 @@ typedef struct {
     int mask;
     bool extended;
 } canio_match_obj_t;
-
-void common_hal_canio_match_construct(canio_match_obj_t *self, int address, int mask, bool extended);
-int common_hal_canio_match_address_get(const canio_match_obj_t *self);
-int common_hal_canio_match_mask_get(const canio_match_obj_t *self);
-bool common_hal_canio_match_extended_get(const canio_match_obj_t *self);

shared-module/_canio/Message.c

@@ -41,57 +41,57 @@ void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void
     }
 }
 
-int common_hal_canio_message_id_get(const canio_message_obj_t *self)
+int common_hal_canio_message_get_id(const canio_message_obj_t *self)
 {
     return self->id;
 }
 
-void common_hal_canio_message_id_set(canio_message_obj_t *self, int id)
+void common_hal_canio_message_set_id(canio_message_obj_t *self, int id)
 {
     self->id = id;
 }
 
 
-const void *common_hal_canio_message_data_get(const canio_message_obj_t *self)
+const void *common_hal_canio_message_get_data(const canio_message_obj_t *self)
 {
     return self->data;
 }
 
-const void common_hal_canio_message_data_set(canio_message_obj_t *self, const void *data, size_t size)
+const void common_hal_canio_message_set_data(canio_message_obj_t *self, const void *data, size_t size)
 {
     self->size = size;
     memcpy(self->data, data, size);
 }
 
 
-size_t common_hal_canio_message_size_get(const canio_message_obj_t *self)
+size_t common_hal_canio_message_get_size(const canio_message_obj_t *self)
 {
     return self->size;
 }
 
-void common_hal_canio_message_size_set(canio_message_obj_t *self, size_t size)
+void common_hal_canio_message_set_size(canio_message_obj_t *self, size_t size)
 {
     memset(self->data, 0, size);
     self->size = size;
 }
 
 
-bool common_hal_canio_message_rtr_get(const canio_message_obj_t *self)
+bool common_hal_canio_message_get_rtr(const canio_message_obj_t *self)
 {
     return self->rtr;
 }
 
-void common_hal_canio_message_rtr_set(canio_message_obj_t *self, bool rtr)
+void common_hal_canio_message_set_rtr(canio_message_obj_t *self, bool rtr)
 {
     self->rtr = rtr;
 }
 
-bool common_hal_canio_message_extended_get(const canio_message_obj_t *self)
+bool common_hal_canio_message_get_extended(const canio_message_obj_t *self)
 {
     return self->extended;
 }
 
-void common_hal_canio_message_extended_set(canio_message_obj_t *self, bool extended)
+void common_hal_canio_message_set_extended(canio_message_obj_t *self, bool extended)
 {
     self->extended = extended;
 }

shared-module/_canio/Message.h

@@ -36,15 +36,3 @@ typedef struct {
     bool rtr:1;
     bool extended:1;
 } canio_message_obj_t;
-
-void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void *data, size_t size, bool rtr, bool extended);
-bool common_hal_canio_message_rtr_get(const canio_message_obj_t *self);
-bool common_hal_canio_message_extended_get(const canio_message_obj_t *self);
-int common_hal_canio_message_id_get(const canio_message_obj_t *self);
-const void *common_hal_canio_message_data_get(const canio_message_obj_t *self);
-size_t common_hal_canio_message_size_get(const canio_message_obj_t *self);
-void common_hal_canio_message_rtr_set(canio_message_obj_t *self, bool rtr);
-void common_hal_canio_message_extended_set(canio_message_obj_t *self, bool extended);
-void common_hal_canio_message_id_set(canio_message_obj_t *self, int id);
-void common_hal_canio_message_data_set(canio_message_obj_t *self, const void *data, size_t size);
-void common_hal_canio_message_size_set(canio_message_obj_t *self, size_t size);