canio: Split RemoteTransmissionRequest to its own class

It reuses most of canio.Message's implementation, and structure

Author: jepler

shared-bindings/canio/CAN.c

@@ -265,7 +265,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(canio_can_restart_obj, canio_can_restart);
 //|
 //|         An empty filter list causes all messages to be accepted.
 //|
-//|         Timeout dictates how long readinto, read and next() will block."""
+//|         Timeout dictates how long receive() and next() will block."""
 //|         ...
 //|
 STATIC mp_obj_t canio_can_listen(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
@@ -334,8 +334,8 @@ STATIC mp_obj_t canio_can_send(mp_obj_t self_in, mp_obj_t message_in) {
     canio_can_obj_t *self = MP_OBJ_TO_PTR(self_in);
     common_hal_canio_can_check_for_deinit(self);
     mp_obj_type_t *message_type = mp_obj_get_type(message_in);
-    if (message_type != &canio_message_type) {
-        mp_raise_TypeError_varg(translate("expected '%q' but got '%q'"), MP_QSTR_Message, message_type->name);
+    if (message_type != &canio_message_type && message_type != &canio_remote_transmission_request_type) {
+        mp_raise_TypeError_varg(translate("expected '%q' or '%q' but got '%q'"), MP_QSTR_Message, MP_QSTR_RemoteTransmissionRequest, message_type->name);
     }
 
     canio_message_obj_t *message = message_in;

shared-bindings/canio/Listener.c

@@ -50,9 +50,13 @@ STATIC mp_obj_t canio_listener_receive(mp_obj_t self_in) {
     common_hal_canio_listener_check_for_deinit(self);
 
     canio_message_obj_t *message = m_new_obj(canio_message_obj_t);
-    message->base.type = &canio_message_type;
 
     if (common_hal_canio_listener_receiveinto(self, message)) {
+        if (message->rtr) {
+            message->base.type = &canio_remote_transmission_request_type;
+        } else {
+            message->base.type = &canio_message_type;
+        }
         return message;
     } else {
         m_free(message); // message did not escape into vm

shared-bindings/canio/Message.c

@@ -31,63 +31,39 @@
 #include "py/runtime.h"
 
 //| 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 use with a CAN bus.  Provide arguments to create a message to send.  Otherwise, use Listener.readinto() to read a message.
+//|     def __init__(self, id: int, data: bytes, *, extended: bool = False):
+//|         """Construct a Message to send on a CAN bus.
 //|
 //|         :param int id: The numeric ID of the message
 //|         :param bytes data: The content of the message
-//|         :param int size: The amount of data requested, for an rtr
-//|         :param bool rtr: True if the message represents an rtr (Remote Transmission Request)
 //|         :param bool extended: True if the message has an extended identifier, False if it has a standard identifier
 //|
-//|         In CAN, messages can have a size from 0 to 8 bytes.
-//|
-//|         For a non-rtr message, specify ``data``.  For an rtr-message, specify either ``data`` (a dummy buffer of the requested size) or ``size``.
+//|         In CAN, messages can have a length from 0 to 8 bytes.
 //|         """
 //|         ...
 //|
 STATIC mp_obj_t canio_message_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_id, ARG_data, ARG_size, ARG_rtr, ARG_extended, NUM_ARGS };
+    enum { ARG_id, ARG_data, ARG_extended, NUM_ARGS };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_id, MP_ARG_INT, {.u_obj = 0} },
-        { MP_QSTR_data, MP_ARG_OBJ, {.u_obj = 0} },
-        { MP_QSTR_size, MP_ARG_INT, {.u_int = -1} },
-        { MP_QSTR_rtr, MP_ARG_BOOL, {.u_bool = false} },
+        { MP_QSTR_id, MP_ARG_INT | MP_ARG_REQUIRED },
+        { MP_QSTR_data, MP_ARG_OBJ | MP_ARG_REQUIRED },
         { MP_QSTR_extended, MP_ARG_BOOL, {.u_bool = false} },
     };
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     MP_STATIC_ASSERT( MP_ARRAY_SIZE(allowed_args) == NUM_ARGS );
 
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
-    bool rtr = args[ARG_rtr].u_bool;
-    bool extended = args[ARG_extended].u_bool;
-    size_t size = (size_t)args[ARG_size].u_int;
-    bool specified_size = (size != (size_t)-1);
-    bool specified_data = (args[ARG_data].u_obj != NULL);
-
-    if (specified_size && specified_data) {
-        mp_raise_TypeError(translate("specify size or data, but not both"));
-    }
-
     mp_buffer_info_t data;
-    if (specified_data) {
-        mp_get_buffer_raise(args[ARG_data].u_obj, &data, MP_BUFFER_READ);
-    } else if (specified_size) {
-        data.buf = 0;
-        data.len = size;
-    } else {
-        data.buf = 0;
-        data.len = 0;
-    }
+    mp_get_buffer_raise(args[ARG_data].u_obj, &data, MP_BUFFER_READ);
 
     if (data.len > 8) {
         mp_raise_ValueError(translate("Messages limited to 8 bytes"));
     }
 
     canio_message_obj_t *self = m_new_obj(canio_message_obj_t);
     self->base.type = &canio_message_type;
-    common_hal_canio_message_construct(self, args[ARG_id].u_int, data.buf, data.len, rtr, extended);
+    common_hal_canio_message_construct(self, args[ARG_id].u_int, data.buf, data.len, args[ARG_extended].u_bool);
     return self;
 }
 
@@ -115,13 +91,11 @@ STATIC const mp_obj_property_t canio_message_id_obj = {
 };
 
 //|     data: bytes
-//|     """The content of the message, or dummy content in the case of an rtr.
-//|
-//|     Assigning to data also clears the rtr flag, if it was set."""
+//|     """The content of the message"""
 //|
 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_get_data(self), common_hal_canio_message_get_size(self));
+    return mp_obj_new_bytes((const byte*)common_hal_canio_message_get_data(self), common_hal_canio_message_get_length(self));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(canio_message_data_get_obj, canio_message_data_get);
 
@@ -147,7 +121,7 @@ STATIC const mp_obj_property_t canio_message_data_obj = {
 
 
 //|     extended: bool
-//|     """True if the message represents a remote transmission request (RTR)"""
+//|     """True if the message's id is an extended id"""
 //|
 STATIC mp_obj_t canio_message_extended_get(const mp_obj_t self_in) {
     canio_message_obj_t *self = self_in;
@@ -170,36 +144,80 @@ STATIC const mp_obj_property_t canio_message_extended_obj = {
               (mp_obj_t)&mp_const_none_obj},
 };
 
+//| class RemoteTransmissionRequest:
+//|     def __init__(self, id: int, length: int, *, extended: bool = False):
+//|         """Construct a Message to send on a CAN bus.
+//|
+//|         :param int id: The numeric ID of the requested message
+//|         :param int length: The length of the requested message
+//|         :param bool extended: True if the message has an extended identifier, False if it has a standard identifier
+//|
+//|         In CAN, messages can have a length from 0 to 8 bytes.
+//|         """
+//|         ...
+//|
+STATIC mp_obj_t canio_remote_transmission_request_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_id, ARG_length, ARG_extended, NUM_ARGS };
+    static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_id, MP_ARG_INT | MP_ARG_REQUIRED },
+        { MP_QSTR_length, MP_ARG_INT | MP_ARG_REQUIRED },
+        { MP_QSTR_extended, MP_ARG_BOOL, {.u_bool = false} },
+    };
+    mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
+    MP_STATIC_ASSERT( MP_ARRAY_SIZE(allowed_args) == NUM_ARGS );
+
+    mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
+
+    int length = args[ARG_length].u_int;
+    if (length < 0 || length > 8) {
+        mp_raise_ValueError(translate("Messages limited to 8 bytes"));
+    }
 
-//|     rtr: bool
-//|     """True if the message represents a remote transmission request (RTR).  Setting rtr to true zeros out data"""
+    canio_message_obj_t *self = m_new_obj(canio_message_obj_t);
+    self->base.type = &canio_remote_transmission_request_type;
+    common_hal_canio_message_construct(self, args[ARG_id].u_int, NULL, length, args[ARG_extended].u_bool);
+    return self;
+}
+
+//|     extended: bool
+//|     """True if the message's id is an extended id"""
+//|
+
+//|     id: int
+//|     """The numeric ID of the message"""
+//|
+
+//|     length: int
+//|     """The length of the requested message."""
 //|
-STATIC mp_obj_t canio_message_rtr_get(const mp_obj_t self_in) {
+STATIC mp_obj_t canio_remote_transmission_request_length_get(const mp_obj_t self_in) {
     canio_message_obj_t *self = self_in;
-    return mp_obj_new_bool(common_hal_canio_message_get_rtr(self));
+    return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_get_length(self));
 }
-MP_DEFINE_CONST_FUN_OBJ_1(canio_message_rtr_get_obj, canio_message_rtr_get);
+MP_DEFINE_CONST_FUN_OBJ_1(canio_remote_transmission_request_length_get_obj, canio_remote_transmission_request_length_get);
 
-STATIC mp_obj_t canio_message_rtr_set(const mp_obj_t self_in, const mp_obj_t rtr) {
+STATIC mp_obj_t canio_remote_transmission_request_length_set(const mp_obj_t self_in, const mp_obj_t length_in) {
     canio_message_obj_t *self = self_in;
-    common_hal_canio_message_set_rtr(self, mp_obj_is_true(rtr));
+    int length = mp_obj_get_int(length_in);
+    if (length < 0 || length > 8) {
+        mp_raise_ValueError(translate("Messages limited to 8 bytes"));
+    }
+    common_hal_canio_remote_transmission_request_set_length(self, length);
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_2(canio_message_rtr_set_obj, canio_message_rtr_set);
+MP_DEFINE_CONST_FUN_OBJ_2(canio_remote_transmission_request_length_set_obj, canio_remote_transmission_request_length_set);
 
 
-STATIC const mp_obj_property_t canio_message_rtr_obj = {
+STATIC const mp_obj_property_t canio_remote_transmission_request_length_obj = {
     .base.type = &mp_type_property,
-    .proxy = {(mp_obj_t)&canio_message_rtr_get_obj,
-              (mp_obj_t)&canio_message_rtr_set_obj,
+    .proxy = {(mp_obj_t)&canio_remote_transmission_request_length_get_obj,
+              (mp_obj_t)&canio_remote_transmission_request_length_set_obj,
               (mp_obj_t)&mp_const_none_obj},
 };
 
-
 STATIC const mp_rom_map_elem_t canio_message_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_id), MP_ROM_PTR(&canio_message_id_obj) },
     { MP_ROM_QSTR(MP_QSTR_data), MP_ROM_PTR(&canio_message_data_obj) },
-    { MP_ROM_QSTR(MP_QSTR_rtr), MP_ROM_PTR(&canio_message_rtr_obj) },
     { MP_ROM_QSTR(MP_QSTR_extended), MP_ROM_PTR(&canio_message_extended_obj) },
 };
 STATIC MP_DEFINE_CONST_DICT(canio_message_locals_dict, canio_message_locals_dict_table);
@@ -210,3 +228,17 @@ const mp_obj_type_t canio_message_type = {
     .make_new = canio_message_make_new,
     .locals_dict = (mp_obj_t)&canio_message_locals_dict,
 };
+
+STATIC const mp_rom_map_elem_t canio_remote_transmission_request_locals_dict_table[] = {
+    { MP_ROM_QSTR(MP_QSTR_id), MP_ROM_PTR(&canio_message_id_obj) },
+    { MP_ROM_QSTR(MP_QSTR_length), MP_ROM_PTR(&canio_remote_transmission_request_length_obj) },
+    { MP_ROM_QSTR(MP_QSTR_extended), MP_ROM_PTR(&canio_message_extended_obj) },
+};
+STATIC MP_DEFINE_CONST_DICT(canio_remote_transmission_request_locals_dict, canio_remote_transmission_request_locals_dict_table);
+
+const mp_obj_type_t canio_remote_transmission_request_type = {
+    { &mp_type_type },
+    .name = MP_QSTR_RemoteTransmissionRequest,
+    .make_new = canio_remote_transmission_request_make_new,
+    .locals_dict = (mp_obj_t)&canio_remote_transmission_request_locals_dict,
+};

shared-bindings/canio/Message.h

@@ -30,8 +30,9 @@
 #include "shared-module/canio/Message.h"
 
 extern const mp_obj_type_t canio_message_type;
+extern const mp_obj_type_t canio_remote_transmission_request_type;
 
-void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void *data, size_t size, bool rtr, bool extended);
+void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void *data, size_t size, 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);
@@ -40,5 +41,5 @@ 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);
+size_t common_hal_canio_message_get_length(const canio_message_obj_t *self);
+void common_hal_canio_remote_transmission_request_set_length(canio_message_obj_t *self, size_t length);

shared-bindings/canio/__init__.c

@@ -113,6 +113,7 @@ STATIC const mp_rom_map_elem_t canio_module_globals_table[] = {
     { MP_ROM_QSTR(MP_QSTR_Listener), MP_ROM_PTR(&canio_listener_type) },
     { MP_ROM_QSTR(MP_QSTR_Match), MP_ROM_PTR(&canio_match_type) },
     { MP_ROM_QSTR(MP_QSTR_Message), MP_ROM_PTR(&canio_message_type) },
+    { MP_ROM_QSTR(MP_QSTR_RemoteTransmissionRequest), MP_ROM_PTR(&canio_remote_transmission_request_type) },
     { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR__canio) },
 };
 

shared-module/canio/Message.c

@@ -28,16 +28,14 @@
 
 #include <string.h>
 
-void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void *data, size_t size, bool rtr, bool extended)
+void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void *data, size_t size, bool extended)
 {
     self->id = id;
     self->size = size;
-    self->rtr = rtr;
+    self->rtr = !data;
     self->extended = extended;
     if (data) {
         memcpy(self->data, data, size);
-    } else {
-        memset(self->data, 0, size);
     }
 }
 
@@ -65,12 +63,12 @@ const void common_hal_canio_message_set_data(canio_message_obj_t *self, const vo
 }
 
 
-size_t common_hal_canio_message_get_size(const canio_message_obj_t *self)
+size_t common_hal_canio_message_get_length(const canio_message_obj_t *self)
 {
     return self->size;
 }
 
-void common_hal_canio_message_set_size(canio_message_obj_t *self, size_t size)
+void common_hal_canio_remote_transmission_request_set_length(canio_message_obj_t *self, size_t size)
 {
     memset(self->data, 0, size);
     self->size = size;