zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.14k stars 6.23k forks source link

Sensor Async Read API implementation is synchronous/blocking #73676

Closed ubieda closed 1 week ago

ubieda commented 1 month ago

Describe the bug As discussed on 240603 Sensors WG meeting, the Sensor Async Read API sensor_read_async_mempool execution path is synchronous for most if not all drivers instances in-tree. Even though the performance-optimized version of this API is conditioned on RTIO availability for Bus drivers (see #67328 and #67327), we should still comply with the API description and make this API invocation Asynchrnous and Non-blocking. That way, users can start leveraging the new Sensor API in their applications, and take advantage of the optimizations once introduced.

Examples of the synchronous/blocking pattern in-tree: https://github.com/zephyrproject-rtos/zephyr/blob/127cb9edb6c6594e2f09dea7efb335db3cb1f43a/drivers/sensor/tdk/icm42688/icm42688_rtio.c#L17 https://github.com/zephyrproject-rtos/zephyr/blob/127cb9edb6c6594e2f09dea7efb335db3cb1f43a/drivers/sensor/asahi_kasei/akm09918c/akm09918c_async.c#L31 https://github.com/zephyrproject-rtos/zephyr/blob/127cb9edb6c6594e2f09dea7efb335db3cb1f43a/drivers/sensor/bosch/bma4xx/bma4xx.c#L341

Impact Users can't rely on the asynchronous nature of sensor_read_async_mempool API. For instance, invoking this API inside a time-sensitive context would not work (e.g: https://github.com/zephyrproject-rtos/zephyr/blob/127cb9edb6c6594e2f09dea7efb335db3cb1f43a/subsys/sensing/sensor_mgmt.c#L288).

Proposed Solution Decouple the sensor read submission from its execution. From many approaches that could work, I'm thinking of:

ubieda commented 1 month ago

FYI - @teburd @yperess @MaureenHelm as discussed today in the Sensors WG Meeting. I'm planning to tackle it unless any of you wants to do so.

karthi012 commented 1 month ago

Hi @ubieda. Adding some of my thoughts here, as there is discussion IMO RTIO should handle the interrupt request in it's own context (i.e, Threads, work queue for RTIO operation)as existing RTIO does not have any. This potentially avoid thread/work handler usage in every sensor driver implementation. Is there any way to handle the time series data from the sensor using the RTIO, don't how it is feasible.

ubieda commented 1 month ago

Hi @karthi012, thanks for your feedback!

IMO RTIO should handle the interrupt request in it's own context (i.e, Threads, work queue for RTIO operation)as existing RTIO does not have any.

As I understand it, the ideal scenario is that all sensor drivers implementing the sensor_read_async_mempool API rely on the RTIO bus drivers, without the need of additional contexts (workqueue, thread, etc). These RTIO bus driver implementations would handle the transactions in a non-blocking/asynchronous manner.

To do that, however, there are multiple pieces that need to be in place and that takes time. This bug only attempts to address the immediate need for this API to be compliant with its description (being async, non-blocking) so the expectation can be met for the next release.

Introducing the shim at the RTIO bus drivers layer (which is what is suggested here: #73869) is planned after this release.

Is there any way to handle the time series data from the sensor using the RTIO, don't how it is feasible.

I'm not sure I understand what you mean by "handling time-series data". Could you expand on this?

karthi012 commented 1 month ago

@ubieda Thanks for your clarification. Please ignore time series data handling.