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

drivers: sensor: lm73c: Added TI LM73C sensor driver #31064

Closed
wants to merge 1 commit into from
Closed

drivers: sensor: lm73c: Added TI LM73C sensor driver #31064

wants to merge 1 commit into from

Conversation

riedlse
Copy link

@riedlse riedlse commented Dec 31, 2020

Added LM73C sensor driver and sample code. Built and
tested on frdm_kw41Z and nrf52840 dongle. Ran twister.

On branch sensor_lm73c
Changes to be committed:
modified: drivers/sensor/CMakeLists.txt
modified: drivers/sensor/Kconfig
new file: drivers/sensor/lm73c/CMakeLists.txt
new file: drivers/sensor/lm73c/Kconfig
new file: drivers/sensor/lm73c/doc/LM73CIMK-0.lbr
new file: drivers/sensor/lm73c/doc/lm73.pdf
new file: drivers/sensor/lm73c/lm73c.c
new file: drivers/sensor/lm73c/lm73c.h
new file: samples/sensor/lm73c/CMakeLists.txt
new file: samples/sensor/lm73c/README.rst
new file: samples/sensor/lm73c/boards/frdm_kw41z.overlay
new file: samples/sensor/lm73c/boards/nrf52840dk_nrf52840.overlay
new file: samples/sensor/lm73c/boards/nrf52840dongle_nrf52840.overlay
new file: samples/sensor/lm73c/prj.conf
new file: samples/sensor/lm73c/sample.yaml
new file: samples/sensor/lm73c/src/main.c

Signed-off-by: Steven Riedl steve@serconsultingllc.com

Added LM73C sensor driver and sample code. Built and
tested on frdm_kw41Z and nrf52840 dongle. Ran twister.

On branch sensor_lm73c

Signed-off-by: Steven Riedl <steve@serconsultingllc.com>

drivers: sensor: lm73c: Added TI LM73C sensor driver

(minor format fixes

A few minor format issues made it past, fixed

On branch sensor_lm73c
Changes to be committed:
	modified:   drivers/sensor/lm73c/Kconfig
	modified:   drivers/sensor/lm73c/lm73c.c
	modified:   samples/sensor/lm73c/README.rst
	modified:   samples/sensor/lm73c/src/main.c

Signed-off-by: Steven Riedl <steve@serconsultingllc.com>

drivers: sensor: lm73c: Added TI LM73C sensor driver

(minor format fixes
A few minor format issues made it past, fixed

On branch sensor_lm73c
Changes to be committed:
	modified:   drivers/sensor/lm73c/lm73c.c
	modified:   samples/sensor/lm73c/src/main.c

Signed-off-by: Steven Riedl <steve@serconsultingllc.com>

drivers: sensor: lm73c: Added TI LM73C sensor driver

(minor format fixes
A few minor format issues made it past, fixed
Really fixed this time, need to look in to other editors...

On branch sensor_lm73c
Changes to be committed:
	modified:   drivers/sensor/lm73c/lm73c.c
	modified:   samples/sensor/lm73c/src/main.c

Signed-off-by: Steven Riedl <steve@serconsultingllc.com>

drivers: sensor: lm73c: Added TI LM73C sensor driver

Commit format fix

On branch sensor_lm73c
Changes to be committed:
	modified:   drivers/sensor/lm73c/lm73c.c

Signed-off-by: Steven Riedl <steve@serconsultingllc.com>
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Please also edit the commit message so it isn't a squash of a bunch of original commits.

drivers/sensor/lm73c/Kconfig Show resolved Hide resolved
drivers/sensor/lm73c/doc/LM73CIMK-0.lbr Show resolved Hide resolved
drivers/sensor/lm73c/lm73c.c Show resolved Hide resolved
drivers/sensor/lm73c/lm73c.c Show resolved Hide resolved
drivers/sensor/lm73c/lm73c.c Show resolved Hide resolved
samples/sensor/lm73c/README.rst Show resolved Hide resolved
Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

I agree with @pabigot 's comments, plus a few more below

samples/sensor/lm73c/CMakeLists.txt Show resolved Hide resolved
- "ttemp: (.*)"
fixture: fixture_i2c_lm73c
depends_on: i2c
filter: dt_compat_enabled("ti,lm73c")
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will pick up the boards you added overlays for. Please replace this with a platform_allow list

Copy link
Collaborator

@pabigot pabigot Jan 6, 2021

Choose a reason for hiding this comment

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

By experimentation this is true; only certain boards are normally tested. But I believe we're supposed to be avoiding platform_allow lists.

If you make it frdm_k64f instead of (or in addition to) frdm_kw41z then at least one will be tested.

I don't know where the list of tested reference boards is hidden.

Copy link
Author

Choose a reason for hiding this comment

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

Should I just delete the sample.yaml then? use the one from the 9808

sample:
name: MCP9808 Temperature Sensor
tests:
sample.sensor.mcp9808:
harness: sensor
tags: sensors
depends_on: i2c
filter: dt_compat_enabled("microchip,mcp9808")

I was looking at bme280 and mcp9808, thought I was taking the best of both worlds

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, the sample.yaml needs to be there. It just needs to have at least one board for which there's a devicetree node, so that this command does something:

tirzah[882]$ twister -T samples/sensor/lm73c/
Renaming output directory to /mnt/nordic/zp/zephyr/twister-out.12
INFO    - Zephyr version: zephyr-v2.4.0-2711-g8f057fcdea
INFO    - JOBS: 32
INFO    - Selecting default platforms per test case
INFO    - Building initial testcase list...
INFO    - 1 test suites (24 configurations) selected, 23 configurations discarded due to filters.
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:    1/   1  100%  skipped:   24, failed:    0
INFO    - 0 of 0 test configurations passed (0.00%), 0 failed, 24 skipped with 0 warnings in 2.03 seconds
INFO    - In total 0 test cases were executed, 24 skipped on 24 out of total 318 platforms (7.55%)
INFO    - 0 test configurations executed on platforms, 0 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing xunit report /mnt/nordic/zp/zephyr/twister-out/twister.xml...
INFO    - Writing xunit report /mnt/nordic/zp/zephyr/twister-out/twister_report.xml...
INFO    - Writing JSON report /mnt/nordic/zp/zephyr/twister-out/twister.json
INFO    - Run completed

@riedlse
Copy link
Author

riedlse commented Jan 9, 2021

Mistakenly created a new branch that has all of these fixes and more, and wound up creating a pull request there, so I ma closing this one.

@riedlse riedlse closed this Jan 9, 2021
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.

None yet

4 participants