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 2020/11/22 08:18:48 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request #2369: driver/sensors: support custom types of sensor.
Donny9 opened a new pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369
## Summary
Support sensor for user-defined type, these special sensor whose event size is not fixed or dynamically change.
"SENSOR_TYPE_CUSTOM" is defined include/nuttx/sensors/sensor.h.
You should register sensor device by sensor_custom_register with sensor character device path (ex: "/dev/sensor/dummy") and the element size of intermediate buffer if you use it. On the contrary, you can unregister character device by sensor_custom_register with specific path.
## Impact
Add user-defined types of interfaces to make drivers compatible with more sensors.
## Testing
daily 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528356173
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
The buffer size(buffer_size) is different from the element size(esize) since the sensor lower half normally generate the structural data stream. For examples, the custom sensor could define:
struct custem_event_s
{
uint64_t timestamp;
int16_t adc[8];
};
and call sensor_custom_register(lower, 1024, sizeof(struct custem_event_s), "/dev/sensor/custom0");
of course, the custom driver could pass 1 as esize if the generate the pure byte stream or variable size structural data.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528357663
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
Ok. Seems strange to me to allow partial writes of a fixed struct. Would make more sense to me to specify the number of elements and the element size. But that is unrelated to this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528356173
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
The buffer size(buffer_size) is different from the element size(esize) since the sensor lower half normally generate the structural data stream. For examples, the custom sensor could define:
struct custem_event_s
{
uint64_t timestamp;
int16_t adc[8];
};
and call sensor_custom_register(lower, 1024, sizeof(struct custem_event_s), "/dev/sensor/custom0");
Of course, the custom driver could pass 1 as esize if the driver generate the pure byte stream or variable size structural data.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528347287
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
I'm not following why do you need an extra function with an extra parameter when the lower half struct already gives this information directly. Isn't actually `lower->buffer_size` what is needed?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d merged pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
v01d merged pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528356173
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
The buffer size(buffer_size) is different from the element size(esize) since the sensor lower half normally generate the structural data stream. For examples, the custom sensor could define:
```
struct custem_event_s
{
uint64_t timestamp;
int16_t adc[8];
};
struct sensor_lowerhalf_s g_lower =
{
.buffer_size = 1024,
};
sensor_custom_register(lower, "/dev/sensor/custom0", sizeof(struct custem_event_s));
```
So buffer_size is 1024B and esize is 16B. Of course, the custom driver could pass 1 as esize if the driver generate the pure byte stream or variable size structural data.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528687562
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
Okay, I will modify this issue later.
##########
File path: drivers/sensors/sensor.c
##########
@@ -98,6 +99,7 @@ static int sensor_poll(FAR struct file *filep, FAR struct pollfd *fds,
static const struct sensor_info g_sensor_info[] =
{
+ {1, "custom"},
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528934845
##########
File path: include/nuttx/sensors/sensor.h
##########
@@ -532,7 +541,7 @@ struct sensor_ops_s
/**************************************************************************
* Name: control
*
- * In this method, we allow user to set some special config for the sensor,
+ * In this method, user can set some special config for the sensor,
Review comment:
```suggestion
* With this method, the user can set some special config for the sensor,
```
##########
File path: include/nuttx/sensors/sensor.h
##########
@@ -648,13 +657,14 @@ extern "C"
* "upper half" Sensor device and registers that device so that can be used
* by application code.
*
- * We will register the chararter device by node name format based on the
+ * You can register the chararter device by node name format based on the
Review comment:
```suggestion
* You can register the character device by node name format based on the
```
##########
File path: include/nuttx/sensors/sensor.h
##########
@@ -667,22 +677,65 @@ extern "C"
int sensor_register(FAR struct sensor_lowerhalf_s *dev, int devno);
+/****************************************************************************
+ * Name: sensor_custom_register
+ *
+ * Description:
+ * This function binds an instance of a "lower half" Sensor driver with the
+ * "upper half" Sensor device and registers that device so that can be used
+ * by application code.
+ *
+ * You can register the character device type by specific path and esize.
+ * This API corresponds to the sensor_custom_unregister.
+ *
+ * Input Parameters:
+ * dev - A pointer to an instance of lower half sensor driver. This
+ * instance is bound to the sensor driver and must persists as long
+ * as the driver persists.
+ * path - The user specifies path of device. ex: /dev/sensor/xxx.
+ * esize - The element size of intermediate circular buffer.
+ *
+ * Returned Value:
+ * OK if the driver was successfully register; A negated errno value is
+ * returned on any failure.
+ *
+ ****************************************************************************/
+
+int sensor_custom_register(FAR struct sensor_lowerhalf_s *dev,
+ FAR const char *path, uint8_t esize);
+
/****************************************************************************
* Name: sensor_unregister
*
* Description:
* This function unregister character node and release all resource about
- * upper half driver.
+ * upper half driver. This API corresponds to the sensor_register.
*
* Input Parameters:
* dev - A pointer to an instance of lower half sensor driver. This
- * instance is bound to the sensor driver and must persist as long
+ * instance is bound to the sensor driver and must persists as long
* as the driver persists.
* devno - The user specifies which device of this type, from 0.
****************************************************************************/
void sensor_unregister(FAR struct sensor_lowerhalf_s *dev, int devno);
+/****************************************************************************
+ * Name: sensor_custom_unregister
+ *
+ * Description:
+ * This function unregister character node and release all resource about
Review comment:
```suggestion
* This function unregisters the device node and releases all resource from
```
##########
File path: include/nuttx/sensors/sensor.h
##########
@@ -667,22 +677,65 @@ extern "C"
int sensor_register(FAR struct sensor_lowerhalf_s *dev, int devno);
+/****************************************************************************
+ * Name: sensor_custom_register
+ *
+ * Description:
+ * This function binds an instance of a "lower half" Sensor driver with the
+ * "upper half" Sensor device and registers that device so that can be used
+ * by application code.
+ *
+ * You can register the character device type by specific path and esize.
+ * This API corresponds to the sensor_custom_unregister.
+ *
+ * Input Parameters:
+ * dev - A pointer to an instance of lower half sensor driver. This
+ * instance is bound to the sensor driver and must persists as long
+ * as the driver persists.
+ * path - The user specifies path of device. ex: /dev/sensor/xxx.
+ * esize - The element size of intermediate circular buffer.
+ *
+ * Returned Value:
+ * OK if the driver was successfully register; A negated errno value is
+ * returned on any failure.
+ *
+ ****************************************************************************/
+
+int sensor_custom_register(FAR struct sensor_lowerhalf_s *dev,
+ FAR const char *path, uint8_t esize);
+
/****************************************************************************
* Name: sensor_unregister
*
* Description:
* This function unregister character node and release all resource about
- * upper half driver.
+ * upper half driver. This API corresponds to the sensor_register.
*
* Input Parameters:
* dev - A pointer to an instance of lower half sensor driver. This
- * instance is bound to the sensor driver and must persist as long
+ * instance is bound to the sensor driver and must persists as long
Review comment:
```suggestion
* instance is bound to the sensor driver and must persist as long
```
##########
File path: include/nuttx/sensors/sensor.h
##########
@@ -648,13 +657,14 @@ extern "C"
* "upper half" Sensor device and registers that device so that can be used
* by application code.
*
- * We will register the chararter device by node name format based on the
+ * You can register the chararter device by node name format based on the
* type of sensor. Multiple types of the same type are distinguished by
- * numbers. eg: accel0, accel1
+ * numbers. eg: accel0, accel1. This type of sensor must be less than
+ * SENSOR_TYPE_COUNT. This API corresponds to the sensor_unregister.
*
* Input Parameters:
* dev - A pointer to an instance of lower half sensor driver. This
- * instance is bound to the sensor driver and must persist as long
+ * instance is bound to the sensor driver and must persists as long
Review comment:
```suggestion
* instance is bound to the sensor driver and must persist as long
```
it was fine as it was
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528354573
##########
File path: drivers/sensors/sensor.c
##########
@@ -98,6 +99,7 @@ static int sensor_poll(FAR struct file *filep, FAR struct pollfd *fds,
static const struct sensor_info g_sensor_info[] =
{
+ {1, "custom"},
Review comment:
maybe change to zero if the value is not significant, as it is a more usual "invalid" value
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528508261
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
After discussing, the buffer number is better than buffer size, we will provide a new PR to change the unit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528347049
##########
File path: drivers/sensors/sensor.c
##########
@@ -98,6 +99,7 @@ static int sensor_poll(FAR struct file *filep, FAR struct pollfd *fds,
static const struct sensor_info g_sensor_info[] =
{
+ {1, "custom"},
Review comment:
what does this "1" signify?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528354442
##########
File path: drivers/sensors/sensor.c
##########
@@ -98,6 +99,7 @@ static int sensor_poll(FAR struct file *filep, FAR struct pollfd *fds,
static const struct sensor_info g_sensor_info[] =
{
+ {1, "custom"},
Review comment:
1 means the element size is one byte, but it can be any value since the first entry content isn't really used by the code. It is just a placehold to avoid other place reference the entry by (type + 1).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2369: driver/sensors: support custom types of sensor.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2369:
URL: https://github.com/apache/incubator-nuttx/pull/2369#discussion_r528356173
##########
File path: drivers/sensors/sensor.c
##########
@@ -651,22 +682,22 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
{
if (!lower->buffer_size)
{
- lower->buffer_size = g_sensor_info[lower->type].esize;
+ lower->buffer_size = esize;
+ }
+ else
+ {
+ lower->buffer_size = ROUNDUP(lower->buffer_size, esize);
}
lower->push_event = sensor_push_event;
}
else
{
lower->notify_event = sensor_notify_event;
+ lower->buffer_size = 0;
Review comment:
The buffer size(buffer_size) is different from the element size(esize) since the sensor lower half normally generate the structural data stream. For examples, the custom sensor could define:
```
struct custem_event_s
{
uint64_t timestamp;
int16_t adc[8];
};
struct sensor_lowerhalf_s g_lower =
{
.buffer_size = 1024,
};
sensor_custom_register(lower, "/dev/sensor/custom0", sizeof(struct custem_event_s));
```
So buffer_size is 1024B and esize is 16B. Of course, the custom driver could pass 1 as esize if the driver generate the pure byte stream or variable size structural data.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org