You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Tim Hardisty <ti...@hardisty.co.uk> on 2023/03/11 16:12:07 UTC
Sensor implementation
I submitted a PR for a driver for the Broadcom APDS-9922 ambient light and proximity sensor, written with what one might call the "traditional" method of setting up the device via ioctl, then reading data when available according to the device setup, via poll notify.
The reviewer (Hi Alan - I'm not complaining!!) has suggested it perhaps ought to use the "new" sensor methodology with a sensor_lower_half_s etc.
Looking at sensors that use this I can't see any that have the range of set-up options of this device and are just "there", in the main apart from calibration, for example. Nor any NuttX documentation I can see?
Is this following a Linux methodology, like the power devices now do? When I did a power supply device drive I was pointed in the direction of Linux documentation that (after much reading and cogitation) helped explain what NuttX was essentially emulating and, with the addition of some more members to the regulator_desc_s struct it was then fine and I wrote it that way.
I am still not convinced that this ALS/Proximity sensor necessarily fits the "new" sensor methodology but if someone can point me in the direction of relevant documentation I will gladly take a look.
Thx,
TimH
Re: Sensor implementation
Posted by "Alan C. Assis" <ac...@gmail.com>.
Agreed!
I suggested to use the new sensor because it was submitted sometime
ago and I don't see broad adoption.
Maybe it is too much for simple sensors and the fact that there is not
documentation makes things more difficult to digest.
So, I think it shouldn't be a replacement for our previous char
drivers. Let to developer to decide what fits better for him.
BR,
Alan
On 3/12/23, Frank-Christian Kruegel <nu...@istda.com> wrote:
> My thoughts: simple things should be simple. Getting temperature or
> humidity values from a sensor only requires a few lines of code. A
> simple character driver fells just right for me.
>
> More code brings more potential bugs with it. The character drivers
> should remain as an option for simple things.
>
> Frank-Christian
>
>
> Am 12.03.2023 um 12:44 schrieb Tim Hardisty:
>> I had a quick skip through the video. My initial thoughts are:
>>
>> 1) This seems to be aimed at multi-sensor, multi-message systems, using
>> uORB; and, perhaps even, really and perhaps only intended for PX4?
>> 2) is it a "generic" solution, that is a really good idea, and NuttX is
>> right to be adopting it, or is it just "a" way of sensors interacting with
>> higher level software?
>> 3) was it discussed here before?
>>
>> Part of me thinks that this should be a layer on top of the
>> tried-and-tested character driver approach, and it shouldn't actually be
>> replacing it?
>>
>> In the absence of any documentation, and even looking at the LTR-308
>> sensor as an example, it seems to be geared towards a sensor that will
>> take regular readings (CONFIG_SENSORS_xxxxx_POLL_INTERVAL) and report
>> timestamped changes.
>>
>> Does it cater properly for "occasional" sensors that rarely need to send
>> data? I can see there is some kind of push event so I imagine "my" sensor
>> driver could adopt this methodology and send a message if the light level
>> changes or a near/far proximity event occurs, but it seems the(my) whole
>> system has to adopt a more complex messaging system for no good reason:
>> the traditional poll_notify works well for me!
>>
>> So, in summary, if this is the official way that sensors should be
>> implemented, I will revisit my working driver, pain though that is. If,
>> however, it is just "a" way I would rather leave it alone and see it
>> merged once checked. In the future someone - or I - can revisit it should
>> there ever be a wish for the APDS-9922 to follow the uORB/rpmsg approach.
>>
>> Discuss! Guide me!
>>
>> On 11/03/2023, 23:31, "Alan C. Assis" <acassis@gmail.com
>> <ma...@gmail.com>> wrote:
>>
>>
>> On 3/11/23, Xiang Xiao <xiaoxiang781216@gmail.com
>> <ma...@gmail.com>> wrote:
>>> On Sun, Mar 12, 2023 at 12:12 AM Tim Hardisty <tim@hardisty.co.uk
>>> <ma...@hardisty.co.uk>> wrote:
>>>
>>>> I submitted a PR for a driver for the Broadcom APDS-9922 ambient light
>>>> and
>>>> proximity sensor, written with what one might call the "traditional"
>>>> method of setting up the device via ioctl, then reading data when
>>>> available
>>>> according to the device setup, via poll notify.
>>>>
>>>> The reviewer (Hi Alan - I'm not complaining!!) has suggested it perhaps
>>>> ought to use the "new" sensor methodology with a sensor_lower_half_s
>>>> etc.
>>>>
>>>> Looking at sensors that use this I can't see any that have the range of
>>>> set-up options of this device and are just "there", in the main apart
>>>> from
>>>> calibration, for example. Nor any NuttX documentation I can see?
>>>>
>>>> Is this following a Linux methodology, like the power devices now do?
>>>> When
>>>> I did a power supply device drive I was pointed in the direction of
>>>> Linux
>>>> documentation that (after much reading and cogitation) helped explain
>>>> what
>>>> NuttX was essentially emulating and, with the addition of some more
>>>> members
>>>> to the regulator_desc_s struct it was then fine and I wrote it that
>>>> way.
>>>>
>>>> I am still not convinced that this ALS/Proximity sensor necessarily
>>>> fits
>>>> the "new" sensor methodology but if someone can point me in the
>>>> direction
>>>> of relevant documentation I will gladly take a look.
>>>>
>>>>
>>> Here is an intro video: https://www.youtube.com/watch?v=ESpAE6wqy9o
>>> <https://www.youtube.com/watch?v=ESpAE6wqy9o>
>>>
>>
>>
>> Thank you Xiang!
>>
>>
>> Since this subsystem is more complex than original char driver sensor,
>> is there plans for Documentation ?
>>
>>
>> Tim, I suggest you to take a look at LTR-308 sensoe (ltr308.c) is it
>> also an ALS. So you can use it as reference.
>>
>>
>> BR,
>>
>>
>> Alan
>>
>>
>>
>>
>
>
Re: Sensor implementation
Posted by Frank-Christian Kruegel <nu...@istda.com>.
My thoughts: simple things should be simple. Getting temperature or
humidity values from a sensor only requires a few lines of code. A
simple character driver fells just right for me.
More code brings more potential bugs with it. The character drivers
should remain as an option for simple things.
Frank-Christian
Am 12.03.2023 um 12:44 schrieb Tim Hardisty:
> I had a quick skip through the video. My initial thoughts are:
>
> 1) This seems to be aimed at multi-sensor, multi-message systems, using uORB; and, perhaps even, really and perhaps only intended for PX4?
> 2) is it a "generic" solution, that is a really good idea, and NuttX is right to be adopting it, or is it just "a" way of sensors interacting with higher level software?
> 3) was it discussed here before?
>
> Part of me thinks that this should be a layer on top of the tried-and-tested character driver approach, and it shouldn't actually be replacing it?
>
> In the absence of any documentation, and even looking at the LTR-308 sensor as an example, it seems to be geared towards a sensor that will take regular readings (CONFIG_SENSORS_xxxxx_POLL_INTERVAL) and report timestamped changes.
>
> Does it cater properly for "occasional" sensors that rarely need to send data? I can see there is some kind of push event so I imagine "my" sensor driver could adopt this methodology and send a message if the light level changes or a near/far proximity event occurs, but it seems the(my) whole system has to adopt a more complex messaging system for no good reason: the traditional poll_notify works well for me!
>
> So, in summary, if this is the official way that sensors should be implemented, I will revisit my working driver, pain though that is. If, however, it is just "a" way I would rather leave it alone and see it merged once checked. In the future someone - or I - can revisit it should there ever be a wish for the APDS-9922 to follow the uORB/rpmsg approach.
>
> Discuss! Guide me!
>
> On 11/03/2023, 23:31, "Alan C. Assis" <acassis@gmail.com <ma...@gmail.com>> wrote:
>
>
> On 3/11/23, Xiang Xiao <xiaoxiang781216@gmail.com <ma...@gmail.com>> wrote:
>> On Sun, Mar 12, 2023 at 12:12 AM Tim Hardisty <tim@hardisty.co.uk <ma...@hardisty.co.uk>> wrote:
>>
>>> I submitted a PR for a driver for the Broadcom APDS-9922 ambient light
>>> and
>>> proximity sensor, written with what one might call the "traditional"
>>> method of setting up the device via ioctl, then reading data when
>>> available
>>> according to the device setup, via poll notify.
>>>
>>> The reviewer (Hi Alan - I'm not complaining!!) has suggested it perhaps
>>> ought to use the "new" sensor methodology with a sensor_lower_half_s etc.
>>>
>>> Looking at sensors that use this I can't see any that have the range of
>>> set-up options of this device and are just "there", in the main apart
>>> from
>>> calibration, for example. Nor any NuttX documentation I can see?
>>>
>>> Is this following a Linux methodology, like the power devices now do?
>>> When
>>> I did a power supply device drive I was pointed in the direction of Linux
>>> documentation that (after much reading and cogitation) helped explain
>>> what
>>> NuttX was essentially emulating and, with the addition of some more
>>> members
>>> to the regulator_desc_s struct it was then fine and I wrote it that way.
>>>
>>> I am still not convinced that this ALS/Proximity sensor necessarily fits
>>> the "new" sensor methodology but if someone can point me in the direction
>>> of relevant documentation I will gladly take a look.
>>>
>>>
>> Here is an intro video: https://www.youtube.com/watch?v=ESpAE6wqy9o <https://www.youtube.com/watch?v=ESpAE6wqy9o>
>>
>
>
> Thank you Xiang!
>
>
> Since this subsystem is more complex than original char driver sensor,
> is there plans for Documentation ?
>
>
> Tim, I suggest you to take a look at LTR-308 sensoe (ltr308.c) is it
> also an ALS. So you can use it as reference.
>
>
> BR,
>
>
> Alan
>
>
>
>
Re: Sensor implementation
Posted by Tim Hardisty <ti...@jti.uk.com.INVALID>.
I had a quick skip through the video. My initial thoughts are:
1) This seems to be aimed at multi-sensor, multi-message systems, using uORB; and, perhaps even, really and perhaps only intended for PX4?
2) is it a "generic" solution, that is a really good idea, and NuttX is right to be adopting it, or is it just "a" way of sensors interacting with higher level software?
3) was it discussed here before?
Part of me thinks that this should be a layer on top of the tried-and-tested character driver approach, and it shouldn't actually be replacing it?
In the absence of any documentation, and even looking at the LTR-308 sensor as an example, it seems to be geared towards a sensor that will take regular readings (CONFIG_SENSORS_xxxxx_POLL_INTERVAL) and report timestamped changes.
Does it cater properly for "occasional" sensors that rarely need to send data? I can see there is some kind of push event so I imagine "my" sensor driver could adopt this methodology and send a message if the light level changes or a near/far proximity event occurs, but it seems the(my) whole system has to adopt a more complex messaging system for no good reason: the traditional poll_notify works well for me!
So, in summary, if this is the official way that sensors should be implemented, I will revisit my working driver, pain though that is. If, however, it is just "a" way I would rather leave it alone and see it merged once checked. In the future someone - or I - can revisit it should there ever be a wish for the APDS-9922 to follow the uORB/rpmsg approach.
Discuss! Guide me!
On 11/03/2023, 23:31, "Alan C. Assis" <acassis@gmail.com <ma...@gmail.com>> wrote:
On 3/11/23, Xiang Xiao <xiaoxiang781216@gmail.com <ma...@gmail.com>> wrote:
> On Sun, Mar 12, 2023 at 12:12 AM Tim Hardisty <tim@hardisty.co.uk <ma...@hardisty.co.uk>> wrote:
>
>> I submitted a PR for a driver for the Broadcom APDS-9922 ambient light
>> and
>> proximity sensor, written with what one might call the "traditional"
>> method of setting up the device via ioctl, then reading data when
>> available
>> according to the device setup, via poll notify.
>>
>> The reviewer (Hi Alan - I'm not complaining!!) has suggested it perhaps
>> ought to use the "new" sensor methodology with a sensor_lower_half_s etc.
>>
>> Looking at sensors that use this I can't see any that have the range of
>> set-up options of this device and are just "there", in the main apart
>> from
>> calibration, for example. Nor any NuttX documentation I can see?
>>
>> Is this following a Linux methodology, like the power devices now do?
>> When
>> I did a power supply device drive I was pointed in the direction of Linux
>> documentation that (after much reading and cogitation) helped explain
>> what
>> NuttX was essentially emulating and, with the addition of some more
>> members
>> to the regulator_desc_s struct it was then fine and I wrote it that way.
>>
>> I am still not convinced that this ALS/Proximity sensor necessarily fits
>> the "new" sensor methodology but if someone can point me in the direction
>> of relevant documentation I will gladly take a look.
>>
>>
> Here is an intro video: https://www.youtube.com/watch?v=ESpAE6wqy9o <https://www.youtube.com/watch?v=ESpAE6wqy9o>
>
Thank you Xiang!
Since this subsystem is more complex than original char driver sensor,
is there plans for Documentation ?
Tim, I suggest you to take a look at LTR-308 sensoe (ltr308.c) is it
also an ALS. So you can use it as reference.
BR,
Alan
Re: Sensor implementation
Posted by "Alan C. Assis" <ac...@gmail.com>.
On 3/11/23, Xiang Xiao <xi...@gmail.com> wrote:
> On Sun, Mar 12, 2023 at 12:12 AM Tim Hardisty <ti...@hardisty.co.uk> wrote:
>
>> I submitted a PR for a driver for the Broadcom APDS-9922 ambient light
>> and
>> proximity sensor, written with what one might call the "traditional"
>> method of setting up the device via ioctl, then reading data when
>> available
>> according to the device setup, via poll notify.
>>
>> The reviewer (Hi Alan - I'm not complaining!!) has suggested it perhaps
>> ought to use the "new" sensor methodology with a sensor_lower_half_s etc.
>>
>> Looking at sensors that use this I can't see any that have the range of
>> set-up options of this device and are just "there", in the main apart
>> from
>> calibration, for example. Nor any NuttX documentation I can see?
>>
>> Is this following a Linux methodology, like the power devices now do?
>> When
>> I did a power supply device drive I was pointed in the direction of Linux
>> documentation that (after much reading and cogitation) helped explain
>> what
>> NuttX was essentially emulating and, with the addition of some more
>> members
>> to the regulator_desc_s struct it was then fine and I wrote it that way.
>>
>> I am still not convinced that this ALS/Proximity sensor necessarily fits
>> the "new" sensor methodology but if someone can point me in the direction
>> of relevant documentation I will gladly take a look.
>>
>>
> Here is an intro video: https://www.youtube.com/watch?v=ESpAE6wqy9o
>
Thank you Xiang!
Since this subsystem is more complex than original char driver sensor,
is there plans for Documentation ?
Tim, I suggest you to take a look at LTR-308 sensoe (ltr308.c) is it
also an ALS. So you can use it as reference.
BR,
Alan
Re: Sensor implementation
Posted by Xiang Xiao <xi...@gmail.com>.
On Sun, Mar 12, 2023 at 12:12 AM Tim Hardisty <ti...@hardisty.co.uk> wrote:
> I submitted a PR for a driver for the Broadcom APDS-9922 ambient light and
> proximity sensor, written with what one might call the "traditional"
> method of setting up the device via ioctl, then reading data when available
> according to the device setup, via poll notify.
>
> The reviewer (Hi Alan - I'm not complaining!!) has suggested it perhaps
> ought to use the "new" sensor methodology with a sensor_lower_half_s etc.
>
> Looking at sensors that use this I can't see any that have the range of
> set-up options of this device and are just "there", in the main apart from
> calibration, for example. Nor any NuttX documentation I can see?
>
> Is this following a Linux methodology, like the power devices now do? When
> I did a power supply device drive I was pointed in the direction of Linux
> documentation that (after much reading and cogitation) helped explain what
> NuttX was essentially emulating and, with the addition of some more members
> to the regulator_desc_s struct it was then fine and I wrote it that way.
>
> I am still not convinced that this ALS/Proximity sensor necessarily fits
> the "new" sensor methodology but if someone can point me in the direction
> of relevant documentation I will gladly take a look.
>
>
Here is an intro video: https://www.youtube.com/watch?v=ESpAE6wqy9o
> Thx,
>
> TimH
>
>
>