Skip to content

Commit 44c5b2b

Browse files
jeplerjepler
committed
Respond to review comments
Thanks @tannewt!
1 parent a69b298 commit 44c5b2b

File tree

11 files changed

+53
-69
lines changed

11 files changed

+53
-69
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,6 @@ bool common_hal_canio_listener_readinto(canio_listener_obj_t *self, canio_messag
375375
}
376376

377377
void common_hal_canio_listener_deinit(canio_listener_obj_t *self) {
378-
// free our FIFO, clear our matches, SOMETHING
379378
if (self->can) {
380379
clear_filters(self);
381380
if (self->fifo_idx == 0) {

‎shared-bindings/_canio/CAN.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@
5353
//| connected to a transceiver which controls the H and L pins on a shared
5454
//| bus.
5555
//|
56-
//| :param ~microcontrller.Pin rx: the pin to receive with.
57-
//| :param ~microcontrller.Pin tx: the pin to transmit with, or None if the peripheral should operate in "silent" mode.
56+
//| :param ~microcontroller.Pin rx: the pin to receive with.
57+
//| :param ~microcontroller.Pin tx: the pin to transmit with, or None if the peripheral should operate in "silent" mode.
5858
//| :param int baudrate: The bit rate of the bus in Hz. All devices on the bus must agree on this value.
5959
//| :param bool loopback: True if the peripheral will be operated in loopback mode.
6060
//| :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
7676

7777
mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
7878

79-
mp_printf(&mp_plat_print, "ARG_rx=%d args[ARG_rx].u_obj=%p\n", ARG_rx, args[ARG_rx].u_obj);
80-
mp_printf(&mp_plat_print, "ARG_tx=%d args[ARG_tx].u_obj=%p\n", ARG_tx, args[ARG_tx].u_obj);
81-
8279
mcu_pin_obj_t *rx_pin = validate_obj_is_free_pin(args[ARG_rx].u_obj);
83-
mp_printf(&mp_plat_print, "rx_pin=%p\n", rx_pin);
8480
mcu_pin_obj_t *tx_pin = validate_obj_is_free_pin_or_none(args[ARG_tx].u_obj);
85-
mp_printf(&mp_plat_print, "tx_pin=%p\n", tx_pin);
8681

8782
canio_can_obj_t *self = m_new_obj(canio_can_obj_t);
8883
self->base.type = &canio_can_type;
@@ -94,7 +89,7 @@ mp_printf(&mp_plat_print, "tx_pin=%p\n", tx_pin);
9489
}
9590

9691

97-
//| auto_restart: int
92+
//| auto_restart: bool
9893
//| """If True, will restart communications after entering bus-off state"""
9994
//|
10095
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 = {
240235
};
241236

242237

243-
#if 0
244-
//| # pending_tx_count: int
245-
//| # """The number of messages waiting to be transmitted. (read-only)"""
246-
//|
247-
#endif
248-
249238
//| def restart(self) -> None:
250239
//| """If the device is in the bus off state, restart it."""
251240
//| ...

‎shared-bindings/_canio/Listener.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
//|
4040

4141
//| def read(self) -> Optional[Message]:
42-
//| """Returns a message, after waiting up to self.timeout seconds
42+
//| """Reads a message, after waiting up to self.timeout seconds
4343
//|
4444
//| If no message is received in time, None is returned. Otherwise,
4545
//| a Message is returned."""
@@ -62,10 +62,8 @@ STATIC mp_obj_t canio_listener_read(mp_obj_t self_in) {
6262
STATIC MP_DEFINE_CONST_FUN_OBJ_1(canio_listener_read_obj, canio_listener_read);
6363

6464
//| def readinto(self, message: Message) -> bool:
65-
//| """Returns a message, after waiting up to self.timeout seconds
66-
//|
67-
//| Returns True (and modifies message) if a message was received,
68-
//| False otherwise."""
65+
//| """Returns True (and modifies message) if a message was received
66+
//| before ``timeout`` seconds elapsed, False otherwise."""
6967
//| ...
7068
//|
7169
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) {
115113
if (common_hal_canio_listener_in_waiting(self)) {
116114
return canio_listener_read(self_in);
117115
}
118-
return self;
116+
return MP_OBJ_STOP_ITERATION;
119117
}
120118
STATIC MP_DEFINE_CONST_FUN_OBJ_1(canio_listener_next_obj, canio_listener_next);
121119

‎shared-bindings/_canio/Match.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
*/
2626

2727
#include "shared-bindings/_canio/Match.h"
28-
#include "shared-module/_canio/Match.h"
2928

3029
#include "py/objproperty.h"
3130
#include "py/runtime.h"
@@ -46,7 +45,7 @@
4645
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) {
4746
enum { ARG_address, ARG_mask, ARG_extended, NUM_ARGS };
4847
static const mp_arg_t allowed_args[] = {
49-
{ MP_QSTR_address, MP_ARG_INT | MP_ARG_REQUIRED, {.u_int = 0} },
48+
{ MP_QSTR_address, MP_ARG_INT | MP_ARG_REQUIRED },
5049
{ MP_QSTR_mask, MP_ARG_INT, {.u_int = 0} },
5150
{ MP_QSTR_extended, MP_ARG_BOOL, {.u_bool = false} },
5251
};
@@ -79,7 +78,7 @@ STATIC mp_obj_t canio_match_make_new(const mp_obj_type_t *type, size_t n_args, c
7978

8079
STATIC mp_obj_t canio_match_address_get(mp_obj_t self_in) {
8180
canio_match_obj_t *self = self_in;
82-
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_address_get(self));
81+
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_get_address(self));
8382
}
8483
MP_DEFINE_CONST_FUN_OBJ_1(canio_match_address_get_obj, canio_match_address_get);
8584

@@ -97,7 +96,7 @@ const mp_obj_property_t canio_match_address_obj = {
9796

9897
STATIC mp_obj_t canio_match_mask_get(mp_obj_t self_in) {
9998
canio_match_obj_t *self = self_in;
100-
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_mask_get(self));
99+
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_match_get_mask(self));
101100
}
102101
MP_DEFINE_CONST_FUN_OBJ_1(canio_match_mask_get_obj, canio_match_mask_get);
103102

@@ -114,7 +113,7 @@ const mp_obj_property_t canio_match_mask_obj = {
114113

115114
STATIC mp_obj_t canio_match_extended_get(mp_obj_t self_in) {
116115
canio_match_obj_t *self = self_in;
117-
return mp_obj_new_bool(common_hal_canio_match_extended_get(self));
116+
return mp_obj_new_bool(common_hal_canio_match_get_extended(self));
118117
}
119118
MP_DEFINE_CONST_FUN_OBJ_1(canio_match_extended_get_obj, canio_match_extended_get);
120119

‎shared-bindings/_canio/Match.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@
2727
#pragma once
2828

2929
#include "py/obj.h"
30+
#include "shared-module/_canio/Match.h"
3031

3132
extern const mp_obj_type_t canio_match_type;
3233

33-
// Nothing now.
34+
void common_hal_canio_match_construct(canio_match_obj_t *self, int address, int mask, bool extended);
35+
int common_hal_canio_match_get_address(const canio_match_obj_t *self);
36+
int common_hal_canio_match_get_mask(const canio_match_obj_t *self);
37+
bool common_hal_canio_match_get_extended(const canio_match_obj_t *self);

‎shared-bindings/_canio/Message.c

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,14 @@
2525
*/
2626

2727
#include "shared-bindings/_canio/Message.h"
28-
#include "shared-module/_canio/Message.h"
2928

3029
#include "py/obj.h"
3130
#include "py/objproperty.h"
3231
#include "py/runtime.h"
3332

3433
//| class Message:
3534
//| def __init__(self, id: int=0, data: Optional[bytes] = None, *, size: Optional[int] = None, rtr: bool = False, extended: bool = False):
36-
//| """Construct a Message to send on a CAN bus
35+
//| """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.
3736
//|
3837
//| :param int id: The numeric ID of the message
3938
//| :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,
9796
//|
9897
STATIC mp_obj_t canio_message_id_get(const mp_obj_t self_in) {
9998
canio_message_obj_t *self = self_in;
100-
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_id_get(self));
99+
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_get_id(self));
101100
}
102101
MP_DEFINE_CONST_FUN_OBJ_1(canio_message_id_get_obj, canio_message_id_get);
103102

104103
STATIC mp_obj_t canio_message_id_set(const mp_obj_t self_in, const mp_obj_t id) {
105104
canio_message_obj_t *self = self_in;
106-
common_hal_canio_message_id_set(self, mp_obj_get_int(id));
105+
common_hal_canio_message_set_id(self, mp_obj_get_int(id));
107106
return mp_const_none;
108107
}
109108
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 = {
122121
//|
123122
STATIC mp_obj_t canio_message_data_get(const mp_obj_t self_in) {
124123
canio_message_obj_t *self = self_in;
125-
return mp_obj_new_bytes((const byte*)common_hal_canio_message_data_get(self), common_hal_canio_message_size_get(self));
124+
return mp_obj_new_bytes((const byte*)common_hal_canio_message_get_data(self), common_hal_canio_message_get_size(self));
126125
}
127126
MP_DEFINE_CONST_FUN_OBJ_1(canio_message_data_get_obj, canio_message_data_get);
128127

@@ -133,7 +132,7 @@ STATIC mp_obj_t canio_message_data_set(const mp_obj_t self_in, const mp_obj_t da
133132
if (data.len > 8) {
134133
mp_raise_ValueError(translate("Messages limited to 8 bytes"));
135134
}
136-
common_hal_canio_message_data_set(self, data.buf, data.len);
135+
common_hal_canio_message_set_data(self, data.buf, data.len);
137136
return mp_const_none;
138137
}
139138
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 = {
154153
//|
155154
STATIC mp_obj_t canio_message_size_get(const mp_obj_t self_in) {
156155
canio_message_obj_t *self = self_in;
157-
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_size_get(self));
156+
return MP_OBJ_NEW_SMALL_INT(common_hal_canio_message_get_size(self));
158157
}
159158
MP_DEFINE_CONST_FUN_OBJ_1(canio_message_size_get_obj, canio_message_size_get);
160159

@@ -164,7 +163,7 @@ STATIC mp_obj_t canio_message_size_set(const mp_obj_t self_in, const mp_obj_t si
164163
if (size > 8) {
165164
mp_raise_ValueError(translate("Messages limited to 8 bytes"));
166165
}
167-
common_hal_canio_message_size_set(self, size);
166+
common_hal_canio_message_set_size(self, size);
168167
return mp_const_none;
169168
}
170169
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 = {
182181
//|
183182
STATIC mp_obj_t canio_message_extended_get(const mp_obj_t self_in) {
184183
canio_message_obj_t *self = self_in;
185-
return mp_obj_new_bool(common_hal_canio_message_extended_get(self));
184+
return mp_obj_new_bool(common_hal_canio_message_get_extended(self));
186185
}
187186
MP_DEFINE_CONST_FUN_OBJ_1(canio_message_extended_get_obj, canio_message_extended_get);
188187

189188
STATIC mp_obj_t canio_message_extended_set(const mp_obj_t self_in, const mp_obj_t extended) {
190189
canio_message_obj_t *self = self_in;
191-
common_hal_canio_message_size_set(self, mp_obj_is_true(extended));
190+
common_hal_canio_message_set_extended(self, mp_obj_is_true(extended));
192191
return mp_const_none;
193192
}
194193
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 = {
207206
//|
208207
STATIC mp_obj_t canio_message_rtr_get(const mp_obj_t self_in) {
209208
canio_message_obj_t *self = self_in;
210-
return mp_obj_new_bool(common_hal_canio_message_rtr_get(self));
209+
return mp_obj_new_bool(common_hal_canio_message_get_rtr(self));
211210
}
212211
MP_DEFINE_CONST_FUN_OBJ_1(canio_message_rtr_get_obj, canio_message_rtr_get);
213212

214213
STATIC mp_obj_t canio_message_rtr_set(const mp_obj_t self_in, const mp_obj_t rtr) {
215214
canio_message_obj_t *self = self_in;
216-
common_hal_canio_message_size_set(self, mp_obj_is_true(rtr));
215+
common_hal_canio_message_set_rtr(self, mp_obj_is_true(rtr));
217216
return mp_const_none;
218217
}
219218
MP_DEFINE_CONST_FUN_OBJ_2(canio_message_rtr_set_obj, canio_message_rtr_set);

‎shared-bindings/_canio/Message.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,18 @@
2727
#pragma once
2828

2929
#include "py/obj.h"
30+
#include "shared-module/_canio/Message.h"
3031

3132
extern const mp_obj_type_t canio_message_type;
33+
34+
void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void *data, size_t size, bool rtr, bool extended);
35+
const void *common_hal_canio_message_get_data(const canio_message_obj_t *self);
36+
void common_hal_canio_message_set_data(canio_message_obj_t *self, const void *data, size_t size);
37+
bool common_hal_canio_message_get_extended(const canio_message_obj_t *self);
38+
void common_hal_canio_message_set_extended(canio_message_obj_t *self, bool extended);
39+
int common_hal_canio_message_get_id(const canio_message_obj_t *self);
40+
void common_hal_canio_message_set_id(canio_message_obj_t *self, int id);
41+
bool common_hal_canio_message_get_rtr(const canio_message_obj_t *self);
42+
void common_hal_canio_message_set_rtr(canio_message_obj_t *self, bool rtr);
43+
size_t common_hal_canio_message_get_size(const canio_message_obj_t *self);
44+
void common_hal_canio_message_set_size(canio_message_obj_t *self, size_t size);

‎shared-module/_canio/Match.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ void common_hal_canio_match_construct(canio_match_obj_t *self, int address, int
3232
self->extended = extended;
3333
}
3434

35-
int common_hal_canio_match_address_get(const canio_match_obj_t *self) {
35+
int common_hal_canio_match_get_address(const canio_match_obj_t *self) {
3636
return self->address;
3737
}
38-
int common_hal_canio_match_mask_get(const canio_match_obj_t *self) {
38+
int common_hal_canio_match_get_mask(const canio_match_obj_t *self) {
3939
return self->mask;
4040
}
41-
bool common_hal_canio_match_extended_get(const canio_match_obj_t *self) {
41+
bool common_hal_canio_match_get_extended(const canio_match_obj_t *self) {
4242
return self->extended;
4343
}

‎shared-module/_canio/Match.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,3 @@ typedef struct {
3434
int mask;
3535
bool extended;
3636
} canio_match_obj_t;
37-
38-
void common_hal_canio_match_construct(canio_match_obj_t *self, int address, int mask, bool extended);
39-
int common_hal_canio_match_address_get(const canio_match_obj_t *self);
40-
int common_hal_canio_match_mask_get(const canio_match_obj_t *self);
41-
bool common_hal_canio_match_extended_get(const canio_match_obj_t *self);

‎shared-module/_canio/Message.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,57 +41,57 @@ void common_hal_canio_message_construct(canio_message_obj_t *self, int id, void
4141
}
4242
}
4343

44-
int common_hal_canio_message_id_get(const canio_message_obj_t *self)
44+
int common_hal_canio_message_get_id(const canio_message_obj_t *self)
4545
{
4646
return self->id;
4747
}
4848

49-
void common_hal_canio_message_id_set(canio_message_obj_t *self, int id)
49+
void common_hal_canio_message_set_id(canio_message_obj_t *self, int id)
5050
{
5151
self->id = id;
5252
}
5353

5454

55-
const void *common_hal_canio_message_data_get(const canio_message_obj_t *self)
55+
const void *common_hal_canio_message_get_data(const canio_message_obj_t *self)
5656
{
5757
return self->data;
5858
}
5959

60-
const void common_hal_canio_message_data_set(canio_message_obj_t *self, const void *data, size_t size)
60+
const void common_hal_canio_message_set_data(canio_message_obj_t *self, const void *data, size_t size)
6161
{
6262
self->size = size;
6363
memcpy(self->data, data, size);
6464
}
6565

6666

67-
size_t common_hal_canio_message_size_get(const canio_message_obj_t *self)
67+
size_t common_hal_canio_message_get_size(const canio_message_obj_t *self)
6868
{
6969
return self->size;
7070
}
7171

72-
void common_hal_canio_message_size_set(canio_message_obj_t *self, size_t size)
72+
void common_hal_canio_message_set_size(canio_message_obj_t *self, size_t size)
7373
{
7474
memset(self->data, 0, size);
7575
self->size = size;
7676
}
7777

7878

79-
bool common_hal_canio_message_rtr_get(const canio_message_obj_t *self)
79+
bool common_hal_canio_message_get_rtr(const canio_message_obj_t *self)
8080
{
8181
return self->rtr;
8282
}
8383

84-
void common_hal_canio_message_rtr_set(canio_message_obj_t *self, bool rtr)
84+
void common_hal_canio_message_set_rtr(canio_message_obj_t *self, bool rtr)
8585
{
8686
self->rtr = rtr;
8787
}
8888

89-
bool common_hal_canio_message_extended_get(const canio_message_obj_t *self)
89+
bool common_hal_canio_message_get_extended(const canio_message_obj_t *self)
9090
{
9191
return self->extended;
9292
}
9393

94-
void common_hal_canio_message_extended_set(canio_message_obj_t *self, bool extended)
94+
void common_hal_canio_message_set_extended(canio_message_obj_t *self, bool extended)
9595
{
9696
self->extended = extended;
9797
}

0 commit comments

Comments
 (0)