drivers/modem/modem_iface_uart: Update API

The UART IFACE API currently exposes the context struct
modem_iface_uart_data, expecting the user to fill in some
of the fields, with no documentation specifying which fields
and what they mean.

This API update moves all user configurable values in the context
out into a config struct, modem_iface_uart_config, within which
members are documented.

This prevents the user from interacting directly with the context
making use of the library safer and more readable.

The config structure helps make code readable by using "named args"
in a const struct instead of a long, nameless, parameter list passed
to the modem_iface_uart_init function.

The new API function modem_iface_uart_rx_wait is added to prevent the
user from having to interact with the rx sem in the context directly.

The context can now be safely interated with without direct access to
any of its members.

Pointers to the ring buffer params in the context have been moved to
config struct, as these are only useful during initialization of the
context.

Signed-off-by: Bjarki Arge Andreasen <baa@trackunit.com>
This commit is contained in:
Bjarki Arge Andreasen 2022-11-21 07:16:38 +01:00 committed by Carles Cufí
parent 32bf596297
commit a4afcf8c93
10 changed files with 107 additions and 58 deletions

View file

@ -174,7 +174,7 @@ static void gsm_rx(struct gsm_modem *gsm)
LOG_DBG("starting");
while (true) {
(void)k_sem_take(&gsm->gsm_data.rx_sem, K_FOREVER);
modem_iface_uart_rx_wait(&gsm->context.iface, K_FOREVER);
/* The handler will listen AT channel */
gsm->context.cmd_handler.process(&gsm->context.cmd_handler,
@ -1305,13 +1305,15 @@ static int gsm_init(const struct device *dev)
#endif /* CONFIG_MODEM_SHELL */
gsm->context.is_automatic_oper = false;
gsm->gsm_data.rx_rb_buf = &gsm->gsm_rx_rb_buf[0];
gsm->gsm_data.rx_rb_buf_len = sizeof(gsm->gsm_rx_rb_buf);
gsm->gsm_data.hw_flow_control = DT_PROP(GSM_UART_NODE,
hw_flow_control);
ret = modem_iface_uart_init(&gsm->context.iface, &gsm->gsm_data,
DEVICE_DT_GET(GSM_UART_NODE));
const struct modem_iface_uart_config uart_config = {
.rx_rb_buf = &gsm->gsm_rx_rb_buf[0],
.rx_rb_buf_len = sizeof(gsm->gsm_rx_rb_buf),
.hw_flow_control = DT_PROP(GSM_UART_NODE, hw_flow_control),
.dev = DEVICE_DT_GET(GSM_UART_NODE),
};
ret = modem_iface_uart_init(&gsm->context.iface, &gsm->gsm_data, &uart_config);
if (ret < 0) {
LOG_DBG("iface uart error %d", ret);
return ret;

View file

@ -23,10 +23,6 @@ struct modem_iface_uart_data {
/* HW flow control */
bool hw_flow_control;
/* ring buffer char buffer */
char *rx_rb_buf;
size_t rx_rb_buf_len;
/* ring buffer */
struct ring_buf rx_rb;
@ -55,17 +51,48 @@ int modem_iface_uart_init_dev(struct modem_iface *iface,
const struct device *dev);
/**
* @brief Init modem interface for UART
* @brief Modem uart interface configuration
*
* @param *iface: modem interface to initialize.
* @param *data: modem interface data to use
* @param *dev_name: name of the UART device to use
*
* @retval 0 if ok, < 0 if error.
* @param rx_rb_buf Buffer used for internal ring buffer
* @param rx_rb_buf_len Size of buffer used for internal ring buffer
* @param dev UART device used for interface
* @param hw_flow_control Set if hardware flow control is used
*/
int modem_iface_uart_init(struct modem_iface *iface,
struct modem_iface_uart_data *data,
const struct device *dev);
struct modem_iface_uart_config {
char *rx_rb_buf;
size_t rx_rb_buf_len;
const struct device *dev;
bool hw_flow_control;
};
/**
* @brief Initialize modem interface for UART
*
* @param iface Interface structure to initialize
* @param data UART data structure used by the modem interface
* @param config UART configuration structure used to configure UART data structure
*
* @return -EINVAL if any argument is invalid
* @return 0 if successful
*/
int modem_iface_uart_init(struct modem_iface *iface, struct modem_iface_uart_data *data,
const struct modem_iface_uart_config *config);
/**
* @brief Wait for rx data ready from uart interface
*
* @param iface Interface to wait on
*
* @return 0 if data is ready
* @return -EBUSY if returned without waiting
* @return -EAGAIN if timeout occurred
*/
static inline int modem_iface_uart_rx_wait(struct modem_iface *iface, k_timeout_t timeout)
{
struct modem_iface_uart_data *data = (struct modem_iface_uart_data *)iface->iface_data;
return k_sem_take(&data->rx_sem, timeout);
}
#ifdef __cplusplus
}

View file

@ -148,13 +148,12 @@ int modem_iface_uart_init_dev(struct modem_iface *iface,
return rc;
}
int modem_iface_uart_init(struct modem_iface *iface,
struct modem_iface_uart_data *data,
const struct device *dev)
int modem_iface_uart_init(struct modem_iface *iface, struct modem_iface_uart_data *data,
const struct modem_iface_uart_config *config)
{
int ret;
if (!iface || !data) {
if (iface == NULL || data == NULL || config == NULL) {
return -EINVAL;
}
@ -162,12 +161,15 @@ int modem_iface_uart_init(struct modem_iface *iface,
iface->read = modem_iface_uart_async_read;
iface->write = modem_iface_uart_async_write;
ring_buf_init(&data->rx_rb, data->rx_rb_buf_len, data->rx_rb_buf);
ring_buf_init(&data->rx_rb, config->rx_rb_buf_len, config->rx_rb_buf);
k_sem_init(&data->rx_sem, 0, 1);
k_sem_init(&data->tx_sem, 0, 1);
/* get UART device */
ret = modem_iface_uart_init_dev(iface, dev);
/* Configure hardware flow control */
data->hw_flow_control = config->hw_flow_control;
/* Get UART device */
ret = modem_iface_uart_init_dev(iface, config->dev);
if (ret < 0) {
iface->iface_data = NULL;
iface->read = NULL;

View file

@ -198,13 +198,12 @@ int modem_iface_uart_init_dev(struct modem_iface *iface,
return 0;
}
int modem_iface_uart_init(struct modem_iface *iface,
struct modem_iface_uart_data *data,
const struct device *dev)
int modem_iface_uart_init(struct modem_iface *iface, struct modem_iface_uart_data *data,
const struct modem_iface_uart_config *config)
{
int ret;
if (!iface || !data) {
if (iface == NULL || data == NULL || config == NULL) {
return -EINVAL;
}
@ -212,11 +211,14 @@ int modem_iface_uart_init(struct modem_iface *iface,
iface->read = modem_iface_uart_read;
iface->write = modem_iface_uart_write;
ring_buf_init(&data->rx_rb, data->rx_rb_buf_len, data->rx_rb_buf);
ring_buf_init(&data->rx_rb, config->rx_rb_buf_len, config->rx_rb_buf);
k_sem_init(&data->rx_sem, 0, 1);
/* get UART device */
ret = modem_iface_uart_init_dev(iface, dev);
/* Configure hardware flow control */
data->hw_flow_control = config->hw_flow_control;
/* Get UART device */
ret = modem_iface_uart_init_dev(iface, config->dev);
if (ret < 0) {
iface->iface_data = NULL;
iface->read = NULL;

View file

@ -848,7 +848,7 @@ static void modem_rx(void)
while (true) {
/* Wait for incoming data */
k_sem_take(&mdata.iface_data.rx_sem, K_FOREVER);
modem_iface_uart_rx_wait(&mctx.iface, K_FOREVER);
mctx.cmd_handler.process(&mctx.cmd_handler, &mctx.iface);
}
@ -1169,10 +1169,14 @@ static int modem_init(const struct device *dev)
}
/* modem interface */
mdata.iface_data.rx_rb_buf = &mdata.iface_rb_buf[0];
mdata.iface_data.rx_rb_buf_len = sizeof(mdata.iface_rb_buf);
ret = modem_iface_uart_init(&mctx.iface, &mdata.iface_data,
MDM_UART_DEV);
const struct modem_iface_uart_config uart_config = {
.rx_rb_buf = &mdata.iface_rb_buf[0],
.rx_rb_buf_len = sizeof(mdata.iface_rb_buf),
.dev = MDM_UART_DEV,
.hw_flow_control = DT_PROP(MDM_UART_NODE, hw_flow_control),
};
ret = modem_iface_uart_init(&mctx.iface, &mdata.iface_data, &uart_config);
if (ret < 0) {
goto error;
}

View file

@ -24,7 +24,8 @@
#include "modem_cmd_handler.h"
#include "modem_iface_uart.h"
#define MDM_UART_DEV DEVICE_DT_GET(DT_INST_BUS(0))
#define MDM_UART_NODE DT_INST_BUS(0)
#define MDM_UART_DEV DEVICE_DT_GET(MDM_UART_NODE)
#define MDM_CMD_TIMEOUT K_SECONDS(10)
#define MDM_CMD_CONN_TIMEOUT K_SECONDS(120)
#define MDM_REGISTRATION_TIMEOUT K_SECONDS(180)

View file

@ -806,7 +806,7 @@ static void modem_rx(void)
{
while (true) {
/* Wait for incoming data */
k_sem_take(&mdata.iface_data.rx_sem, K_FOREVER);
modem_iface_uart_rx_wait(&mctx.iface, K_FOREVER);
mctx.cmd_handler.process(&mctx.cmd_handler, &mctx.iface);
}
@ -2364,9 +2364,14 @@ static int modem_init(const struct device *dev)
}
/* Uart handler. */
mdata.iface_data.rx_rb_buf = &mdata.iface_rb_buf[0];
mdata.iface_data.rx_rb_buf_len = sizeof(mdata.iface_rb_buf);
ret = modem_iface_uart_init(&mctx.iface, &mdata.iface_data, MDM_UART_DEV);
const struct modem_iface_uart_config uart_config = {
.rx_rb_buf = &mdata.iface_rb_buf[0],
.rx_rb_buf_len = sizeof(mdata.iface_rb_buf),
.dev = MDM_UART_DEV,
.hw_flow_control = DT_PROP(MDM_UART_NODE, hw_flow_control),
};
ret = modem_iface_uart_init(&mctx.iface, &mdata.iface_data, &uart_config);
if (ret < 0) {
goto error;
}

View file

@ -27,7 +27,8 @@
#include "modem_iface_uart.h"
#include "modem_socket.h"
#define MDM_UART_DEV DEVICE_DT_GET(DT_INST_BUS(0))
#define MDM_UART_NODE DT_INST_BUS(0)
#define MDM_UART_DEV DEVICE_DT_GET(MDM_UART_NODE)
#define MDM_MAX_DATA_LENGTH 1024
#define MDM_RECV_BUF_SIZE 1024
#define MDM_MAX_SOCKETS 5

View file

@ -940,7 +940,7 @@ static void modem_rx(void)
{
while (true) {
/* wait for incoming data */
k_sem_take(&mdata.iface_data.rx_sem, K_FOREVER);
modem_iface_uart_rx_wait(&mctx.iface, K_FOREVER);
mctx.cmd_handler.process(&mctx.cmd_handler, &mctx.iface);
@ -2165,12 +2165,14 @@ static int modem_init(const struct device *dev)
}
/* modem interface */
mdata.iface_data.hw_flow_control = DT_PROP(MDM_UART_NODE,
hw_flow_control);
mdata.iface_data.rx_rb_buf = &mdata.iface_rb_buf[0];
mdata.iface_data.rx_rb_buf_len = sizeof(mdata.iface_rb_buf);
ret = modem_iface_uart_init(&mctx.iface, &mdata.iface_data,
MDM_UART_DEV);
const struct modem_iface_uart_config uart_config = {
.rx_rb_buf = &mdata.iface_rb_buf[0],
.rx_rb_buf_len = sizeof(mdata.iface_rb_buf),
.dev = MDM_UART_DEV,
.hw_flow_control = DT_PROP(MDM_UART_NODE, hw_flow_control),
};
ret = modem_iface_uart_init(&mctx.iface, &mdata.iface_data, &uart_config);
if (ret < 0) {
goto error;
}

View file

@ -203,7 +203,7 @@ static void esp_rx(struct esp_data *data)
{
while (true) {
/* wait for incoming data */
k_sem_take(&data->iface_data.rx_sem, K_FOREVER);
modem_iface_uart_rx_wait(&data->mctx.iface, K_FOREVER);
data->mctx.cmd_handler.process(&data->mctx.cmd_handler,
&data->mctx.iface);
@ -1286,11 +1286,14 @@ static int esp_init(const struct device *dev)
}
/* modem interface */
data->iface_data.hw_flow_control = DT_PROP(ESP_BUS, hw_flow_control);
data->iface_data.rx_rb_buf = &data->iface_rb_buf[0];
data->iface_data.rx_rb_buf_len = sizeof(data->iface_rb_buf);
ret = modem_iface_uart_init(&data->mctx.iface, &data->iface_data,
DEVICE_DT_GET(DT_INST_BUS(0)));
const struct modem_iface_uart_config uart_config = {
.rx_rb_buf = &data->iface_rb_buf[0],
.rx_rb_buf_len = sizeof(data->iface_rb_buf),
.dev = DEVICE_DT_GET(DT_INST_BUS(0)),
.hw_flow_control = DT_PROP(ESP_BUS, hw_flow_control),
};
ret = modem_iface_uart_init(&data->mctx.iface, &data->iface_data, &uart_config);
if (ret < 0) {
goto error;
}