Skip to content

Commit

Permalink
drv/uart: major overhaul
Browse files Browse the repository at this point in the history
* uartdev is refactored to be non-blocking on UART Tx
* unaligned access bugs fixed in uartdev (just lucky that they were not
  a problem before?)
* better resync when receiving data in uartdev (probably breaks motor
  position/speed)
* store LPF2 input/output flags in uartdev and use them for checking to
  see if we can actually write formatted data to a sensor
* refactor uartdev and uart drivers to allow reading and writing more
  than one byte at a time
* combine uart drivers by MCU
* fixes 2 UART devices not working at the same time on movehub/cityhub
* fixes setting mode locking up on movehub/cityhub
* iodev callback functions updated to be async
* better error checking when accessing iodev from micropython

movehub fw size +2032
  • Loading branch information
dlech committed Jun 10, 2019
1 parent 6dc665c commit 738c46f
Show file tree
Hide file tree
Showing 31 changed files with 1,454 additions and 1,149 deletions.
2 changes: 2 additions & 0 deletions bricks/cityhub/pbioconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@
// Copyright (c) 2019 David Lechner

#define PBIO_CONFIG_UARTDEV (1)
#define PBIO_CONFIG_UARTDEV_NUM_DEV (2)

#define PBIO_CONFIG_ENABLE_DEINIT (0)
#define PBIO_CONFIG_ENABLE_SYS (1)
3 changes: 3 additions & 0 deletions bricks/debug/modules/boot.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from advanced import *
from debug import *
from parameters import *
4 changes: 2 additions & 2 deletions bricks/debug/mpconfigport.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
#define PYBRICKS_HEAP_KB 64 // half of RAM

// Pybricks modules
#define PYBRICKS_PY_ADVANCED (0)
#define PYBRICKS_PY_ADVANCED (1)
#define PYBRICKS_PY_BATTERY (0)
#define PYBRICKS_PY_DEBUG (1)
#define PYBRICKS_PY_MOTOR (0)
#define PYBRICKS_PY_PARAMETERS (0)
#define PYBRICKS_PY_PARAMETERS (1)
#define PYBRICKS_PY_PUPDEVICES (0)
#define PYBRICKS_PY_TOOLS (0)

Expand Down
2 changes: 2 additions & 0 deletions bricks/debug/pbioconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@
// Copyright (c) 2019 David Lechner

#define PBIO_CONFIG_UARTDEV (1)
#define PBIO_CONFIG_UARTDEV_NUM_DEV (1)

#define PBIO_CONFIG_ENABLE_SYS (1)
2 changes: 2 additions & 0 deletions bricks/movehub/pbioconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@
// Copyright (c) 2019 David Lechner

#define PBIO_CONFIG_UARTDEV (1)
#define PBIO_CONFIG_UARTDEV_NUM_DEV (2)

#define PBIO_CONFIG_ENABLE_DEINIT (0)
#define PBIO_CONFIG_ENABLE_SYS (1)
2 changes: 1 addition & 1 deletion bricks/stm32.mk
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,12 @@ PBIO_SRC_C = $(addprefix ports/pybricks/lib/pbio/,\
drv/$(PBIO_PLATFORM)/bluetooth.c \
drv/$(PBIO_PLATFORM)/light.c \
drv/$(PBIO_PLATFORM)/motor.c \
drv/$(PBIO_PLATFORM)/uart.c \
drv/adc/adc_stm32f$(CPU_FAMILY).c \
drv/battery/battery_adc.c \
drv/button/button_gpio.c \
drv/gpio/gpio_stm32f$(CPU_FAMILY).c \
drv/ioport/ioport_lpf2.c \
drv/uart/uart_stm32f$(CPU_FAMILY).c \
platform/$(PBIO_PLATFORM)/clock.c \
platform/$(PBIO_PLATFORM)/platform.c \
platform/$(PBIO_PLATFORM)/sys.c \
Expand Down
2 changes: 1 addition & 1 deletion extmod/modadvanced.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ STATIC mp_obj_t advanced_IODevice_mode(size_t n_args, const mp_obj_t *args) {
}
else {
// set mode
pb_assert(pb_iodevice_set_mode(self->iodev, mp_obj_get_int(args[1])));
pb_iodevice_set_mode(self->iodev, mp_obj_get_int(args[1]));
return mp_const_none;
}
}
Expand Down
12 changes: 6 additions & 6 deletions extmod/modpupdevices.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ STATIC mp_obj_t pupdevices_ColorAndDistSensor_make_new(const mp_obj_type_t *type

pb_assert(pbdrv_ioport_get_iodev(port, &self->iodev));
pb_iodevice_assert_type_id(self->iodev, PBIO_IODEV_TYPE_ID_COLOR_DIST_SENSOR);
pb_assert(pb_iodevice_set_mode(self->iodev, 8));
pb_iodevice_set_mode(self->iodev, 8);
return MP_OBJ_FROM_PTR(self);
}

STATIC uint8_t pupdevices_ColorAndDistSensor_combined_mode(pbio_iodev_t *iodev, uint8_t idx) {
pb_assert(pb_iodevice_set_mode(iodev, 8));
pb_iodevice_set_mode(iodev, 8);
uint8_t *data;
pb_assert(pbio_iodev_get_raw_values(iodev, &data));
pb_assert(pbio_iodev_get_data(iodev, &data));
return data[idx];
}

Expand Down Expand Up @@ -91,17 +91,17 @@ MP_DEFINE_CONST_FUN_OBJ_1(pupdevices_ColorAndDistSensor_reflection_obj, pupdevic
STATIC mp_obj_t pupdevices_ColorAndDistSensor_ambient(mp_obj_t self_in) {
pupdevices_ColorAndDistSensor_obj_t *self = MP_OBJ_TO_PTR(self_in);
pb_iodevice_assert_type_id(self->iodev, PBIO_IODEV_TYPE_ID_COLOR_DIST_SENSOR);
pb_assert(pb_iodevice_set_mode(self->iodev, 4));
pb_iodevice_set_mode(self->iodev, 4);
return pb_iodevice_get_values(self->iodev);
}
MP_DEFINE_CONST_FUN_OBJ_1(pupdevices_ColorAndDistSensor_ambient_obj, pupdevices_ColorAndDistSensor_ambient);

STATIC mp_obj_t pupdevices_ColorAndDistSensor_rgb(mp_obj_t self_in) {
pupdevices_ColorAndDistSensor_obj_t *self = MP_OBJ_TO_PTR(self_in);
pb_iodevice_assert_type_id(self->iodev, PBIO_IODEV_TYPE_ID_COLOR_DIST_SENSOR);
pb_assert(pb_iodevice_set_mode(self->iodev, 6));
pb_iodevice_set_mode(self->iodev, 6);
uint8_t *data;
pb_assert(pbio_iodev_get_raw_values(self->iodev, &data));
pb_assert(pbio_iodev_get_data(self->iodev, &data));
mp_obj_t rgb[3];
for (uint8_t col = 0; col < 3; col++) {
int16_t intensity = ((*(int16_t *)(data + col * 2))*10)/44;
Expand Down
44 changes: 32 additions & 12 deletions extmod/pbiodevice.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,25 @@
#include "pberror.h"
#include "pbiodevice.h"

static void wait(pbio_error_t (*end)(pbio_iodev_t *), void (*cancel)(pbio_iodev_t *), pbio_iodev_t* iodev) {
nlr_buf_t nlr;
pbio_error_t err;

if (nlr_push(&nlr) == 0) {
while ((err = end(iodev)) == PBIO_ERROR_AGAIN) {
MICROPY_EVENT_POLL_HOOK
}
nlr_pop();
pb_assert(err);
} else {
cancel(iodev);
while (end(iodev) == PBIO_ERROR_AGAIN) {
MICROPY_VM_HOOK_LOOP
}
nlr_raise(nlr.ret_val);
}
}

void pb_iodevice_assert_type_id(pbio_iodev_t *iodev, pbio_iodev_type_id_t type_id) {
if (!iodev->info || iodev->info->type_id != type_id) {
pb_assert(PBIO_ERROR_NO_DEV);
Expand All @@ -35,22 +54,19 @@ pbio_error_t pb_iodevice_get_mode(pbio_iodev_t *iodev, uint8_t *current_mode) {
return PBIO_SUCCESS;
}

pbio_error_t pb_iodevice_set_mode(pbio_iodev_t *iodev, uint8_t new_mode) {
void pb_iodevice_set_mode(pbio_iodev_t *iodev, uint8_t new_mode) {
pbio_error_t err;

// FIXME: it would be better to do this check on a per-sensor basis since
// some sensors use setting the mode as a oneshot to update the sensor
// value - e.g. LEGO EV3 Ultrasonic sensor in certain modes.
if (iodev->mode == new_mode){
return PBIO_SUCCESS;
return;
}

err = pbio_iodev_set_mode(iodev, new_mode);
// Wait for mode change to complete unless there was an error.
while (err == PBIO_SUCCESS && iodev->mode != new_mode) {
mp_hal_delay_ms(1);
}
return err;
while ((err = pbio_iodev_set_mode_begin(iodev, new_mode)) == PBIO_ERROR_AGAIN);
pb_assert(err);
wait(pbio_iodev_set_mode_end, pbio_iodev_set_mode_cancel, iodev);
}

mp_obj_t pb_iodevice_get_values(pbio_iodev_t *iodev) {
Expand All @@ -59,8 +75,8 @@ mp_obj_t pb_iodevice_get_values(pbio_iodev_t *iodev) {
uint8_t len, i;
pbio_iodev_data_type_t type;

pb_assert(pbio_iodev_get_raw_values(iodev, &data));
pb_assert(pbio_iodev_get_bin_format(iodev, &len, &type));
pb_assert(pbio_iodev_get_data(iodev, &data));
pb_assert(pbio_iodev_get_data_format(iodev, iodev->mode, &len, &type));

// this shouldn't happen, but just in case...
if (len == 0) {
Expand Down Expand Up @@ -105,8 +121,9 @@ mp_obj_t pb_iodevice_set_values(pbio_iodev_t *iodev, mp_obj_t values) {
mp_obj_t *items;
uint8_t len, i;
pbio_iodev_data_type_t type;
pbio_error_t err;

pb_assert(pbio_iodev_get_bin_format(iodev, &len, &type));
pb_assert(pbio_iodev_get_data_format(iodev, iodev->mode, &len, &type));

// if we only have one value, it doesn't have to be a tuple/list
if (len == 1 && (mp_obj_is_integer(values)
Expand Down Expand Up @@ -144,7 +161,10 @@ mp_obj_t pb_iodevice_set_values(pbio_iodev_t *iodev, mp_obj_t values) {
}
}

pb_assert(pbio_iodev_set_raw_values(iodev, data));
while ((err = pbio_iodev_set_data_begin(iodev, iodev->mode, data)) == PBIO_ERROR_AGAIN);
pb_assert(err);
wait(pbio_iodev_set_data_end, pbio_iodev_set_data_cancel, iodev);

return mp_const_none;
}

Expand Down
2 changes: 1 addition & 1 deletion extmod/pbiodevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@
void pb_iodevice_assert_type_id(pbio_iodev_t *iodev, pbio_iodev_type_id_t type_id);
pbio_error_t pb_iodevice_get_type_id(pbio_iodev_t *iodev, pbio_iodev_type_id_t *id);
pbio_error_t pb_iodevice_get_mode(pbio_iodev_t *iodev, uint8_t *current_mode);
pbio_error_t pb_iodevice_set_mode(pbio_iodev_t *iodev, uint8_t new_mode);
void pb_iodevice_set_mode(pbio_iodev_t *iodev, uint8_t new_mode);
mp_obj_t pb_iodevice_get_values(pbio_iodev_t *iodev);
mp_obj_t pb_iodevice_set_values(pbio_iodev_t *iodev, mp_obj_t values);
Loading

0 comments on commit 738c46f

Please sign in to comment.