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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 1, 2025
Merged

Apds9960 polling without interrupt pin #88624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea39521
drivers: sensor: Removed apds9960 interrupt pin
batkay Apr 23, 2025
dd4af1a
drivers: sensor: Make apds9960 polling wait
batkay Apr 23, 2025
ea6e076
drivers: sensor: Added timeout for apds9960 sampling
batkay Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions drivers/sensor/apds9960/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,26 @@ endchoice
config APDS9960_TRIGGER
bool

choice APDS9960_FETCH_MODE
prompt "Fetching Mode"
default APDS9960_FETCH_MODE_INTERRUPT
help
Specify whether to wait for interrupt

config APDS9960_FETCH_MODE_POLL
bool "Poll without interrupt pin"
help
Uses I2C to check sample

config APDS9960_FETCH_MODE_INTERRUPT
bool "Wait for interrupt"
depends on GPIO
depends on $(dt_compat_any_has_prop,$(DT_COMPAT_AVAGO_APDS9960),int-gpios)
help
Wait for interrupt before reading

endchoice

config APDS9960_ENABLE_ALS
bool "Ambient Light Sense"
default y
Expand Down
49 changes: 48 additions & 1 deletion drivers/sensor/apds9960/apds9960.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

LOG_MODULE_REGISTER(APDS9960, CONFIG_SENSOR_LOG_LEVEL);

#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
static void apds9960_handle_cb(struct apds9960_data *drv_data)
{
apds9960_setup_int(drv_data->dev->config, false);
Expand All @@ -45,20 +46,25 @@ static void apds9960_gpio_callback(const struct device *dev,

apds9960_handle_cb(drv_data);
}
#endif

static int apds9960_sample_fetch(const struct device *dev,
enum sensor_channel chan)
{
const struct apds9960_config *config = dev->config;
struct apds9960_data *data = dev->data;
uint8_t tmp;
#ifdef CONFIG_APDS9960_FETCH_MODE_POLL
int64_t start_time;
#endif

if (chan != SENSOR_CHAN_ALL) {
LOG_ERR("Unsupported sensor channel");
return -ENOTSUP;
}

#ifndef CONFIG_APDS9960_TRIGGER
#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
apds9960_setup_int(config, true);

#ifdef CONFIG_APDS9960_ENABLE_ALS
Expand All @@ -71,15 +77,40 @@ static int apds9960_sample_fetch(const struct device *dev,
LOG_ERR("Power on bit not set.");
return -EIO;
}

k_sem_take(&data->data_sem, K_FOREVER);
#endif
#endif

if (i2c_reg_read_byte_dt(&config->i2c,
APDS9960_STATUS_REG, &tmp)) {
return -EIO;
}

#ifdef CONFIG_APDS9960_FETCH_MODE_POLL
start_time = k_uptime_get();
#ifdef CONFIG_APDS9960_ENABLE_ALS
while (!(tmp & APDS9960_STATUS_AINT)) {
k_sleep(K_MSEC(APDS9960_DEFAULT_WAIT_TIME));
if (i2c_reg_read_byte_dt(&config->i2c, APDS9960_STATUS_REG, &tmp)) {
return -EIO;
}
if ((k_uptime_get() - start_time) > APDS9960_MAX_WAIT_TIME) {
return -ETIMEDOUT;
}
}
#else
while (!(tmp & APDS9960_STATUS_PINT)) {
k_sleep(K_MSEC(APDS9960_DEFAULT_WAIT_TIME));
if (i2c_reg_read_byte_dt(&config->i2c, APDS9960_STATUS_REG, &tmp)) {
return -EIO;
}
if ((k_uptime_get() - start_time) > APDS9960_MAX_WAIT_TIME) {
return -ETIMEDOUT;
}
Comment on lines +92 to +109
Copy link
Preview

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

[nitpick] The polling loops for ALS and non-ALS modes are nearly identical except for the status mask; consider refactoring the duplicated logic into a helper function that accepts the status mask as a parameter.

Suggested change
while (!(tmp & APDS9960_STATUS_AINT)) {
k_sleep(K_MSEC(APDS9960_DEFAULT_WAIT_TIME));
if (i2c_reg_read_byte_dt(&config->i2c, APDS9960_STATUS_REG, &tmp)) {
return -EIO;
}
if ((k_uptime_get() - start_time) > APDS9960_MAX_WAIT_TIME) {
return -ETIMEDOUT;
}
}
#else
while (!(tmp & APDS9960_STATUS_PINT)) {
k_sleep(K_MSEC(APDS9960_DEFAULT_WAIT_TIME));
if (i2c_reg_read_byte_dt(&config->i2c, APDS9960_STATUS_REG, &tmp)) {
return -EIO;
}
if ((k_uptime_get() - start_time) > APDS9960_MAX_WAIT_TIME) {
return -ETIMEDOUT;
}
if (apds9960_poll_status(&config->i2c, APDS9960_STATUS_AINT, start_time)) {
return -EIO;
}
#else
if (apds9960_poll_status(&config->i2c, APDS9960_STATUS_PINT, start_time)) {
return -EIO;

Copilot uses AI. Check for mistakes.

}
#endif
#endif

LOG_DBG("status: 0x%x", tmp);
if (tmp & APDS9960_STATUS_PINT) {
if (i2c_reg_read_byte_dt(&config->i2c,
Expand All @@ -99,12 +130,14 @@ static int apds9960_sample_fetch(const struct device *dev,
}

#ifndef CONFIG_APDS9960_TRIGGER
#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
if (i2c_reg_update_byte_dt(&config->i2c,
APDS9960_ENABLE_REG,
APDS9960_ENABLE_PON,
0)) {
return -EIO;
}
#endif
#endif

if (i2c_reg_write_byte_dt(&config->i2c,
Expand Down Expand Up @@ -352,9 +385,18 @@ static int apds9960_sensor_setup(const struct device *dev)
}
#endif

#ifdef CONFIG_APDS9960_FETCH_MODE_POLL
if (i2c_reg_update_byte_dt(&config->i2c, APDS9960_ENABLE_REG, APDS9960_ENABLE_PON,
APDS9960_ENABLE_PON)) {
LOG_ERR("Power on bit not set.");
return -EIO;
}
#endif

return 0;
}

#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
static int apds9960_init_interrupt(const struct device *dev)
{
const struct apds9960_config *config = dev->config;
Expand Down Expand Up @@ -400,6 +442,7 @@ static int apds9960_init_interrupt(const struct device *dev)

return 0;
}
#endif

#ifdef CONFIG_PM_DEVICE
static int apds9960_pm_action(const struct device *dev,
Expand Down Expand Up @@ -458,10 +501,12 @@ static int apds9960_init(const struct device *dev)
return -EIO;
}

#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
if (apds9960_init_interrupt(dev) < 0) {
LOG_ERR("Failed to initialize interrupt!");
return -EIO;
}
#endif

return 0;
}
Expand All @@ -477,7 +522,9 @@ static DEVICE_API(sensor, apds9960_driver_api) = {

static const struct apds9960_config apds9960_config = {
.i2c = I2C_DT_SPEC_INST_GET(0),
#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
.int_gpio = GPIO_DT_SPEC_INST_GET(0, int_gpios),
#endif
#if CONFIG_APDS9960_PGAIN_8X
.pgain = APDS9960_PGAIN_8X,
#elif CONFIG_APDS9960_PGAIN_4X
Expand Down
10 changes: 9 additions & 1 deletion drivers/sensor/apds9960/apds9960.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,15 @@
#define APDS9960_DEFAULT_GPULSE 0xC9
#define APDS9960_DEFAULT_GCONF3 0

/* Polling Wait Times (ms) */
#define APDS9960_DEFAULT_WAIT_TIME 2.78
Copy link
Preview

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

APDS9960_DEFAULT_WAIT_TIME is defined as a floating-point value, but K_MSEC typically expects an integer value; consider using an integer (e.g., 3) or explicitly handling conversion to avoid unexpected behavior.

Suggested change
#define APDS9960_DEFAULT_WAIT_TIME 2.78
#define APDS9960_DEFAULT_WAIT_TIME 3

Copilot uses AI. Check for mistakes.

#define APDS9960_MAX_WAIT_TIME 10000

struct apds9960_config {
struct i2c_dt_spec i2c;
#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
struct gpio_dt_spec int_gpio;
#endif
uint8_t pgain;
uint8_t again;
uint8_t ppcount;
Expand All @@ -231,11 +237,12 @@ struct apds9960_data {
#ifdef CONFIG_APDS9960_TRIGGER
sensor_trigger_handler_t p_th_handler;
const struct sensor_trigger *p_th_trigger;
#else
#elif CONFIG_APDS9960_FETCH_MODE_INTERRUPT
struct k_sem data_sem;
#endif
};

#ifdef CONFIG_APDS9960_FETCH_MODE_INTERRUPT
static inline void apds9960_setup_int(const struct apds9960_config *cfg,
bool enable)
{
Expand All @@ -245,6 +252,7 @@ static inline void apds9960_setup_int(const struct apds9960_config *cfg,

gpio_pin_interrupt_configure_dt(&cfg->int_gpio, flags);
}
#endif

#ifdef CONFIG_APDS9960_TRIGGER
void apds9960_work_cb(struct k_work *work);
Expand Down
1 change: 0 additions & 1 deletion dts/bindings/sensor/avago,apds9960.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ include: [sensor-device.yaml, i2c-device.yaml]
properties:
int-gpios:
type: phandle-array
required: true
description: Interrupt pin.

The interrupt pin of APDS9960 is open-drain, active low.
Expand Down