Skip to content

Apds9960 polling without interrupt pin #88624

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

batkay
Copy link

@batkay batkay commented Apr 15, 2025

Description
This change allows for I2C communication with APDS9960 sensor without using the interrupt pin. If a sample is not available, it will block for the default amount of time between samples (2.78ms) before trying again. This allows it to work with boards like the Adafruit APDS9960 breakout with a QWIIC connector.

Specific Changes:

  • Made interrupt pin assignment optional
  • Added new configuration option to disable the interrupt
  • Use status register to determine whether a sample is ready or not
  • Sleep if a sample is not ready, and try again in non interrupt mode

By default, interrupt pin is enabled when the int-gpios property is provided so older projects are not effected.

Testing
Build the sample APDS9960 sensor code with CONFIG_APDS9960_INTERRUPT_PIN set to n and CONFIG_APDS9960_TRIGGER_GLOBAL_THREAD set to n, or alternatively delete the property int-gpios from the devicetree overlay.

Logic analyzer shows i2c checking status, and waiting if a sample is not ready
i2c_wait

RTT viewer shows the proximity and ambient light data being printed (same as before behavior)
rtt_viewer

Copy link

Hello @batkay, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

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.

Thanks for your contribution and for the detailed testing proof. I've left some comments I've caught at a glance. Let me know your thoughts!

@batkay batkay force-pushed the apds9960_polling branch from 53baef1 to 9a9a41f Compare April 21, 2025 04:52
@batkay batkay requested a review from ubieda April 21, 2025 14:12
@batkay batkay force-pushed the apds9960_polling branch 2 times, most recently from f6edff8 to a76fd92 Compare April 21, 2025 19:14
@ubieda
Copy link
Member

ubieda commented Apr 21, 2025

Specific Changes:

  • Made interrupt pin assignment optional
  • Added new configuration option to disable the interrupt
  • Use status register to determine whether a sample is ready or not
  • Sleep if a sample is not ready, and try again in non interrupt mode

By default, interrupt pin is enabled when the int-gpios property is provided so older projects are not effected.

Also, it seems some changes got lost in the way. Please revisit the commits and their contents. As it is right now, it seems only the driver changes are present in the PR.

@batkay batkay force-pushed the apds9960_polling branch 2 times, most recently from 9541f81 to b045a97 Compare April 21, 2025 21:58
@batkay
Copy link
Author

batkay commented Apr 22, 2025

@ubieda Just as a response to the previous comment, some of the changes are in the same commits. For example, making the interrupt optional and creating config options to enable/ disable its functionality were in the same commit since it didn't make sense (in my opinion) to break those up.

@batkay batkay force-pushed the apds9960_polling branch 3 times, most recently from 6f1f063 to d30ab1e Compare April 23, 2025 20:03
ubieda
ubieda previously approved these changes Apr 23, 2025
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.

The changes seem to meet the initial review.

I'm a bit concerned that apds9960_sample_fetch() is turning cumbersome with a bunch of ifdef directives, but I acknowledge it was like that to begin with.

Let's wait for other reviewers to chime in. Thanks for your contribution!

Made interrupt pin on apds9960 optional

Signed-off-by: Thomas Lang <[email protected]>
batkay added 2 commits April 23, 2025 16:46
sensor_fetch for apds9960 will now block until a valid sample is ready

Signed-off-by: Thomas Lang <[email protected]>
Timeout occurs if status register does not register an interrupt

Signed-off-by: Thomas Lang <[email protected]>
@batkay batkay force-pushed the apds9960_polling branch from 2c36d02 to ea6e076 Compare April 23, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants