sensor: add TI FDC2X1X sensor driver and sample application #31056

skullbox305 · 2020-12-30T16:03:54Z

This patch adds support for the Texas Instruments FDC2X1X Capacitance-to-Digital Converter for Proximity and Level Sensing Applications.
Along with the driver, a sample sensor application is provided which tests the device driver.

Signed-off-by: Igor Knippenberg igor.knippenberg@gmail.com

skullbox305 · 2021-01-22T19:19:16Z

One more thing: the driver must at least be build-tested by modifying tests/drivers/build_all.

Did you miss this one?

No, I made the changes to tests/drivers/build_all. Did I forget to add something?

MaureenHelm

Overall really nice work, thank you for contributing!

MaureenHelm · 2021-01-22T19:44:17Z

include/drivers/sensor.h

+	/** Channel 0, raw data */
+	SENSOR_CHAN_RAW_CH0,
+	/** Channel 1, raw data */
+	SENSOR_CHAN_RAW_CH1,
+	/** Channel 2, raw data */
+	SENSOR_CHAN_RAW_CH2,
+	/** Channel 3, raw data */
+	SENSOR_CHAN_RAW_CH3,
+


I'm not a fan of adding channels without a type or standard unit. What does the quantity represent?

mbolivar-nordic · 2021-01-23T02:16:18Z

No, I made the changes to tests/drivers/build_all. Did I forget to add something?

Sorry; I don't know how I missed that. Thanks.

addressed

skullbox305 · 2021-01-24T01:51:27Z

Overall really nice work, thank you for contributing!

Thank you!
I added the necessary conversion from raw to capacitance (pF) and frequency (MHz).

pabigot

Some thoughts on sensor units. Note that we're now in 2.5 stabilization so this won't get merged until the 2.6 merge window re-opens.

pabigot · 2021-01-25T14:39:52Z

include/drivers/sensor.h

+	/** CH3 Capacitance, in Picofarad **/
+	SENSOR_CHAN_CAPACITANCE_CH3,
+
+	/** CH0 Frequency, in MHz **/


I'm not sure MHz is an ideal unit. Hz would allow for frequencies like 0.5 Hz, i.e. every two seconds; or 0.000012 (a very inaccurate once-per-day).

This would limit the frequency to 2 GHz, though, which might be limiting in some applications. Perhaps a future sensor API could select the best unit based on the value, and return either frequency in Hz or interval in some time unit, depending on which is more accurate.

This could be discussed in the API meeting. Sticking with MHz or even kHz would be a reasonable resolution of such a discussion.

I think you're right. Hz would make more sense. I'm not sure if >2GHz would be necessary for sensors, but who knows. Would it be possible to have both units, Hz now and MHz later if needed? This way you could use Hz for <2GHz and MHz for values above. Also I agree with your other suggestion. Having the flexibility to get the value returned in the most accurate unit would be really nice.

I will wait for the resolution of the API meeting though and then change it based on that. Could you give me an update on the resolution?

How about converting these sensor channel enums to be private to the FDC2X1X driver instead? Similar to how it is done for the MCUX ACMP sensor driver in

zephyr/include/drivers/sensor/mcux_acmp.h

Line 21 in f71cd2a

enum sensor_channel_mcux_acmp {

?

API meeting 26th Jan 2021:

Moving the enums to the driver-specific header is the concluded best approach for this.

Thank you both for your suggestion! I moved the channel enums to the driver-specific header now

mbolivar-nordic · 2021-02-23T22:11:32Z

drivers/sensor/fdc2x1x/fdc2x1x.c

+	};								   \
+									   \
+	static const struct fdc2x1x_config fdc2x1x_config_##n = {	   \
+		.i2c_name = DT_INST_BUS_LABEL(n),			   \


Hi again. I just see one more round of churn before this PR LGTM from a driver model perspective.

We are making changes to the way that sensor devices access their buses for Zephyr 2.6. These mean that getting them by name using device_get_binding is discouraged.

You should now store the device directly in the configuration structure in ROM by moving these fields to struct fdc2x1x_config from struct fdc2x1x_data:

const struct device *bus; const struct device *gpio;

These should now be initialized using DEVICE_DT_GET. For example, .bus = DEVICE_DT_GET(DT_INST_BUS(n)).

You should then check the device in the init routine using device_is_ready.

Finally, fields like i2c_name which exist only to store devicetree label properties should be removed. You can use bus->name instead if you need a name.

For an example, see the drivers/led/led_pwm.c changes in #32558.

Thanks for your patience!

Hi. I addressed your comments. The bus and gpio devices are now initialized with DEVICE_DT_GET and checked with device_is_ready. I also removed the i2c_name and both gpio labels.

mbolivar-nordic

Thanks! Sorry for the delay.

carlescufi · 2021-03-16T09:01:28Z

@skullbox305 could you please rebase?

Adds support for the Texas Instruments FDC2X1X Capacitance-to-Digital Converter for Proximity and Level Sensing Applications. Signed-off-by: Igor Knippenberg <igor.knippenberg@gmail.com>

This sample application periodically reads raw data from the FDC2X1X sensor in polling mode or optionally with data ready trigger. It is able to read the 12-Bit and 28-Bit, as well as the 2-Channel and 4-Channel versions (FDC2112, FDC2114, FDC2212, FDC2214). The 4-channel versions are chosen through devicetree properties. The default is FDC2112. Signed-off-by: Igor Knippenberg <igor.knippenberg@gmail.com>

skullbox305 · 2021-03-16T13:33:59Z

@mbolivar-nordic Not a problem. Thank you and the others for the review! It was really helpful
@carlescufi I rebased it now

skullbox305 requested review from carlescufi, galak, MaureenHelm and nashif as code owners December 30, 2020 16:03

github-actions bot added area: API Changes to public APIs area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Documentation area: Samples Samples labels Dec 30, 2020

skullbox305 force-pushed the fdc2x1x branch 17 times, most recently from 360a949 to f5a66e0 Compare January 4, 2021 18:08

zephyrbot added area: Sensors Sensors area: Device Tree labels Jan 8, 2021

zephyrbot requested review from avisconti and mbolivar-nordic January 8, 2021 14:40

MaureenHelm requested changes Jan 22, 2021

View reviewed changes

pabigot mentioned this pull request Jan 23, 2021

sensors: BME280: obtain device pointers directly #30536

Merged

skullbox305 force-pushed the fdc2x1x branch 2 times, most recently from ff14a62 to 745b108 Compare January 24, 2021 01:46

skullbox305 requested a review from MaureenHelm January 24, 2021 01:51

skullbox305 force-pushed the fdc2x1x branch 3 times, most recently from 253690c to 60340fe Compare January 24, 2021 13:52

pabigot reviewed Jan 25, 2021

View reviewed changes

skullbox305 force-pushed the fdc2x1x branch from 60340fe to c0bc83e Compare January 26, 2021 18:33

skullbox305 force-pushed the fdc2x1x branch from c0bc83e to 5113b2b Compare February 9, 2021 18:29

mbolivar-nordic suggested changes Feb 23, 2021

View reviewed changes

skullbox305 force-pushed the fdc2x1x branch 3 times, most recently from 27324d2 to 3bbf94a Compare February 28, 2021 19:36

skullbox305 requested a review from mbolivar-nordic February 28, 2021 20:18

MaureenHelm approved these changes Mar 1, 2021

View reviewed changes

mbolivar-nordic approved these changes Mar 15, 2021

View reviewed changes

skullbox305 added 2 commits March 16, 2021 14:30

drivers: sensors: fdc2x1x: Add driver for Texas Instruments FDC2X1X

706c92b

Adds support for the Texas Instruments FDC2X1X Capacitance-to-Digital Converter for Proximity and Level Sensing Applications. Signed-off-by: Igor Knippenberg <igor.knippenberg@gmail.com>

skullbox305 force-pushed the fdc2x1x branch from 3bbf94a to f7d93e0 Compare March 16, 2021 13:32

carlescufi merged commit 066be0a into zephyrproject-rtos:master Mar 17, 2021

skullbox305 deleted the fdc2x1x branch March 31, 2021 22:04

teburd mentioned this pull request Nov 6, 2023

RFC: Sensor Channel Specifiers #63830

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sensor: add TI FDC2X1X sensor driver and sample application #31056

sensor: add TI FDC2X1X sensor driver and sample application #31056

skullbox305 commented Dec 30, 2020

skullbox305 commented Jan 22, 2021

MaureenHelm left a comment

MaureenHelm Jan 22, 2021

mbolivar-nordic commented Jan 23, 2021

skullbox305 commented Jan 24, 2021

pabigot left a comment

pabigot Jan 25, 2021

skullbox305 Jan 26, 2021 •

edited

Loading

henrikbrixandersen Jan 26, 2021

carlescufi Jan 26, 2021

skullbox305 Jan 26, 2021

mbolivar-nordic Feb 23, 2021

skullbox305 Feb 28, 2021

mbolivar-nordic left a comment

carlescufi commented Mar 16, 2021

skullbox305 commented Mar 16, 2021

sensor: add TI FDC2X1X sensor driver and sample application #31056

sensor: add TI FDC2X1X sensor driver and sample application #31056

Conversation

skullbox305 commented Dec 30, 2020

skullbox305 commented Jan 22, 2021

MaureenHelm left a comment

Choose a reason for hiding this comment

MaureenHelm Jan 22, 2021

Choose a reason for hiding this comment

mbolivar-nordic commented Jan 23, 2021

skullbox305 commented Jan 24, 2021

pabigot left a comment

Choose a reason for hiding this comment

pabigot Jan 25, 2021

Choose a reason for hiding this comment

skullbox305 Jan 26, 2021 • edited Loading

Choose a reason for hiding this comment

henrikbrixandersen Jan 26, 2021

Choose a reason for hiding this comment

carlescufi Jan 26, 2021

Choose a reason for hiding this comment

skullbox305 Jan 26, 2021

Choose a reason for hiding this comment

mbolivar-nordic Feb 23, 2021

Choose a reason for hiding this comment

skullbox305 Feb 28, 2021

Choose a reason for hiding this comment

mbolivar-nordic left a comment

Choose a reason for hiding this comment

carlescufi commented Mar 16, 2021

skullbox305 commented Mar 16, 2021

skullbox305 Jan 26, 2021 •

edited

Loading