Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sensor: sensing: unified api #64478

Merged
merged 15 commits into from
Jan 24, 2024

Conversation

lixuzha
Copy link
Collaborator

@lixuzha lixuzha commented Oct 27, 2023

Implement the sensing subsystem.

This includes the following features:

  1. Unified API - Use sensor_driver_api instead of sensing_sensor_api to fix Duplicates APIs issue raised in RFC: Architecting a sensor subsystem #62223
  2. Implement the sensing subsystem base on unified API.
  3. Implement phy_3d_sensor driver as the reference code of physical sensor.
  4. Implement hinge_angle driver as the reference code of virtual sensor.

The sensing subsystem working process is as below.

    sequenceDiagram
    title Sensing runtime process

    participant Sensing as Sensing Subsys
    participant Sensing_dispatch as Sensing Dispatch Thread
    participant phy_3d_sensor
    participant hinge_angle_sensor
    participant Application

    Application-->>Sensing: call sensing_open_sensor to get hinge_angle_sensor's handle<br>and registe sensing_callback_list
    activate Application
    Application-->>Sensing: call sensing_set_config to set hinge_angle_sensor's sampling freq.
    activate hinge_angle_sensor
    Sensing-->>hinge_angle_sensor: call .attr_set to set sampling freq.
    hinge_angle_sensor-->>Sensing: call sensing_set_config to config its reporters.
    Sensing-->>phy_3d_sensor: call .attr_set to set sampling freq.
    deactivate hinge_angle_sensor
    deactivate Application

    rect rgb(191, 223, 255)
    loop
        Sensing-->>hinge_angle_sensor: Call its .submit callback to submit SQE periodically.
        Sensing-->>phy_3d_sensor: Call its .submit callback  to submit SQE periodically.
        phy_3d_sensor-->>Sensing_dispatch: Produce the CQE.
        Sensing_dispatch-->>hinge_angle_sensor: Consume CQE produced by phy_3d_sensor<br>and dispatch data to it's clients by calling sensing_callback_list.on_data_event()
        hinge_angle_sensor-->>Sensing_dispatch: Produce the CQE.
        Sensing_dispatch-->>Application: Consume CQE produced by hinge_angle_sensor<br>and dispatch data to it's clients by calling sensing_callback_list.on_data_event()
    end
    end
Loading

Depends: #60063

Links

Copy link
Collaborator

@teburd teburd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like that we're not adding a new API, that it appears to mostly avoid copying the data around, and that there is a simple callback list involved.

I like that even algorithms are effectively using a pull then push model. As I read the code a read submission comes in from the sensor subsystem and upon completion data is broadcast to the callback list. The read for algorithms is completed once enough callbacks (on data event) occur and the algorithm runs. This feels and looks pretty nice to me.

I think many things here could use with some more code comments and documentation explaining how things are working, but to me this looks like its on the right track.

I think more broadly though that each sensing_sensor needs to connect a sensor to multiple lists of callbacks rather than a single list of callbacks. The assumption that I believe should be made here is that a sensor or algorithm may be a source of multiple data streams (e.g. the muxed FIFO issue noted in #60063) and that for each channel (accel xyz could be a channel) a set of callbacks are associated.

I could very well be wrong on that but I guess that's how I'd expect things to work in some way or another.

while (true) {
struct rtio_cqe cqe;

rc = rtio_cqe_copy_out(&sensing_rtio_ctx, &cqe, 1, K_FOREVER);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice usage of the user space abilities here


static inline uint64_t get_us(void)
{
return k_ticks_to_us_floor64(k_uptime_ticks());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ticks are likely going to represent in the most common case millisecond levels of precision. So I'm skeptical that this get_us(void) function will be able to do what we need it to.

Likely what we want is k_cycle_get_64() and converting the cycle count to microseconds. Or perhaps doing some math to get the cycles since the last tick (maybe we need this functionality) so that sub-tick timing precision without rollovers can be obtained. This would be useful for sensor drivers as well I believe as they need a way to precisely timestamp the data.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I'm OK with timing for now just moving forward with more of a "whatever" until #63651 is resolved. I think we're likely to land on cycles like @teburd said.

* true: it's time for client consuming the data
* false: client time not arrived yet, not consume the data
*/
if (!sensor_test_consume_time(sensor, conn, get_us())) {
Copy link
Collaborator

@teburd teburd Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it would drop data if the consumer is not yet expecting it based on get_us(). I'd note that ticks are imprecise and this could fail to pass the branch in cases that we might otherwise expect. I'd also note that this is going to decimate data without filtering and this was a concern given in the architecture wg. I don't have a strong opinion on the filtering aspect, but we should definitely ensure the timing precision is good so this sort of branching occurs when we want it to.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we discussed this in the AWG before we created the sensor WG. We shouldn't do any filtering in the subsystem. It's better to present the client with ALL the data that's available and let them choose a decimation strategy.

conn->source->dev->name);
continue;
}
conn->callback_list.on_data_event(conn, data,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like that we do not need to copy data as the callbacks are happening

{
struct hinge_angle_context *data = dev->data;

if (data->sqe) {
Copy link
Collaborator

@teburd teburd Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting idea though I'm not sure I follow entirely the thinking. In this case submit() is expecting to set the current submission the hinge angle algorithm works on. It can do this because there's the knowledge that only the sensor dispatch thread is going to be submitting presumably. So only one read request is ever going to be around I suppose.

I'd rather see this work like everything else and push/pop from a queue. In moving away from submit starting work as it does today I have #62605. In this case submit would push to the queue, the current sqe to fulfill would be setup in a poll call in that case, and only done if in submit it is needed.

e.g.

this would look more like..

/* All request producers call submit */
static void hinge_submit(const struct device *dev, struct rtio_iodev_sqe *sqe)
{
   ...
   
   rtio_mpsc_push(iodev->sq, sqe);
   
   if (!atomic_ptr_get(data->sqe)) {
      rtio_add_pollable(sqe->r, iodev);
   }
}

/* Request consumer (the iodev) does its work in poll */
static void hinge_poll(struct const device *dev)
{
   ...
   
   data->sqe = rtio_mpsc_pop(iodev->sq);
   
   /* Do work here if possible */
}

return ret;
}

static int hinge_submit(const struct device *dev,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an infallible call and should be void hinge_submit(const struct device *dev, struct rtio_iodev_sqe *sqe);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hinge_submit is an instance of typedef int (*sensor_submit_t)(const struct device *sensor, struct rtio_iodev_sqe *sqe), so it should be int.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cleanup needs to be done around this API it seems, thanks for pointing this out

@KeHIntel
Copy link
Collaborator

KeHIntel commented Nov 2, 2023

I think more broadly though that each sensing_sensor needs to connect a sensor to multiple lists of callbacks rather than a single list of callbacks. The assumption that I believe should be made here is that a sensor or algorithm may be a source of multiple data streams (e.g. the muxed FIFO issue noted in #60063) and that for each channel (accel xyz could be a channel) a set of callbacks are associated.

Thanks for the review, on above requirement, we have a little different concept. In our view, the sensors in sensing subsystem should be a standard part instead of multi-function tools. A standard part means that it has standard input/output defined, no matter it is built by whom. A standard part can help people easily assembly them together into a sensor tree and create a functional sensing subsystem, like playing LEGO blocks.

So for the case that one sensor may be a source of multiple data streams, the preference is to split it into multiple standard sensors, each to serve one type of usages.

@lixuzha
Copy link
Collaborator Author

lixuzha commented Nov 2, 2023

I really like that we're not adding a new API, that it appears to mostly avoid copying the data around, and that there is a simple callback list involved.

Thanks for your comments. I'll update it later.

Copy link
Collaborator

@yperess yperess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase, streaming APIs landed.

int64_t scaled_value;
int64_t shifted_value;

shifted_value = (int64_t)q << shift;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to be careful with this it caught me off guard a few times, if shift is negative this will not work as expected. you need to left shift on positive and right shift by the absolute value on negative.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your reminder. This function can be removed after using sensor_read and decoder instead of fetch/get

Comment on lines 203 to 211
ret = sensor_sample_fetch_chan(cfg->hw_dev, data->custom->chan_all);
if (ret) {
LOG_ERR("%s: sample fetch failed: %d", dev->name, ret);
rtio_iodev_sqe_err(sqe, ret);
return ret;
}

ret = sensor_channel_get(cfg->hw_dev, data->custom->chan_all, value);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be using fetch/get anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Would update later.

Comment on lines 248 to 259
static struct phy_3d_sensor_data _CONCAT(data, _inst); \
static const struct phy_3d_sensor_config _CONCAT(cfg, _inst) = {\
.hw_dev = DEVICE_DT_GET( \
DT_PHANDLE(DT_DRV_INST(_inst), \
underlying_device)), \
.sensor_type = DT_PROP(DT_DRV_INST(_inst), sensor_type),\
}; \
SENSING_SENSOR_DT_DEFINE(DT_DRV_INST(_inst), \
&phy_3d_sensor_reg, &_CONCAT(ctx, _inst), \
SENSING_SENSOR_DT_INST_DEFINE(_inst, &phy_3d_sensor_reg, NULL, \
&phy_3d_sensor_init, NULL, \
&_CONCAT(data, _inst), &_CONCAT(cfg, _inst), \
POST_KERNEL, CONFIG_SENSOR_INIT_PRIORITY, \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a critical nuance, the physical sensors should for example when using the ICM42688 create:

  1. sensor device (const struct device)
  2. 3 separate const struct sensing_sensor_info objects for ACCEL, GYRO, and DIE_TEMPERATURE.

The current model creates a separate device for each data type

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll fix it.

I would like to do the following change:

  1. Change the type of sensor-type from int to array
# Copyright (c) 2023, Intel Corporation
# SPDX-License-Identifier: Apache-2.0
#

description: Sensing subsystem sensor common properties bindings.

include: sensor-device.yaml

properties:
---  sensor-type:
+++  sensor-types:
---    type: int
+++    type: array
    required: true
    description: sensor type id (follow HID spec definition)
  1. In SENSING_SENSOR_DEFINE, it will create multiple instances of sensing_sensor by enumerating sensor-types.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a key distinction here, you need 1 sensor but multiple sensor_info nodes. Please see my sample PR at #56963

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR, and add the commit sensing: support multiple sensor types in one device
In this commit, the single bmi160 device would create two sensing_sensor instances for Accel and Gyro.

@nashif
Copy link
Member

nashif commented Nov 13, 2023

there is an issue with llvm:

zephyr/subsys/sensing/CMakeFiles/subsys__sensing.dir/sensor/hinge_angle/hinge_angle.c.obj -c /__w/zephyr/zephyr/subsys/sensing/sensor/hinge_angle/hinge_angle.c
/__w/zephyr/zephyr/subsys/sensing/sensor/hinge_angle/hinge_angle.c:167:29: error: initializer element is not a compile-time constant
DT_INST_FOREACH_STATUS_OKAY(SENSING_HINGE_ANGLE_DT_DEFINE);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/__w/zephyr/zephyr/include/zephyr/devicetree.h:4153:25: note: expanded from macro 'DT_INST_FOREACH_STATUS_OKAY'
                              DT_DRV_COMPAT)(fn)),              \
                              ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/__w/zephyr/zephyr/twister-out/native_posix/samples/subsys/sensing/simple/sample.subsys.sensing.simple/zephyr/include/generated/devicetree_generated.h:3862:61: note: expanded from macro 'DT_FOREACH_OKAY_INST_zephyr_sensing_hinge_angle'
#define DT_FOREACH_OKAY_INST_zephyr_sensing_hinge_angle(fn) fn(0)
                                                            ^
/__w/zephyr/zephyr/subsys/sensing/sensor/hinge_angle/hinge_angle.c:160:2: note: expanded from macro 'SENSING_HINGE_ANGLE_DT_DEFINE'
        SENSING_SENSOR_DT_INST_DEFINE(_inst, &hinge_reg,                        \
        ^
note: (skipping 11 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
/__w/zephyr/zephyr/include/zephyr/sys/util_internal.h:64:39: note: expanded from macro '__COND_CODE'
        __GET_ARG2_DEBRACKET(one_or_two_args _if_code, _else_code)
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
/__w/zephyr/zephyr/include/zephyr/sys/util_internal.h:69:65: note: expanded from macro '__GET_ARG2_DEBRACKET'
#define __GET_ARG2_DEBRACKET(ignore_this, val, ...) __DEBRACKET val
                                                    ~~~~~~~~~~~~^~~
/__w/zephyr/zephyr/include/zephyr/sys/util_internal.h:72:26: note: expanded from macro '__DEBRACKET'
#define __DEBRACKET(...) __VA_ARGS__
                         ^~~~~~~~~~~
1 error generated.

@lixuzha lixuzha force-pushed the sensing_unified_api branch 2 times, most recently from 5dc27cd to b4aeea1 Compare November 27, 2023 08:13
@lixuzha
Copy link
Collaborator Author

lixuzha commented Nov 27, 2023

there is an issue with llvm:

zephyr/subsys/sensing/CMakeFiles/subsys__sensing.dir/sensor/hinge_angle/hinge_angle.c.obj -c /__w/zephyr/zephyr/subsys/sensing/sensor/hinge_angle/hinge_angle.c
/__w/zephyr/zephyr/subsys/sensing/sensor/hinge_angle/hinge_angle.c:167:29: error: initializer element is not a compile-time constant
DT_INST_FOREACH_STATUS_OKAY(SENSING_HINGE_ANGLE_DT_DEFINE);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/__w/zephyr/zephyr/include/zephyr/devicetree.h:4153:25: note: expanded from macro 'DT_INST_FOREACH_STATUS_OKAY'
                              DT_DRV_COMPAT)(fn)),              \
                              ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/__w/zephyr/zephyr/twister-out/native_posix/samples/subsys/sensing/simple/sample.subsys.sensing.simple/zephyr/include/generated/devicetree_generated.h:3862:61: note: expanded from macro 'DT_FOREACH_OKAY_INST_zephyr_sensing_hinge_angle'
#define DT_FOREACH_OKAY_INST_zephyr_sensing_hinge_angle(fn) fn(0)
                                                            ^
/__w/zephyr/zephyr/subsys/sensing/sensor/hinge_angle/hinge_angle.c:160:2: note: expanded from macro 'SENSING_HINGE_ANGLE_DT_DEFINE'
        SENSING_SENSOR_DT_INST_DEFINE(_inst, &hinge_reg,                        \
        ^
note: (skipping 11 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
/__w/zephyr/zephyr/include/zephyr/sys/util_internal.h:64:39: note: expanded from macro '__COND_CODE'
        __GET_ARG2_DEBRACKET(one_or_two_args _if_code, _else_code)
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
/__w/zephyr/zephyr/include/zephyr/sys/util_internal.h:69:65: note: expanded from macro '__GET_ARG2_DEBRACKET'
#define __GET_ARG2_DEBRACKET(ignore_this, val, ...) __DEBRACKET val
                                                    ~~~~~~~~~~~~^~~
/__w/zephyr/zephyr/include/zephyr/sys/util_internal.h:72:26: note: expanded from macro '__DEBRACKET'
#define __DEBRACKET(...) __VA_ARGS__
                         ^~~~~~~~~~~
1 error generated.

I'll check it, thanks.

yperess
yperess previously requested changes Dec 7, 2023
* true: it's time for client consuming the data
* false: client time not arrived yet, not consume the data
*/
if (!sensor_test_consume_time(sensor, conn, get_us())) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we discussed this in the AWG before we created the sensor WG. We shouldn't do any filtering in the subsystem. It's better to present the client with ALL the data that's available and let them choose a decimation strategy.

Comment on lines +69 to +74
static const struct phy_3d_sensor_custom custom_accel = {
.chan_all = SENSOR_CHAN_ACCEL_XYZ,
.shift = SENSING_ACCEL_Q31_SHIFT,
.q31_to_sensor_value = accel_q31_to_sensor_value,
.sensor_value_to_q31 = accel_sensor_value_to_q31,
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like we're still making the phy_3d_sensor an either/or. In the case of the BMI160 you'll need to support both accel and gyro here.

Comment on lines 106 to 114
switch (cfg->sensor_type) {
case SENSING_SENSOR_TYPE_MOTION_ACCELEROMETER_3D:
data->custom = &custom_accel;
break;
case SENSING_SENSOR_TYPE_MOTION_GYROMETER_3D:
data->custom = &custom_gyro;
break;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell this is still the same as my previous comments. You're creating 1 struct device per type. You need to have 1 struct device per physical sensor and N struct sensor_info objects based on the number of types that are supported.

.hw_dev = DEVICE_DT_GET( \
DT_PHANDLE(DT_DRV_INST(_inst), \
underlying_device)), \
.sensor_type = DT_PROP(DT_DRV_INST(_inst), sensor_type),\
.sensor_type = DT_PROP_BY_IDX(DT_DRV_INST(_inst), sensor_types, 0),\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just using the first type in the list.

@teburd
Copy link
Collaborator

teburd commented Dec 15, 2023

Looks like some minor lint issues on the commits and some code lines are preventing CI from being green, please fix this as usually people won't review without a green CI pass

ghu0510 and others added 13 commits December 18, 2023 13:40
1) move variable shift behind struct sensing_sensor_value_header in
struct sensing_sensor_value_q31
2) add pointer checking for sensing API
3) add context sensing_callback_list and sensing_data_event_t
4) add macro SENSING_SENSITIVITY_INDEX_ALL
5) rename zephyr,sensing-phy-3d-sensor.yaml

Signed-off-by: Guangfu Hu <guangfu.hu@intel.com>
1. use the sensor_driver_api instead of the sensing_sensor_api.
2. clean up the sensing_sensor.h after using the unified api.
3. select RTIO while CONFIG_SENSING is enabled.
4. remove the sensing_sensor_post_data and sensing_sensor_xxx_data_ready by
introducing the RTIO

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
1. set/get interval sensitivity is implmented in sensor management module.
2. add the runtime thread to handle the sensor configuration.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Create the dispatch thread to send sensor data to its client by
checking time.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Add two helper functions as the couple functions of sensor_value_to_milli
and sensor_value_to_micro.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
This patch is to implement the phy_3d_sensor in sensing subsystem. It's a
wrapper layer of AGM sensor driver for sensing subsystem. It supports
the accelerometer and gyrometer now.
This has been tested with bmi160 on native_posix simulator.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Add stream-mode property to indicate sensing sensor working stream mode or
poll mode.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Add hinge angle virtual sensor for sensing subsystem, hinge angle sensor
takes both base accel and lid accel as reporter.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
1. add sensing_set_config/sensing_get_config in sample code
2. add hinge_angle sensor in sample code.

Build and test:
  west build -p -b native_sim samples/subsys/sensing/simple/ -t run

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Many sensors have multiple functions, for example, icm42688 supports
accel, gyro and temperature, and the sensor streaming api always mixes
the multiple functions in one function call. So we need add a layer in
sensing subsystem to dispatch the result returned from sensor streaming
api for each function.
I changed the sensor-type(int) to sensor-types(array) in sensing sensor
device bindings, so that one device can map to multiple instances of
sensing sensor.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Some doc comments were making doxygen unhappy and the doc build fail.
The description of the sensor connection was also updated slightly to
simplify the description a bit.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
Fixes a few small code formatting issues in sensor_mgmt.c adding
brackets where needed for an if and droping a line continuation slash
that failed one of the CI lints.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
Initializers must be compile time constants (not expressions!) for clang
to be happy. Clang rightfully pointed out that the callback_list member
of sensing_connection was being initialized using an expression that was
not compile-time constant.

The macros passed along a pointer created by taking the address of a
global constant and then at initialization time attempting to
dereference it using the * operator. So in the end the compiler has
identifier its trying to first take the address of and then later
dereference in an initializer. Clang didn't appreciate this, though gcc
seemed to be just fine.

Actually clang 17 might work just fine with this as well, but clang 16
(the clang I tried) didn't like it at all.

Instead store the pointer to the callback list rather than copying the
struct. This could be done the other way and not passing a
pointer through the macros perhaps.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
The sensing_connections can be shared between sensing_sensors under the
same device.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
1. add function sensing_iodev_submit and defined struct
   sensing_submit_config
2. fix the issue of phy_3d_sensor

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
@lixuzha
Copy link
Collaborator Author

lixuzha commented Dec 18, 2023

Looks like some minor lint issues on the commits and some code lines are preventing CI from being green, please fix this as usually people won't review without a green CI pass

Fixed.

@nashif nashif dismissed yperess’s stale review January 8, 2024 19:52

Please revisit and re-review.

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to the sensing API look good to me

@carlescufi carlescufi merged commit 5058355 into zephyrproject-rtos:main Jan 24, 2024
18 checks passed
@@ -110,7 +115,8 @@ typedef void *sensing_sensor_handle_t;
*/
typedef void (*sensing_data_event_t)(
sensing_sensor_handle_t handle,
const void *buf);
const void *buf,
void *context);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs doxygen comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Submitted #68317 to fix it.

Comment on lines 152 to +153
sensing_data_event_t on_data_event;
void *context;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need doxygen comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Samples Samples area: Sensor Subsystem area: Sensors Sensors
Projects
Development

Successfully merging this pull request may close these issues.

10 participants