driver: tmp006 rework by smlng · Pull Request #7168 · RIOT-OS/RIOT

smlng · 2017-06-09T21:08:47Z

rework general implementation of tmp006
adapt driver test accordingly
add support for saul

this concludes the effort of full SAUL support for board pba-d-01-kw2x see #7152

aabadie

I found (very) small inconsistencies and the problem with public header file.
I don't have the hardware for testing. Can you provide an output of the test application and another using saul ?
Otherwise, good job, it looks much better now.

aabadie · 2017-06-13T21:02:02Z

sys/auto_init/saul/auto_init_tmp006.c

+
+/**
+ * @brief   Reference the driver struct
+ * @{


why do you need to define this a block ? maybe just @brief is enough?

aabadie · 2017-06-13T21:05:38Z

drivers/tmp006/tmp006.c

 #define TMP006_DID_VALUE           0x0067 /**< Device ID */

-#define I2C_SPEED                  I2C_SPEED_FAST
+#define I2C_SPEED                   I2C_SPEED_FAST


align seems broken compared to other defines

aabadie · 2017-06-13T21:07:06Z

drivers/tmp006/include/tmp006_params.h

+#define TMP006_PARAM_ADDR          (TMP006_I2C_ADDRESS)
+#endif
+#ifndef TMP006_PARAM_RATE
+#define TMP006_PARAM_RATE           TMP006_CONFIG_CR_DEF


maybe use ( ) around TMP006_CONFIG_CR_DEF and align with others

aabadie · 2017-06-13T21:09:27Z

drivers/tmp006/include/tmp006.h

@@ -1,5 +1,6 @@
 /*
 * Copyright (C) 2014 PHYTEC Messtechnik GmbH
+ *               2017 HAW Hamburg


Why moving this file from drivers/include to drivers/tmp006/include ? I think drivers public API definition should remain in the former.

I like the idea of having most driver code within its subfolder without to much external/distributed dependencies - as drivers/Makefile.include already knows about the drivers include dir.

I see the point but it is inconsistent with the other driver implementations. I would prefer to keep it in drivers/include.

Yes, you're right - I'll move it back for the time being. But have a look at my proposal in #6923, to get the general idea I'm aiming for.

I remember this one and as I said, I think it's a good idea in general to allow the use of external drivers implementation. How will this work with pseudomodules ? Some generic drivers use them to specify a precise device, see bmx280 for instance.
Anyway, for the moment, I think we should stick to the current design and move the discussion in #6923

yes, (as said above) I agree with you, and already moved the header back. So we are good here?

smlng · 2017-06-14T08:25:27Z

comments addressed, need to squash

smlng · 2017-06-14T08:30:20Z

output from tests/driver_tmp006, with a slight temperature peak to 26°C when I touched the sensor:

2017-06-14 10:27:01,595 - INFO # Data Tabm: 2265   Tobj: 2661
2017-06-14 10:27:02,597 - INFO # Raw data T:  2900   V:   -15
2017-06-14 10:27:02,600 - INFO # Data Tabm: 2265   Tobj: 2647
2017-06-14 10:27:03,603 - INFO # Raw data T:  3000   V: -7430
2017-06-14 10:27:03,606 - INFO # Data Tabm: 2343   Tobj: 0
2017-06-14 10:27:04,609 - INFO # Raw data T:  3392   V:   615
2017-06-14 10:27:04,611 - INFO # Data Tabm: 2650   Tobj: 4335
2017-06-14 10:27:05,614 - INFO # Raw data T:  3196   V:  2497
2017-06-14 10:27:05,617 - INFO # Data Tabm: 2496   Tobj: 7380
2017-06-14 10:27:06,620 - INFO # Raw data T:  3160   V:   512
2017-06-14 10:27:06,623 - INFO # Data Tabm: 2468   Tobj: 3969

and output from examples/saul:

> 2017-06-14 10:29:37,659 - INFO #  main(): This is RIOT! (Version: 2017.04-devel-1283-g111e6-MacBook-Air.lan-driver/tmp006/rework)
2017-06-14 10:29:37,660 - INFO # Welcome to RIOT!
2017-06-14 10:29:37,661 - INFO # 
2017-06-14 10:29:37,665 - INFO # Type `help` for help, type `saul` to see all SAUL devices
2017-06-14 10:29:37,666 - INFO # 
> saul
2017-06-14 10:29:40,431 - INFO #  saul
2017-06-14 10:29:40,432 - INFO # ID	Class		Name
2017-06-14 10:29:40,434 - INFO # #0	ACT_SWITCH	LED(red)
2017-06-14 10:29:40,437 - INFO # #1	ACT_SWITCH	LED(green)
2017-06-14 10:29:40,439 - INFO # #2	ACT_SWITCH	LED(blue)
2017-06-14 10:29:40,441 - INFO # #3	SENSE_BTN	Button(SW0)
2017-06-14 10:29:40,443 - INFO # #4	SENSE_BTN	Button(CS0)
2017-06-14 10:29:40,445 - INFO # #5	SENSE_MAG	mag3110
2017-06-14 10:29:40,447 - INFO # #6	SENSE_ACCEL	mma8652
2017-06-14 10:29:40,449 - INFO # #7	SENSE_TEMP	hdc1000
2017-06-14 10:29:40,450 - INFO # #8	SENSE_HUM	hdc1000
2017-06-14 10:29:40,453 - INFO # #9	SENSE_TEMP	tmp006
2017-06-14 10:29:40,455 - INFO # #10	SENSE_COLOR	tcs37727
> saul read 9
2017-06-14 10:29:45,678 - INFO #  saul read 9
2017-06-14 10:29:45,682 - INFO # Reading from #9 (tmp006|SENSE_TEMP)
2017-06-14 10:29:45,684 - INFO # Data:	[0] 23.68°C
2017-06-14 10:29:45,686 - INFO # 	[1] 25.08°C
> saul read 9
2017-06-14 10:29:49,850 - INFO #  saul read 9
2017-06-14 10:29:49,854 - INFO # Reading from #9 (tmp006|SENSE_TEMP)
2017-06-14 10:29:49,856 - INFO # Data:	[0] 23.68°C
2017-06-14 10:29:49,858 - INFO # 	[1] 24.85°C
> saul read 9
2017-06-14 10:29:56,542 - INFO #  saul read 9
2017-06-14 10:29:56,546 - INFO # Reading from #9 (tmp006|SENSE_TEMP)
2017-06-14 10:29:56,548 - INFO # Data:	[0] 24.84°C
2017-06-14 10:29:56,549 - INFO # 	[1] -12.62°C
> saul read 9
2017-06-14 10:30:01,014 - INFO #  saul read 9
2017-06-14 10:30:01,018 - INFO # Reading from #9 (tmp006|SENSE_TEMP)
2017-06-14 10:30:01,020 - INFO # Data:	[0] 25.71°C
2017-06-14 10:30:01,021 - INFO # 	[1] 11.97°C
> saul read 9
2017-06-14 10:30:03,903 - INFO #  saul read 9
2017-06-14 10:30:03,907 - INFO # Reading from #9 (tmp006|SENSE_TEMP)
2017-06-14 10:30:03,908 - INFO # Data:	[0] 25.93°C
2017-06-14 10:30:03,909 - INFO # 	[1] 17.24°C

Both tests run on board pba-d-01-kw2x (PhyNode).

smlng · 2017-06-14T09:21:32Z

squashed

aabadie · 2017-06-14T09:44:02Z

drivers/tmp006/tmp006.c

    *tobj = (t - 273.15);
 }
+
+int tmp006_read_temperature( tmp006_t *dev, int16_t *ta, int16_t *to)


extra space before tmp006_t

good catch, fixed and squashed directly

aabadie

ACK

aabadie · 2017-06-14T21:10:42Z

forgot this one: go!

smlng self-assigned this Jun 9, 2017

smlng requested review from PeterKietzmann and haukepetersen June 9, 2017 21:08

smlng added this to the Release 2017.07 milestone Jun 9, 2017

smlng mentioned this pull request Jun 9, 2017

SAUL: add full support for PhyNode (pba-d-01-kw2x) #7152

Closed

smlng force-pushed the driver/tmp006/rework branch 2 times, most recently from 3b4f654 to 160b33d Compare June 9, 2017 21:32

smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 9, 2017

smlng force-pushed the driver/tmp006/rework branch from 160b33d to e3be16c Compare June 9, 2017 21:53

smlng requested a review from aabadie June 13, 2017 14:25

aabadie requested changes Jun 13, 2017

View reviewed changes

smlng force-pushed the driver/tmp006/rework branch from e3be16c to 111e684 Compare June 14, 2017 08:20

smlng force-pushed the driver/tmp006/rework branch from 111e684 to 7bd5461 Compare June 14, 2017 09:21

aabadie reviewed Jun 14, 2017

View reviewed changes

smlng force-pushed the driver/tmp006/rework branch from 7bd5461 to 8312216 Compare June 14, 2017 09:46

aabadie approved these changes Jun 14, 2017

View reviewed changes

smlng added 3 commits June 14, 2017 20:44

drivers: rework tmp006

42a0912

tests: adapt driver_tmp006 to rework

c17d64e

saul: add support for tmp006

054c002

smlng force-pushed the driver/tmp006/rework branch from 8312216 to 054c002 Compare June 14, 2017 18:45

aabadie merged commit 92147cf into RIOT-OS:master Jun 14, 2017

smlng deleted the driver/tmp006/rework branch June 15, 2017 07:07

Conversation

smlng commented Jun 9, 2017

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Jun 14, 2017

Uh oh!

smlng commented Jun 14, 2017

Uh oh!

smlng commented Jun 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jun 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants