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

sensors: Add channel specifiers #71093

Merged
merged 1 commit into from
May 17, 2024

Conversation

teburd
Copy link
Collaborator

@teburd teburd commented Apr 4, 2024

Use a structured channel specifier rather than a single enum when specifying channels to read in the new read/decoder API.

Implements a slight variation of #63830

yperess
yperess previously requested changes Apr 4, 2024
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.

Seems reasonable to me, please run clang-format and push.

@teburd teburd force-pushed the sensor_api_chan_spec branch 5 times, most recently from 1084abb to f5bb842 Compare April 5, 2024 16:31
@tristan-google
Copy link
Collaborator

If it's not too much trouble it'd be nice to separate the clang-formatting into a different commit. The new channel struct looks good to me.

@teburd
Copy link
Collaborator Author

teburd commented Apr 5, 2024

If it's not too much trouble it'd be nice to separate the clang-formatting into a different commit. The new channel struct looks good to me.

yeah of course, I can split the commits or drop the unchanged areas that clang-format touched as a whole... seems clang-format version dependent on what it does sometimes

@teburd teburd force-pushed the sensor_api_chan_spec branch from 0ed3ff9 to 8274a4d Compare April 6, 2024 14:35
@teburd teburd requested a review from yperess April 6, 2024 14:54
@teburd teburd force-pushed the sensor_api_chan_spec branch 6 times, most recently from ddbc1cd to 55f461a Compare April 10, 2024 16:15
@teburd
Copy link
Collaborator Author

teburd commented Apr 10, 2024

Has turned into a bit of a minor rabbit hole and might not finish before ZDS at this point

@teburd teburd force-pushed the sensor_api_chan_spec branch 2 times, most recently from b7426ac to 01c66ec Compare April 10, 2024 17:29
@teburd teburd force-pushed the sensor_api_chan_spec branch 2 times, most recently from fd46712 to eae752b Compare April 22, 2024 16:55
@teburd teburd requested a review from str4t0m May 7, 2024 13:21
laurenmurphyx64 added a commit to laurenmurphyx64/zephyr that referenced this pull request May 8, 2024
Updates dht_polling sample to new sensor API.

Depends on zephyrproject-rtos#71093 and zephyrproject-rtos#71160.

Signed-off-by: Lauren Murphy <lauren.murphy@intel.com>
@teburd teburd force-pushed the sensor_api_chan_spec branch from 06e35e1 to 0e684a0 Compare May 9, 2024 16:30
@teburd
Copy link
Collaborator Author

teburd commented May 9, 2024

Update 4: Fix sensor shell build issues

ubieda
ubieda previously approved these changes May 10, 2024
Copy link
Member

@ubieda ubieda left a comment

Choose a reason for hiding this comment

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

I still see the issue with the Sensor Shell command, however, it's easy to fix and there are many PRs depending on this one. I'm approving it since I'm good with merging in and follow-up with subsequent PRs to complement it.

PR with Shell fix: #72542

drivers/sensor/bosch/bma4xx/bma4xx.c Outdated Show resolved Hide resolved
drivers/sensor/bosch/bma4xx/bma4xx.c Outdated Show resolved Hide resolved
str4t0m
str4t0m previously approved these changes May 13, 2024
Copy link
Collaborator

@str4t0m str4t0m left a comment

Choose a reason for hiding this comment

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

My concerns have been addressed, thanks!

It may be best to include the sensor shell fix from 72542 here already, but leaving that to the sensor maintainers.

@teburd teburd requested a review from MaureenHelm May 14, 2024 15:41
@henrikbrixandersen henrikbrixandersen self-requested a review May 15, 2024 14:30
@teburd teburd dismissed stale reviews from str4t0m and ubieda via 375b0e6 May 16, 2024 14:08
@teburd teburd force-pushed the sensor_api_chan_spec branch 2 times, most recently from 375b0e6 to 8a72f32 Compare May 16, 2024 14:08
@teburd
Copy link
Collaborator Author

teburd commented May 16, 2024

Update 5

  • Revert some inadvertent variable renames that had nothing to do with the channel spec changes
  • Fix struct member indentation (ugh)
  • Add a note and build assert around why chan specs are passed by value

@teburd teburd force-pushed the sensor_api_chan_spec branch 2 times, most recently from 41a7816 to b1c0e7e Compare May 16, 2024 14:17
Use a structured channel specifier rather than a single enum when
specifying channels to read in the new read/decoder API.

Replaces usages of a seperate channel and channel_index parameter
where previously used with a struct sensor_chan_spec.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
@teburd teburd force-pushed the sensor_api_chan_spec branch from b1c0e7e to c8df175 Compare May 16, 2024 21:04
@carlescufi carlescufi merged commit b249535 into zephyrproject-rtos:main May 17, 2024
21 checks passed
@teburd teburd deleted the sensor_api_chan_spec branch May 17, 2024 11:47
laurenmurphyx64 added a commit to laurenmurphyx64/zephyr that referenced this pull request May 28, 2024
Updates dht_polling sample to new sensor API.

Depends on zephyrproject-rtos#71093 and zephyrproject-rtos#71160.

Signed-off-by: Lauren Murphy <lauren.murphy@intel.com>
laurenmurphyx64 added a commit to laurenmurphyx64/zephyr that referenced this pull request May 30, 2024
Updates dht_polling sample to new sensor API.

Depends on zephyrproject-rtos#71093 and zephyrproject-rtos#71160.

Signed-off-by: Lauren Murphy <lauren.murphy@intel.com>
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.

9 participants