Skip to content

Commit

Permalink
pbio/iodev: Refactor uartdev interface.
Browse files Browse the repository at this point in the history
The current iodev and uartdev code will only ever be used for LEGO uart
devices, so we can reduce the level of abstraction and complexity
considerably.

Combined with the previous and next commits, this reduces code size by
about 580 bytes and helps prepare for async functionality later on,
which is the main reason for this refactoring.

The goal it to keep src/uartdev.c completely in charge of uart devices,
instead of relying on user modules correctly calling operations like
set_mode_end.
  • Loading branch information
laurensvalk committed May 26, 2023
1 parent ca4cb16 commit 1ae92c3
Show file tree
Hide file tree
Showing 29 changed files with 527 additions and 605 deletions.
1 change: 0 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@
"request": "launch",
"program": "${workspaceFolder}/bricks/virtualhub/build-debug/virtualhub-micropython",
"args": [
"${file}"
],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
Expand Down
1 change: 0 additions & 1 deletion bricks/_common/sources.mk
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ PYBRICKS_PYBRICKS_SRC_C = $(addprefix pybricks/,\
util_pb/pb_conversions.c \
util_pb/pb_device_ev3dev.c \
util_pb/pb_device_nxt.c \
util_pb/pb_device_stm32.c \
util_pb/pb_error.c \
util_pb/pb_serial_ev3dev.c \
util_pb/pb_task.c \
Expand Down
2 changes: 1 addition & 1 deletion bricks/ev3dev/brickconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#define PYBRICKS_PY_COMMON_CONTROL (1)
#define PYBRICKS_PY_COMMON_IMU (0)
#define PYBRICKS_PY_COMMON_KEYPAD (1)
#define PYBRICKS_PY_COMMON_LIGHT_ARRAY (1)
#define PYBRICKS_PY_COMMON_LIGHT_ARRAY (0)
#define PYBRICKS_PY_COMMON_LIGHT_MATRIX (0)
#define PYBRICKS_PY_COMMON_LOGGER (1)
#define PYBRICKS_PY_COMMON_LOGGER_REAL_FILE (1)
Expand Down
17 changes: 17 additions & 0 deletions bricks/virtualhub/mp_port.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,20 @@ void pb_virtualhub_delay_us(mp_uint_t us) {
pb_virtualhub_poll();
}
}

// Revisit, these don't belong here
pbio_error_t pbio_iodev_is_ready(pbio_iodev_t *iodev) {
return PBIO_SUCCESS;
}

pbio_error_t pbio_iodev_set_mode(pbio_iodev_t *iodev, uint8_t mode) {
return PBIO_SUCCESS;
}

pbio_error_t pbio_iodev_get_data(pbio_iodev_t *iodev, uint8_t mode, uint8_t **data) {
return PBIO_SUCCESS;
}

pbio_error_t pbio_iodev_set_data(pbio_iodev_t *iodev, uint8_t mode, const uint8_t *data) {
return PBIO_SUCCESS;
}
5 changes: 1 addition & 4 deletions lib/pbio/drv/counter/counter_lpf2.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,10 @@ static pbio_error_t pbdrv_counter_lpf2_update(pbdrv_counter_dev_t *dev) {
uint8_t mode_id = priv->supports_abs_angle ?
PBIO_IODEV_MODE_PUP_ABS_MOTOR__CALIB:
PBIO_IODEV_MODE_PUP_REL_MOTOR__POS;
if (iodev->mode != mode_id) {
return PBIO_ERROR_INVALID_OP;
}

// Get pointer to LPF2 data buffer.
uint8_t *data;
err = pbio_iodev_get_data(iodev, &data);
err = pbio_iodev_get_data(iodev, mode_id, &data);
if (err != PBIO_SUCCESS) {
return err;
}
Expand Down
4 changes: 0 additions & 4 deletions lib/pbio/drv/ioport/ioport_lpf2.c
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,6 @@ static void ioport_enable_uart(ioport_dev_t *ioport) {
pbdrv_gpio_out_low(&pdata->uart_buf);
}

static const pbio_iodev_ops_t basic_dev_ops = {
};

static void init_one(uint8_t ioport) {
const pbdrv_ioport_lpf2_port_platform_data_t *pdata =
&pbdrv_ioport_lpf2_platform_data.ports[ioport];
Expand All @@ -188,7 +185,6 @@ static void init_one(uint8_t ioport) {
pbdrv_gpio_set_pull(&pdata->uart_rx, PBDRV_GPIO_PULL_NONE);

basic_devs[ioport].port = PBDRV_CONFIG_IOPORT_LPF2_FIRST_PORT + ioport;
basic_devs[ioport].ops = &basic_dev_ops;
}

// TODO: This should be moved to a common ioport_core.c file or removed entirely
Expand Down
23 changes: 4 additions & 19 deletions lib/pbio/include/pbio/iodev.h
Original file line number Diff line number Diff line change
Expand Up @@ -545,25 +545,11 @@ typedef struct _pbio_iodev_t pbio_iodev_t;

/** @cond INTERNAL */

/**
* Device-specific communication functions.
*/
typedef struct {
pbio_error_t (*set_mode_begin)(pbio_iodev_t *iodev, uint8_t mode);
pbio_error_t (*set_mode_end)(pbio_iodev_t *iodev);
pbio_error_t (*set_data_begin)(pbio_iodev_t *iodev, const uint8_t *data);
pbio_error_t (*set_data_end)(pbio_iodev_t *iodev);
} pbio_iodev_ops_t;

struct _pbio_iodev_t {
/**
* Pointer to the mode info for this device.
*/
const pbio_iodev_info_t *info;
/**
* Pointer to the device-specific communication functions.
*/
const pbio_iodev_ops_t *ops;
/**
* The port the device is attached to.
*/
Expand All @@ -585,12 +571,11 @@ struct _pbio_iodev_t {

size_t pbio_iodev_size_of(pbio_iodev_data_type_t type);
pbio_error_t pbio_iodev_get_data_format(pbio_iodev_t *iodev, uint8_t mode, uint8_t *len, pbio_iodev_data_type_t *type);
pbio_error_t pbio_iodev_get_data(pbio_iodev_t *iodev, uint8_t **data);
pbio_error_t pbio_iodev_set_mode_begin(pbio_iodev_t *iodev, uint8_t mode);
pbio_error_t pbio_iodev_set_mode_end(pbio_iodev_t *iodev);
pbio_error_t pbio_iodev_set_data_begin(pbio_iodev_t *iodev, uint8_t mode, const uint8_t *data);
pbio_error_t pbio_iodev_set_data_end(pbio_iodev_t *iodev);

pbio_error_t pbio_iodev_is_ready(pbio_iodev_t *iodev);
pbio_error_t pbio_iodev_set_mode(pbio_iodev_t *iodev, uint8_t mode);
pbio_error_t pbio_iodev_set_data(pbio_iodev_t *iodev, uint8_t mode, const uint8_t *data);
pbio_error_t pbio_iodev_get_data(pbio_iodev_t *iodev, uint8_t mode, uint8_t **data);
#endif // _PBIO_IODEV_H_

/** @} */
80 changes: 0 additions & 80 deletions lib/pbio/src/iodev.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,83 +52,3 @@ pbio_error_t pbio_iodev_get_data_format(pbio_iodev_t *iodev, uint8_t mode, uint8

return PBIO_SUCCESS;
}

/**
* Gets the raw data from an I/O device.
* @param [in] iodev The I/O device
* @param [out] data Pointer to hold array of data values
* @return ::PBIO_SUCCESS on success
* ::PBIO_ERROR_NO_DEV if the port does not have a device attached
*
* The binary format and size of *data* is determined by ::pbio_iodev_get_data_format().
*/
pbio_error_t pbio_iodev_get_data(pbio_iodev_t *iodev, uint8_t **data) {
if (iodev->info->type_id == PBIO_IODEV_TYPE_ID_NONE) {
return PBIO_ERROR_NO_DEV;
}

*data = iodev->bin_data;

return PBIO_SUCCESS;
}

/**
* Sets the mode of an I/O device.
* @param [in] iodev The I/O device
* @param [in] mode The new mode
* @return ::PBIO_SUCCESS on success
* ::PBIO_ERROR_INVALID_ARG if the mode is not valid
* ::PBIO_ERROR_NOT_SUPPORTED if the device does not support setting the mode
*/
pbio_error_t pbio_iodev_set_mode_begin(pbio_iodev_t *iodev, uint8_t mode) {
if (!iodev->ops->set_mode_begin) {
return PBIO_ERROR_NOT_SUPPORTED;
}

if (mode >= iodev->info->num_modes) {
return PBIO_ERROR_INVALID_ARG;
}

return iodev->ops->set_mode_begin(iodev, mode);
}

pbio_error_t pbio_iodev_set_mode_end(pbio_iodev_t *iodev) {
if (!iodev->ops->set_mode_end) {
return PBIO_ERROR_NOT_SUPPORTED;
}

return iodev->ops->set_mode_end(iodev);
}

/**
* Sets the raw data of an I/O device.
* @param [in] iodev The I/O device
* @param [in] mode The mode
* @param [in] data Array of data values
* @return ::PBIO_SUCCESS on success
* ::PBIO_ERROR_NO_DEV if the port does not have a device attached
* ::PBIO_ERROR_AGAIN if the device is busy with something else
* ::PBIO_ERROR_NOT_SUPPORTED if the device does not support setting values
* ::PBIO_ERROR_INVALID_OP if the current mode does not match the requested mode
*
* The binary format and size of *data* is determined by ::pbio_iodev_get_data_format().
*/
pbio_error_t pbio_iodev_set_data_begin(pbio_iodev_t *iodev, uint8_t mode, const uint8_t *data) {
if (!iodev->ops->set_data_begin) {
return PBIO_ERROR_NOT_SUPPORTED;
}

if (iodev->mode != mode) {
return PBIO_ERROR_INVALID_OP;
}

return iodev->ops->set_data_begin(iodev, data);
}

pbio_error_t pbio_iodev_set_data_end(pbio_iodev_t *iodev) {
if (!iodev->ops->set_data_end) {
return PBIO_ERROR_NOT_SUPPORTED;
}

return iodev->ops->set_data_end(iodev);
}
Loading

0 comments on commit 1ae92c3

Please sign in to comment.