Commit 4ec156c2 authored by Florian Hars's avatar Florian Hars
Browse files

fix epic_stream_read API

Epicardium sensor stream use the FreRTOS Queue-API, which transfers
fixed size blobs of bytes. To work correctly, a client of this
API (like pycardium) must interpret the bytes received from the
queue the same way as the sender. Unfortunately, there is no way to
statically enforce this, so the existing code used a simple heuristic
to detect some misuses of the API: it checks that the size of the
buffer passed by the client to epic_stream_read is a whole multiple of
the sensor data size. But this is prone to errors: if the client
passes a buffer that can hold thirty data packets of size eight, but
accidentally specifies a queue with a packet size of six, the API may
read up to fourty data packets, and if the client doesn't check for
the possibillity that the API returns a number of read packets that is
larger than the maximum requested (which none of the (two) existing
clients did), it will happily read past the end of the buffer,
triggering undefined behavior.

The modified API does instead force the client to pass the size of the
data packets and the maximum number of packets to read, and so makes
sure that the API will never read more packets than requested. If the
client still happens to specify a wrong queue with the right packet
size it will of course still get garbage data.
parent c3c5a5f8
Pipeline #4145 passed with stages
in 1 minute and 48 seconds
......@@ -957,14 +957,15 @@ API(API_PERSONAL_STATE_IS_PERSISTENT, int epic_personal_state_is_persistent());
* :param int sd: Sensor Descriptor. You get sensor descriptors as return
* values when activating the respective sensors.
* :param void* buf: Buffer where sensor data should be read into.
* :param size_t count: How many bytes to read at max. Note that fewer bytes
* might be read. In most cases, this should be ``sizeof(buf)``.
* :return: Number of data packets read (**not** number of bytes) or a negative
* :param size_t elem_size: The number of bytes in a stream element
* :param size_t count: How many elements to read at max. Note that fewer elements
* might be read. In most cases, this should be ``sizeof(buf)/elem_size``.
* :return: Number of data packets read or a negative
* error value. Possible errors:
*
* - ``-ENODEV``: Sensor is not currently available.
* - ``-EBADF``: The given sensor descriptor is unknown.
* - ``-EINVAL``: ``count`` is not a multiple of the sensor's sample size.
* - ``-EINVAL``: ``elem_size`` is not the sensor's sample size.
* - ``-EBUSY``: The descriptor table lock could not be acquired.
*
* **Example**:
......@@ -982,7 +983,8 @@ API(API_PERSONAL_STATE_IS_PERSISTENT, int epic_personal_state_is_persistent());
* n = epic_stream_read(
* foo_sd,
* &sensor_data,
* sizeof(sensor_data)
* sizeof(foo_measurement),
* sizeof(sensor_data)/sizeof(foo_measurement)
* );
*
* // Print out the measured sensor samples
......@@ -991,7 +993,7 @@ API(API_PERSONAL_STATE_IS_PERSISTENT, int epic_personal_state_is_persistent());
* }
* }
*/
API(API_STREAM_READ, int epic_stream_read(int sd, void *buf, size_t count));
API(API_STREAM_READ, int epic_stream_read(int sd, void *buf, size_t elem_size, size_t count));
/**
* BHI160 Sensor Fusion
......
......@@ -78,7 +78,7 @@ out:
return ret;
}
int epic_stream_read(int sd, void *buf, size_t count)
int epic_stream_read(int sd, void *buf, size_t elem_size, size_t count)
{
int ret = 0;
/*
......@@ -112,19 +112,19 @@ int epic_stream_read(int sd, void *buf, size_t count)
}
/* Check buffer size is a multiple of the data packet size */
if (count % stream->item_size != 0) {
if (elem_size != stream->item_size) {
ret = -EINVAL;
goto out_release;
}
size_t i;
for (i = 0; i < count; i += stream->item_size) {
if (!xQueueReceive(stream->queue, buf + i, 0)) {
for (i = 0; i < count; i += 1, buf += stream->item_size) {
if (!xQueueReceive(stream->queue, buf, 0)) {
break;
}
}
ret = i / stream->item_size;
ret = i;
out_release:
xSemaphoreGive(stream_table_lock);
......
......@@ -30,7 +30,15 @@ STATIC mp_obj_t mp_bhi160_read_sensor(mp_obj_t stream_id_in)
struct bhi160_data_vector buf[100];
int stream_id = mp_obj_get_int(stream_id_in);
int n = epic_stream_read(stream_id, buf, sizeof(buf));
int n = epic_stream_read(
stream_id,
buf,
sizeof(struct bhi160_data_vector),
sizeof(buf) / sizeof(struct bhi160_data_vector)
);
if (n < 0) {
mp_raise_OSError(-n);
}
mp_obj_list_t *list = mp_obj_new_list(0, NULL);
for (int i = 0; i < n; i++) {
......
......@@ -27,7 +27,12 @@ STATIC mp_obj_t mp_max30001_read_sensor(mp_obj_t stream_id_in)
int16_t buf[256];
int stream_id = mp_obj_get_int(stream_id_in);
int n = epic_stream_read(stream_id, buf, sizeof(buf));
int n = epic_stream_read(
stream_id, buf, sizeof(int16_t), sizeof(buf) / sizeof(int16_t)
);
if (n < 0) {
mp_raise_OSError(-n);
}
mp_obj_list_t *list = mp_obj_new_list(0, NULL);
for (int i = 0; i < n; i++) {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment