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

sensing: sensing.h: fix doxygen warnings #68317

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

lixuzha
Copy link
Collaborator

@lixuzha lixuzha commented Jan 31, 2024

Add missing parameter information in sensing_data_event_t.

@MaureenHelm
Copy link
Member

@kartben can you revisit this please?

@lixuzha lixuzha force-pushed the fix_doxygen_sensing branch 4 times, most recently from 1bee0f5 to 713cbfa Compare April 7, 2024 04:59
@lixuzha
Copy link
Collaborator Author

lixuzha commented Apr 7, 2024

Rebased the PR to latest code.

@lixuzha
Copy link
Collaborator Author

lixuzha commented Apr 22, 2024

@kartben Could you help to review this please?

teburd
teburd previously approved these changes Apr 22, 2024
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

thanks a lot for adding documentation here!
General comment is that some of the comments are not really providing explanation/context but O would be happy if only the "suggested changes" were addressed for now ; the rest can be improved at a later date :)

Thanks again!

include/zephyr/sensing/sensing.h Outdated Show resolved Hide resolved
Comment on lines 175 to 177
uint32_t interval; /**< Interval of the sensor configuration. */
uint32_t sensitivity; /**< Sensitivity of the sensor configuration. */
uint64_t latency; /**< Latency of the sensor configuration. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than "paraphrasing" the name of the attribute, it would be more helpful to also indicate e.g. the unit for each attribute.

include/zephyr/sensing/sensing.h Outdated Show resolved Hide resolved
include/zephyr/sensing/sensing_datatypes.h Outdated Show resolved Hide resolved
include/zephyr/sensing/sensing_datatypes.h Outdated Show resolved Hide resolved
include/zephyr/sensing/sensing_sensor.h Outdated Show resolved Hide resolved
include/zephyr/sensing/sensing_sensor.h Outdated Show resolved Hide resolved
include/zephyr/sensing/sensing_sensor.h Outdated Show resolved Hide resolved
include/zephyr/sensing/sensing_sensor.h Show resolved Hide resolved
include/zephyr/sensing/sensing_sensor.h Show resolved Hide resolved
@lixuzha
Copy link
Collaborator Author

lixuzha commented Apr 23, 2024

thanks a lot for adding documentation here! General comment is that some of the comments are not really providing explanation/context but O would be happy if only the "suggested changes" were addressed for now ; the rest can be improved at a later date :)

Thanks again!

Ok, thanks. I'll address them.

include/zephyr/sensing/sensing.h Outdated Show resolved Hide resolved
Add missing doxygen comments in various sensing headers.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
@nashif nashif merged commit 3cde0f7 into zephyrproject-rtos:main Apr 30, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants