You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/21 07:42:29 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request, #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Donny9 opened a new pull request, #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653
## Summary
Driver/sensor enhance feature:
* Support pub-sub mechanism to mange all sensor(physical sensor, abstract sensor, rpmsg sensor).
* Support user to register abstract sensor by /dev/usensor node in userspace.
* Support remote core to advertise sensor event to other core, and support remote core to subscribe sensor event from other core, so support multi-core user to access sensor event by rpmsg.
* Support down-sample.
* Allow multi users to access the same sensor device simultaneously.
* Get real state of sensor device by cmd SNIOC_GET_STATE for users.
* Get update state since last read by poll without timeout for users.
* Sensor device will be activated when first user open and will close when last user closed.
* When multi users to access device, driver always set the minimum sampling interval and latency to the sensor device and allow downsampled for users above the minimum sampling interval.
* The circbuffer will overwrite old data when buffer is full, so if users don't read data soon, data will be lost, and the oldest data in circbuffer are returned to the users.
* Always read the last data in the circbuffer as initial value for new users when the sensor device has not yet generated new data.
* when user uses poll, if subscription interval is satisfied, the POLLIN events is returned for each users.
* When new user generate or the state of sensor device changed, the POLLPRI will notify to all users.
* Support multi advertisers to subscribe their own data as loop test.
Refer to:
https://docs.px4.io/v1.12/en/middleware/uorb.html
## Impact
* Provides sensor cross-core access capability.
* Provides a subscription and publish-based mechanism to access sensors.
## Testing
Vela team CI and unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934376341
##########
drivers/sensors/sensor.c:
##########
@@ -609,35 +733,45 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct sensor_upperhalf_s *upper = inode->i_private;
FAR struct sensor_lowerhalf_s *lower = upper->lower;
FAR struct sensor_user_s *user = filep->f_priv;
- int ret;
+ int ret = 0;
sninfo("cmd=%x arg=%08lx\n", cmd, arg);
- ret = nxsem_wait(&upper->exclsem);
- if (ret < 0)
- {
- return ret;
- }
-
switch (cmd)
{
case SNIOC_GET_STATE:
{
+ nxrmutex_lock(&upper->lock);
memcpy((FAR void *)(uintptr_t)arg,
&upper->state, sizeof(upper->state));
user->changed = false;
+ nxrmutex_unlock(&upper->lock);
+ }
+ break;
+
+ case SNIOC_GET_USTATE:
+ {
+ nxrmutex_lock(&upper->lock);
Review Comment:
Ok. Lets keep it as is.
##########
drivers/sensors/sensor.c:
##########
@@ -609,35 +733,45 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct sensor_upperhalf_s *upper = inode->i_private;
FAR struct sensor_lowerhalf_s *lower = upper->lower;
FAR struct sensor_user_s *user = filep->f_priv;
- int ret;
+ int ret = 0;
sninfo("cmd=%x arg=%08lx\n", cmd, arg);
- ret = nxsem_wait(&upper->exclsem);
- if (ret < 0)
- {
- return ret;
- }
-
switch (cmd)
{
case SNIOC_GET_STATE:
{
+ nxrmutex_lock(&upper->lock);
memcpy((FAR void *)(uintptr_t)arg,
&upper->state, sizeof(upper->state));
user->changed = false;
+ nxrmutex_unlock(&upper->lock);
+ }
+ break;
+
+ case SNIOC_GET_USTATE:
+ {
+ nxrmutex_lock(&upper->lock);
Review Comment:
Ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934672003
##########
include/nuttx/sensors/sensor.h:
##########
@@ -925,12 +934,31 @@ struct sensor_lowerhalf_s
* priv - Upper half driver handle
**********************************************************************/
- CODE void (*notify_event)(FAR void *priv);
+ sensor_notify_event_t notify_event;
};
+/****************************************************************************
+ * Name: sensor_lock/sensor_unlock
+ *
+ * Description:
+ * Lower half driver can lock/unlock upper half driver by this interface.
+ *
+ * Input Parameters:
+ * priv - Upper half driver handle
+ ****************************************************************************/
+
+ void (*sensor_lock)(void * priv);
+ void (*sensor_unlock)(void * priv);
Review Comment:
@Donny9 is in vacation, I will provide a patch fix it. @pkarashchenko do yo have more comment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934281438
##########
drivers/sensors/sensor.c:
##########
@@ -609,35 +733,45 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct sensor_upperhalf_s *upper = inode->i_private;
FAR struct sensor_lowerhalf_s *lower = upper->lower;
FAR struct sensor_user_s *user = filep->f_priv;
- int ret;
+ int ret = 0;
sninfo("cmd=%x arg=%08lx\n", cmd, arg);
- ret = nxsem_wait(&upper->exclsem);
- if (ret < 0)
- {
- return ret;
- }
-
switch (cmd)
{
case SNIOC_GET_STATE:
{
+ nxrmutex_lock(&upper->lock);
memcpy((FAR void *)(uintptr_t)arg,
&upper->state, sizeof(upper->state));
user->changed = false;
+ nxrmutex_unlock(&upper->lock);
+ }
+ break;
+
+ case SNIOC_GET_USTATE:
+ {
+ nxrmutex_lock(&upper->lock);
Review Comment:
Sensor is a little bit complex, we can't protect through user's lock(actually, user don't have the lock at all), since it may change the hardware.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934039404
##########
drivers/sensors/sensor.c:
##########
@@ -609,35 +733,45 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct sensor_upperhalf_s *upper = inode->i_private;
FAR struct sensor_lowerhalf_s *lower = upper->lower;
FAR struct sensor_user_s *user = filep->f_priv;
- int ret;
+ int ret = 0;
sninfo("cmd=%x arg=%08lx\n", cmd, arg);
- ret = nxsem_wait(&upper->exclsem);
- if (ret < 0)
- {
- return ret;
- }
-
switch (cmd)
{
case SNIOC_GET_STATE:
{
+ nxrmutex_lock(&upper->lock);
memcpy((FAR void *)(uintptr_t)arg,
&upper->state, sizeof(upper->state));
user->changed = false;
+ nxrmutex_unlock(&upper->lock);
+ }
+ break;
+
+ case SNIOC_GET_USTATE:
+ {
+ nxrmutex_lock(&upper->lock);
Review Comment:
Why `user->state` access is guarded by `upper->lock`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
Donny9 commented on PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#issuecomment-1191317934
This PR https://github.com/apache/incubator-nuttx-apps/pull/1228 need merge to fix ci issue:
```
Error: sensortest.c:136:27: error: dereferencing pointer to incomplete type 'struct sensor_event_ots'
136 | "\n", name, event->timestamp, event->x, event->y);
| ^~
sensortest.c: In function 'print_valf':
Error: sensortest.c:143:21: error: dereferencing pointer to incomplete type 'struct sensor_event_prox'
143 | name, event->timestamp, event->proximity);
| ^~
sensortest.c: In function 'print_valf2':
Error: sensortest.c:150:21: error: dereferencing pointer to incomplete type 'struct sensor_event_baro'
150 | name, event->timestamp, event->pressure, event->temperature);
| ^~
sensortest.c: In function 'print_valf3':
Error: sensortest.c:157:21: error: dereferencing pointer to incomplete type 'struct sensor_event_rgb'
157 | name, event->timestamp, event->r, event->g, event->b);
| ^~
sensortest.c: In function 'print_ppgd':
Error: sensortest.c:165:21: error: dereferencing pointer to incomplete type 'struct sensor_event_ppgd'
165 | name, event->timestamp, event->ppg[0], event->ppg[1],
| ^~
sensortest.c: In function 'print_ppgq':
Error: sensortest.c:176:21: error: dereferencing pointer to incomplete type 'struct sensor_event_ppgq'
176 | name, event->timestamp, event->ppg[0], event->ppg[1], event->ppg[2],
| ^~
sensortest.c: In function 'print_gps':
Error: sensortest.c:189:21: error: dereferencing pointer to incomplete type 'struct sensor_event_gps'
189 | name, event->timestamp, event->time_utc, event->latitude,
| ^~
sensortest.c: In function 'print_gps_satellite':
```
we can test sensor driver by uorb_listener, example:
```
ap> uorb_listener -r 25 -n 10 sensor_baro
[07/21 10:29:25.711242] [1471] [ap]
Mointor objects num:1
[07/21 10:29:25.711914] [1471] [ap] object_name:sensor_baro, object_instance:0
[07/21 10:29:25.776794] [1471] [ap] sensor_baro: timestamp: 108973761230 (22156 us ago) temperature: 27.20 pressure: 994.26
[07/21 10:29:25.817199] [1471] [ap] sensor_baro: timestamp: 108973801635 (22095 us ago) temperature: 27.20 pressure: 994.25
[07/21 10:29:25.857421] [1471] [ap] sensor_baro: timestamp: 108973842041 (21911 us ago) temperature: 27.21 pressure: 994.26
[07/21 10:29:25.898376] [1471] [ap] sensor_baro: timestamp: 108973882507 (22400 us ago) temperature: 27.21 pressure: 994.24
[07/21 10:29:25.938354] [1471] [ap] sensor_baro: timestamp: 108973922912 (22034 us ago) temperature: 27.22 pressure: 994.27
[07/21 10:29:25.978515] [1471] [ap] sensor_baro: timestamp: 108973963317 (21729 us ago) temperature: 27.22 pressure: 994.26
[07/21 10:29:26.019409] [1471] [ap] sensor_baro: timestamp: 108974003784 (22155 us ago) temperature: 27.22 pressure: 994.26
[07/21 10:29:26.059814] [1471] [ap] sensor_baro: timestamp: 108974044250 (22095 us ago) temperature: 27.22 pressure: 994.26
[07/21 10:29:26.101806] [1471] [ap] sensor_baro: timestamp: 108974084716 (23682 us ago) temperature: 27.22 pressure: 994.26
[07/21 10:29:26.140869] [1471] [ap] sensor_baro: timestamp: 108974125305 (22094 us ago) temperature: 27.22 pressure: 994.27
[07/21 10:29:26.142333] [1471] [ap] Object name:sensor_baro0, recieved:10
[07/21 10:29:26.143066] [1471] [ap] Total number of received Message:10/10
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#issuecomment-1192586696
@Donny9 the PR is too large, could you split the unrelated patch to the new PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934151907
##########
drivers/sensors/sensor.c:
##########
@@ -609,35 +733,45 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct sensor_upperhalf_s *upper = inode->i_private;
FAR struct sensor_lowerhalf_s *lower = upper->lower;
FAR struct sensor_user_s *user = filep->f_priv;
- int ret;
+ int ret = 0;
sninfo("cmd=%x arg=%08lx\n", cmd, arg);
- ret = nxsem_wait(&upper->exclsem);
- if (ret < 0)
- {
- return ret;
- }
-
switch (cmd)
{
case SNIOC_GET_STATE:
{
+ nxrmutex_lock(&upper->lock);
memcpy((FAR void *)(uintptr_t)arg,
&upper->state, sizeof(upper->state));
user->changed = false;
+ nxrmutex_unlock(&upper->lock);
+ }
+ break;
+
+ case SNIOC_GET_USTATE:
+ {
+ nxrmutex_lock(&upper->lock);
Review Comment:
Let me re-examine. I think other places involve `upper` together with `user`, so I was expecting that `upper->lock` is for `upper` protection. But maybe I missed something
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
Donny9 commented on PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#issuecomment-1191156676
@jihandong Please push all uorb relate commit to apps, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
Donny9 commented on PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#issuecomment-1192216283
uorb PR:https://github.com/apache/incubator-nuttx-apps/pull/1230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934106472
##########
drivers/sensors/sensor.c:
##########
@@ -609,35 +733,45 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct sensor_upperhalf_s *upper = inode->i_private;
FAR struct sensor_lowerhalf_s *lower = upper->lower;
FAR struct sensor_user_s *user = filep->f_priv;
- int ret;
+ int ret = 0;
sninfo("cmd=%x arg=%08lx\n", cmd, arg);
- ret = nxsem_wait(&upper->exclsem);
- if (ret < 0)
- {
- return ret;
- }
-
switch (cmd)
{
case SNIOC_GET_STATE:
{
+ nxrmutex_lock(&upper->lock);
memcpy((FAR void *)(uintptr_t)arg,
&upper->state, sizeof(upper->state));
user->changed = false;
+ nxrmutex_unlock(&upper->lock);
+ }
+ break;
+
+ case SNIOC_GET_USTATE:
+ {
+ nxrmutex_lock(&upper->lock);
Review Comment:
Since other place(e.g. SNIOC_SET_INTERVAL) which modify field in user->state is locked by upper->lock.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934371214
##########
drivers/sensors/sensor.c:
##########
@@ -609,35 +733,45 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct sensor_upperhalf_s *upper = inode->i_private;
FAR struct sensor_lowerhalf_s *lower = upper->lower;
FAR struct sensor_user_s *user = filep->f_priv;
- int ret;
+ int ret = 0;
sninfo("cmd=%x arg=%08lx\n", cmd, arg);
- ret = nxsem_wait(&upper->exclsem);
- if (ret < 0)
- {
- return ret;
- }
-
switch (cmd)
{
case SNIOC_GET_STATE:
{
+ nxrmutex_lock(&upper->lock);
memcpy((FAR void *)(uintptr_t)arg,
&upper->state, sizeof(upper->state));
user->changed = false;
+ nxrmutex_unlock(&upper->lock);
+ }
+ break;
+
+ case SNIOC_GET_USTATE:
+ {
+ nxrmutex_lock(&upper->lock);
Review Comment:
Ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934375913
##########
include/nuttx/sensors/sensor.h:
##########
@@ -925,12 +934,31 @@ struct sensor_lowerhalf_s
* priv - Upper half driver handle
**********************************************************************/
- CODE void (*notify_event)(FAR void *priv);
+ sensor_notify_event_t notify_event;
};
+/****************************************************************************
+ * Name: sensor_lock/sensor_unlock
+ *
+ * Description:
+ * Lower half driver can lock/unlock upper half driver by this interface.
+ *
+ * Input Parameters:
+ * priv - Upper half driver handle
+ ****************************************************************************/
+
+ void (*sensor_lock)(void * priv);
+ void (*sensor_unlock)(void * priv);
Review Comment:
```suggestion
CODE void (*sensor_lock)(FAR void * priv);
CODE void (*sensor_unlock)(FAR void * priv);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6653: Driver/sensor: enhance sensor driver to support uorb and multi core access sensor by rpmsg
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6653:
URL: https://github.com/apache/incubator-nuttx/pull/6653#discussion_r934740108
##########
include/nuttx/sensors/sensor.h:
##########
@@ -925,12 +934,31 @@ struct sensor_lowerhalf_s
* priv - Upper half driver handle
**********************************************************************/
- CODE void (*notify_event)(FAR void *priv);
+ sensor_notify_event_t notify_event;
};
+/****************************************************************************
+ * Name: sensor_lock/sensor_unlock
+ *
+ * Description:
+ * Lower half driver can lock/unlock upper half driver by this interface.
+ *
+ * Input Parameters:
+ * priv - Upper half driver handle
+ ****************************************************************************/
+
+ void (*sensor_lock)(void * priv);
+ void (*sensor_unlock)(void * priv);
Review Comment:
No. I think we are good to merge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org