You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Matthew Trescott <ma...@gmail.com> on 2022/03/17 05:54:38 UTC

Abstract "channels" driver model

Hi everyone,

I was thinking about how to implement the software for my project
(control module for a Formula SAE car) and realized that the basic ADC
driver model is a bit inconvenient and creates a lot of extra overhead
in userspace threads when implementing advanced features. Some
features I think would make sense in an abstract “channels” driver
model that would benefit many controls applications:

- Only wake up a thread when a value has changed "significantly."

- Support channels generated from bilinear/bicubic interpolation using a table.

- Share the same ADC values among multiple threads with different
priorities, like a high-priority safing algorithm, a medium priority
control algorithm, and a low-priority CAN broadcast thread.

- Share the same ADC values among threads with different needs—the
three listed above only care about the latest value, while a
datalogging thread that saves to flash memory would want to save data
in chunks.

- Implement software filters and allow binding to userspace control
algorithms for Kalman filtering.

- Allow different threads to subscribe to epoll events at different rates.

- Allow data received via an offboard, runtime-configurable serial bus
like CAN or UART to be assigned a meaningful name and seamlessly used
for userspace control functions just like ADC measurements, with a
userspace helper thread de-serializing the messages as they arrive.

- Allow the application to wait for two or more channels to be consistent.

- Avoid unnecessary copies and support ADC DMA transfers.

~~~~~~~~
The model I have in mind is something like this:

- Channels are char-devices; e.g. /dev/channels/throttle-position-1

- A userspace application that only cares about the latest measurement
would do something like this to get a pointer that always points to
the latest channel value and avoid the overhead of read():

    uint32_t *throttle_position_1;
    int tps1_fd;
    tps1_fd = open("/dev/channels/throttle-position-1");
    ioctl(tps1_fd, CHANIOC_SUBSCRIBE, (unsigned long)&throttle_position_1);
    ioctl(tps1_fd, CHANIOC_SET_FILTER, ...);
    poll(...);
    // Do some calculations with *throttle_position_1
    // Drive some output

- A periodic userspace task that works with bulk data would read()
from the ring buffer, or get a pointer to the ring buffer with an
ioctl (not sure which would be better)

- New data is copied to the ring buffer in the low- or high-priority work queue.

- Boardctl or board-init logic is responsible for setting up special
ADC triggering (e.g. from a PWM module) and telling the ADC backend
driver to register itself as a channel. DMA can be set up so that ADC
measurements are directly copied to the "latest value" location if no
filtering or conversion is needed.

- Software-generated channels are created with a syscall; the
userspace thread can write to the resulting char device and the data
will be stored in the ring buffer and wake up any threads waiting on
the software-defined channel.

~~~~

Looking for prior art, I noticed that there are the Common Sensor
Register Interace and Sensor Cluster Driver Interface
(drivers/sensors/README.txt). However, these don't cover ADCs or
address the overhead of having a multiple-input control thread needing
to use both epoll() and read(). Nor do they support filtering or
multiple userspace threads accessing the same sensor in a convenient
way.

However, something like this might already exist, just not in mainline
NuttX. If you know of such a thing or have feedback on this idea,
please let me know. Otherwise, I'll move ahead with working on it.

Best regards,
Matt

Re: Abstract "channels" driver model

Posted by Xiang Xiao <xi...@gmail.com>.
On Thu, Mar 17, 2022 at 6:24 PM Tim <Ti...@jti.uk.com.invalid> wrote:

> Just a "thumbs up" from me if you do this (or find something that exists
> that'll do it) - it's a perfect fit for my current project :)
>
>
So the update matches your expectation, please watch github PR
notification, we will upstream the patch after two/three weeks.


> >-----Original Message-----
> >From: Matthew Trescott <ma...@gmail.com>
> >Sent: 17 March 2022 05:55
> >To: dev@nuttx.apache.org
> >Subject: Abstract "channels" driver model
> >
> >Hi everyone,
> >
> >I was thinking about how to implement the software for my project (control
> >module for a Formula SAE car) and realized that the basic ADC driver
> model is
> >a bit inconvenient and creates a lot of extra overhead in userspace
> threads
> >when implementing advanced features. Some features I think would make
> >sense in an abstract “channels” driver model that would benefit many
> controls
> >applications:
> >
> >- Only wake up a thread when a value has changed "significantly."
> >
> >- Support channels generated from bilinear/bicubic interpolation using a
> table.
> >
> >- Share the same ADC values among multiple threads with different
> priorities,
> >like a high-priority safing algorithm, a medium priority control
> algorithm, and a
> >low-priority CAN broadcast thread.
> >
> >- Share the same ADC values among threads with different needs—the three
> >listed above only care about the latest value, while a datalogging thread
> that
> >saves to flash memory would want to save data in chunks.
> >
> >- Implement software filters and allow binding to userspace control
> algorithms
> >for Kalman filtering.
> >
> >- Allow different threads to subscribe to epoll events at different rates.
> >
> >- Allow data received via an offboard, runtime-configurable serial bus
> like CAN
> >or UART to be assigned a meaningful name and seamlessly used for userspace
> >control functions just like ADC measurements, with a userspace helper
> thread
> >de-serializing the messages as they arrive.
> >
> >- Allow the application to wait for two or more channels to be consistent.
> >
> >- Avoid unnecessary copies and support ADC DMA transfers.
> >
> >~~~~~~~~
> >The model I have in mind is something like this:
> >
> >- Channels are char-devices; e.g. /dev/channels/throttle-position-1
> >
> >- A userspace application that only cares about the latest measurement
> would
> >do something like this to get a pointer that always points to the latest
> channel
> >value and avoid the overhead of read():
> >
> >    uint32_t *throttle_position_1;
> >    int tps1_fd;
> >    tps1_fd = open("/dev/channels/throttle-position-1");
> >    ioctl(tps1_fd, CHANIOC_SUBSCRIBE, (unsigned
> long)&throttle_position_1);
> >    ioctl(tps1_fd, CHANIOC_SET_FILTER, ...);
> >    poll(...);
> >    // Do some calculations with *throttle_position_1
> >    // Drive some output
> >
> >- A periodic userspace task that works with bulk data would read() from
> the
> >ring buffer, or get a pointer to the ring buffer with an ioctl (not sure
> which
> >would be better)
> >
> >- New data is copied to the ring buffer in the low- or high-priority work
> queue.
> >
> >- Boardctl or board-init logic is responsible for setting up special ADC
> triggering
> >(e.g. from a PWM module) and telling the ADC backend driver to register
> itself
> >as a channel. DMA can be set up so that ADC measurements are directly
> >copied to the "latest value" location if no filtering or conversion is
> needed.
> >
> >- Software-generated channels are created with a syscall; the userspace
> >thread can write to the resulting char device and the data will be stored
> in the
> >ring buffer and wake up any threads waiting on the software-defined
> >channel.
> >
> >~~~~
> >
> >Looking for prior art, I noticed that there are the Common Sensor Register
> >Interace and Sensor Cluster Driver Interface (drivers/sensors/README.txt).
> >However, these don't cover ADCs or address the overhead of having a
> >multiple-input control thread needing to use both epoll() and read(). Nor
> do
> >they support filtering or multiple userspace threads accessing the same
> >sensor in a convenient way.
> >
> >However, something like this might already exist, just not in mainline
> NuttX. If
> >you know of such a thing or have feedback on this idea, please let me
> know.
> >Otherwise, I'll move ahead with working on it.
> >
> >Best regards,
> >Matt
>
>

Re: Abstract "channels" driver model

Posted by Gregory Nutt <sp...@gmail.com>.
> >- Share the same ADC values among multiple threads with different
> priorities,
> >like a high-priority safing algorithm, a medium priority control
> algorithm, and a
> >low-priority CAN broadcast thread.
> >
> >- Share the same ADC values among threads with different needs—the three
> >listed above only care about the latest value, while a datalogging thread
> that
> >saves to flash memory would want to save data in chunks.
> >de-serializing the messages as they arrive.
>

Some of this data distribution may be addressed by the cluster driver logic
in the OS.   Bob Feretich added this a long time ago, but it has seen
little use.  See:


https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/sensors/cluster_driver.h
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/README.txt#L49

The Sony Spresense offers a a hardware solution that also distributes
sensor data.

Ideally, we should take one driver model and extend and enhance it rather
than re-inventing it periodically.

RE: Abstract "channels" driver model

Posted by Tim <Ti...@JTi.uk.com.INVALID>.
Just a "thumbs up" from me if you do this (or find something that exists that'll do it) - it's a perfect fit for my current project :)

>-----Original Message-----
>From: Matthew Trescott <ma...@gmail.com>
>Sent: 17 March 2022 05:55
>To: dev@nuttx.apache.org
>Subject: Abstract "channels" driver model
>
>Hi everyone,
>
>I was thinking about how to implement the software for my project (control
>module for a Formula SAE car) and realized that the basic ADC driver model is
>a bit inconvenient and creates a lot of extra overhead in userspace threads
>when implementing advanced features. Some features I think would make
>sense in an abstract “channels” driver model that would benefit many controls
>applications:
>
>- Only wake up a thread when a value has changed "significantly."
>
>- Support channels generated from bilinear/bicubic interpolation using a table.
>
>- Share the same ADC values among multiple threads with different priorities,
>like a high-priority safing algorithm, a medium priority control algorithm, and a
>low-priority CAN broadcast thread.
>
>- Share the same ADC values among threads with different needs—the three
>listed above only care about the latest value, while a datalogging thread that
>saves to flash memory would want to save data in chunks.
>
>- Implement software filters and allow binding to userspace control algorithms
>for Kalman filtering.
>
>- Allow different threads to subscribe to epoll events at different rates.
>
>- Allow data received via an offboard, runtime-configurable serial bus like CAN
>or UART to be assigned a meaningful name and seamlessly used for userspace
>control functions just like ADC measurements, with a userspace helper thread
>de-serializing the messages as they arrive.
>
>- Allow the application to wait for two or more channels to be consistent.
>
>- Avoid unnecessary copies and support ADC DMA transfers.
>
>~~~~~~~~
>The model I have in mind is something like this:
>
>- Channels are char-devices; e.g. /dev/channels/throttle-position-1
>
>- A userspace application that only cares about the latest measurement would
>do something like this to get a pointer that always points to the latest channel
>value and avoid the overhead of read():
>
>    uint32_t *throttle_position_1;
>    int tps1_fd;
>    tps1_fd = open("/dev/channels/throttle-position-1");
>    ioctl(tps1_fd, CHANIOC_SUBSCRIBE, (unsigned long)&throttle_position_1);
>    ioctl(tps1_fd, CHANIOC_SET_FILTER, ...);
>    poll(...);
>    // Do some calculations with *throttle_position_1
>    // Drive some output
>
>- A periodic userspace task that works with bulk data would read() from the
>ring buffer, or get a pointer to the ring buffer with an ioctl (not sure which
>would be better)
>
>- New data is copied to the ring buffer in the low- or high-priority work queue.
>
>- Boardctl or board-init logic is responsible for setting up special ADC triggering
>(e.g. from a PWM module) and telling the ADC backend driver to register itself
>as a channel. DMA can be set up so that ADC measurements are directly
>copied to the "latest value" location if no filtering or conversion is needed.
>
>- Software-generated channels are created with a syscall; the userspace
>thread can write to the resulting char device and the data will be stored in the
>ring buffer and wake up any threads waiting on the software-defined
>channel.
>
>~~~~
>
>Looking for prior art, I noticed that there are the Common Sensor Register
>Interace and Sensor Cluster Driver Interface (drivers/sensors/README.txt).
>However, these don't cover ADCs or address the overhead of having a
>multiple-input control thread needing to use both epoll() and read(). Nor do
>they support filtering or multiple userspace threads accessing the same
>sensor in a convenient way.
>
>However, something like this might already exist, just not in mainline NuttX. If
>you know of such a thing or have feedback on this idea, please let me know.
>Otherwise, I'll move ahead with working on it.
>
>Best regards,
>Matt


Re: Abstract "channels" driver model

Posted by Xiang Xiao <xi...@gmail.com>.
On Fri, Mar 18, 2022 at 3:17 AM Matthew Trescott <ma...@gmail.com>
wrote:

> On Thu, Mar 17, 2022 at 2:41 AM Xiang Xiao <xi...@gmail.com>
> wrote:
> >
> > We are working on this to provide multiple read/poll support. The basic
> > idea is that:
> >
> >    1. Upper half driver hold FIFO shared by all client
> >    2. Each client hold the offset into the share FIFO
> >
> > The benefit is that we don't need to maintain multiple queues and avoid
> the
> > memory duplication.
>
> Sounds interesting, but is it a FIFO or a ring buffer? The difference
> being that the location of the head pointer doesn't advance upon reads
> with a ring buffer so all threads get the same data.
>

It's a circle buffer. Each reader has unique head pointer, the upper half
driver will update the head to point to the oldest data if the reader fails
to fetch the data quickly.


>
> Did you already bring up this concept on the mailing list? I don't
> recall.... The reason I brought this up before I started writing code
> is because it's important to define the scope and details of the
> problem and specify what the solution needs to do, before trying to
> solve it. And get feedback. That would certainly help avoid
> reinventing the driver model periodically.
>
>
The basic driver(support single client) already mainline one year ago:
https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/sensors/sensor.h
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c
Here is a simple test driver:
https://github.com/apache/incubator-nuttx-apps/tree/master/testing/sensortest



> > Nor do they support filtering or
> > >
> >
> > Kernel side only support the resample filter to handle the different
> sample
> > frequency request:
> >
> >    1. Upper half collect the sample frequency from all client
> >    2. Forward the fast request to the lower half
> >    3. Down sample to match slow client in the read callback
> >
> >  We think that other advanced filter or algorithm is better to put to the
> > userspace, and will provide an interface compatible with uORB:
> > https://docs.px4.io/master/en/middleware/uorb.html
>
> Ok... but that approach is not necessarily suitable for
> resource-constrained applications. Performing the filtering in
> userspace and copying the data around with sockets or message queues
> means many extra context switches.
>

user space doesn't need socket or message queue to talk with each other,
the buffer is managed by a common kernel virtual sensor driver which is
naturally shared by all clients.
Yes, it may impact on the performance by putting the algo to user space,
this is a tradeoff between the architecture and performance. But, anyway
you can write the algo in kernel space if you prefer.


>
> Besides, you need to implement some kind of filtering in kernel space
> anyway to avoid aliasing when downsampling.
>

The algo can be improved step by step and selected through Kconfig.


>
> (Incidentally, who exactly is the "we" that thinks other advanced
> filters are better put in userspace? Tim's email suggests that's not
> quite everyone's opinion.)
>
>
It come from Unix design philosophy: kernel provide the device abstraction
and user space add more function and strategy on top of kernel facility.
Again, nothing forbid you put filter to kernel space. It's more complex to
support the filter in user space than kernel space. That's why we provide
this common functionality just like file subsystem provide userfs and net
subsystem provide usrsock.


> ~~~
>
> Greg, I did look at the sensor cluster driver; it might be possible to
> extend that and retain API compatibility but otherwise it really
> doesn't do enough for my needs. Would need to see if it can be
> decoupled from SPI and I2C.
>
> Do you have feedback on the userspace pointer concept and its
> POSIX-ness or lack thereof?
>
> Matt
>

Re: Abstract "channels" driver model

Posted by Matthew Trescott <ma...@gmail.com>.
On Thu, Mar 17, 2022 at 2:41 AM Xiang Xiao <xi...@gmail.com> wrote:
>
> We are working on this to provide multiple read/poll support. The basic
> idea is that:
>
>    1. Upper half driver hold FIFO shared by all client
>    2. Each client hold the offset into the share FIFO
>
> The benefit is that we don't need to maintain multiple queues and avoid the
> memory duplication.

Sounds interesting, but is it a FIFO or a ring buffer? The difference
being that the location of the head pointer doesn't advance upon reads
with a ring buffer so all threads get the same data.

Did you already bring up this concept on the mailing list? I don't
recall.... The reason I brought this up before I started writing code
is because it's important to define the scope and details of the
problem and specify what the solution needs to do, before trying to
solve it. And get feedback. That would certainly help avoid
reinventing the driver model periodically.

> Nor do they support filtering or
> >
>
> Kernel side only support the resample filter to handle the different sample
> frequency request:
>
>    1. Upper half collect the sample frequency from all client
>    2. Forward the fast request to the lower half
>    3. Down sample to match slow client in the read callback
>
>  We think that other advanced filter or algorithm is better to put to the
> userspace, and will provide an interface compatible with uORB:
> https://docs.px4.io/master/en/middleware/uorb.html

Ok... but that approach is not necessarily suitable for
resource-constrained applications. Performing the filtering in
userspace and copying the data around with sockets or message queues
means many extra context switches.

Besides, you need to implement some kind of filtering in kernel space
anyway to avoid aliasing when downsampling.

(Incidentally, who exactly is the "we" that thinks other advanced
filters are better put in userspace? Tim's email suggests that's not
quite everyone's opinion.)

~~~

Greg, I did look at the sensor cluster driver; it might be possible to
extend that and retain API compatibility but otherwise it really
doesn't do enough for my needs. Would need to see if it can be
decoupled from SPI and I2C.

Do you have feedback on the userspace pointer concept and its
POSIX-ness or lack thereof?

Matt

Re: Abstract "channels" driver model

Posted by Xiang Xiao <xi...@gmail.com>.
On Thu, Mar 17, 2022 at 1:55 PM Matthew Trescott <ma...@gmail.com>
wrote:

> Hi everyone,
>
> I was thinking about how to implement the software for my project
> (control module for a Formula SAE car) and realized that the basic ADC
> driver model is a bit inconvenient and creates a lot of extra overhead
> in userspace threads when implementing advanced features. Some
> features I think would make sense in an abstract “channels” driver
> model that would benefit many controls applications:
>
> - Only wake up a thread when a value has changed "significantly."
>
> - Support channels generated from bilinear/bicubic interpolation using a
> table.
>
> - Share the same ADC values among multiple threads with different
> priorities, like a high-priority safing algorithm, a medium priority
> control algorithm, and a low-priority CAN broadcast thread.
>
> - Share the same ADC values among threads with different needs—the
> three listed above only care about the latest value, while a
> datalogging thread that saves to flash memory would want to save data
> in chunks.
>
> - Implement software filters and allow binding to userspace control
> algorithms for Kalman filtering.
>
> - Allow different threads to subscribe to epoll events at different rates.
>
> - Allow data received via an offboard, runtime-configurable serial bus
> like CAN or UART to be assigned a meaningful name and seamlessly used
> for userspace control functions just like ADC measurements, with a
> userspace helper thread de-serializing the messages as they arrive.
>
> - Allow the application to wait for two or more channels to be consistent.
>
> - Avoid unnecessary copies and support ADC DMA transfers.
>
> ~~~~~~~~
> The model I have in mind is something like this:
>
> - Channels are char-devices; e.g. /dev/channels/throttle-position-1
>
> - A userspace application that only cares about the latest measurement
> would do something like this to get a pointer that always points to
> the latest channel value and avoid the overhead of read():
>
>     uint32_t *throttle_position_1;
>     int tps1_fd;
>     tps1_fd = open("/dev/channels/throttle-position-1");
>     ioctl(tps1_fd, CHANIOC_SUBSCRIBE, (unsigned long)&throttle_position_1);
>     ioctl(tps1_fd, CHANIOC_SET_FILTER, ...);
>     poll(...);
>     // Do some calculations with *throttle_position_1
>     // Drive some output
>
> - A periodic userspace task that works with bulk data would read()
> from the ring buffer, or get a pointer to the ring buffer with an
> ioctl (not sure which would be better)
>
> - New data is copied to the ring buffer in the low- or high-priority work
> queue.
>
> - Boardctl or board-init logic is responsible for setting up special
> ADC triggering (e.g. from a PWM module) and telling the ADC backend
> driver to register itself as a channel. DMA can be set up so that ADC
> measurements are directly copied to the "latest value" location if no
> filtering or conversion is needed.
>
> - Software-generated channels are created with a syscall; the
> userspace thread can write to the resulting char device and the data
> will be stored in the ring buffer and wake up any threads waiting on
> the software-defined channel.
>
> ~~~~
>
> Looking for prior art, I noticed that there are the Common Sensor
> Register Interace and Sensor Cluster Driver Interface
> (drivers/sensors/README.txt). However, these don't cover ADCs or
> address the overhead of having a multiple-input control thread needing
> to use both epoll() and read().


We are working on this to provide multiple read/poll support. The basic
idea is that:

   1. Upper half driver hold FIFO shared by all client
   2. Each client hold the offset into the share FIFO

The benefit is that we don't need to maintain multiple queues and avoid the
memory duplication.

Nor do they support filtering or
>

Kernel side only support the resample filter to handle the different sample
frequency request:

   1. Upper half collect the sample frequency from all client
   2. Forward the fast request to the lower half
   3. Down sample to match slow client in the read callback

 We think that other advanced filter or algorithm is better to put to the
userspace, and will provide an interface compatible with uORB:
https://docs.px4.io/master/en/middleware/uorb.html

multiple userspace threads accessing the same sensor in a convenient
> way.
>
>
This will be supported in the new update.


> However, something like this might already exist, just not in mainline
> NuttX. If you know of such a thing or have feedback on this idea,
> please let me know. Otherwise, I'll move ahead with working on it.
>
>
In the new update, we will also provide the virtual(userspace) sensor
concept, which mean that you can:

   1. Register the predefined sensor through sensor_register inside kernel
   2. Register the custom sensor through sensor_custom_register kernel
   3. Register the filter/algo output as virtual sensor through ioctl(like
   pmtx) from userspace

userspace can even open a sensor registered by another core(e.g. sensorhub)
in one SoC.


> Best regards,
> Matt
>