Driver for TCS3772 Color Light-to-Digital Converter #3135

jfischer-no · 2015-05-31T23:10:42Z

This PR adds the driver for TCS3772 Color Light-to-Digital Converter with Proximity Sensing.
Driver supports basic functionality only, no Proximity Sensing.
Can be tested with pba-d-01-kw2x Board.

PeterKietzmann · 2015-06-01T07:32:37Z

drivers/include/tcs37727.h

+ * @return                  0 on success
+ * @return                  -1 on error
+ */
+int tcs37727_read(tcs37727_t *dev, uint16_t *rawred, uint16_t *rawgreen,


I would like to see this driver in a similar style as for the isl29125. This would mean to use the color_rgb_t as output type. If you think you also need a higher resolution you could also introduce an own type in addition (just compare the isl29125 implementation)

PeterKietzmann · 2015-06-01T07:55:47Z

drivers/tcs37727/tcs37727.c

+    *rawclear = ((uint16_t)buf[1] << 8) | buf[0];
+    *rawred = ((uint16_t)buf[3] << 8) | buf[2];
+    *rawgreen = ((uint16_t)buf[5] << 8) | buf[4];
+    *rawblue = ((uint16_t)buf[7] << 8) | buf[6];


See comment in header file

PeterKietzmann · 2015-06-01T08:10:05Z

Hm :-(. Any ideas why? <Do I have a different board revision or so?

2015-06-01 10:07:44,199 - INFO # kernel_init(): This is RIOT! (Version: 2014.05-3654-g3c63-test_johann_rgb)
2015-06-01 10:07:44,202 - INFO # kernel_init(): jumping into first task...
2015-06-01 10:07:44,207 - INFO # TCS37727 RGBC Data; Sensor driver test application
2015-06-01 10:07:44,207 - INFO # 
2015-06-01 10:07:44,211 - INFO # Initializing TCS37727 sensor at I2C_0... [Failed]

jfischer-no · 2015-06-01T08:17:32Z

You need PCB version 1426.1 (it is under the Phytec logo, left of micro USB).

In the picture here is 1426.0 version and the sensor is not on it.
https://github.com/RIOT-OS/RIOT/wiki/Board%3A-Phytec-phyWAVE-KW22

jfischer-no · 2015-06-01T08:18:24Z

I think you need more boards, right?

PeterKietzmann · 2015-06-01T09:15:32Z

I've got version 1426.0. Afaik the newer revision is available in the FU? Maybe some of them could test the driver.

jonas-rem · 2015-06-01T09:33:22Z

Just updated the photo on the wiki-page [1].
The wiki-page now shows the most recent version of the board with the TCS3772 light-sensor mounted. I just mention this to avoid confusion.

[1] - https://github.com/RIOT-OS/RIOT/wiki/Board:-Phytec-phyWAVE-KW22

OlegHahm · 2015-06-01T13:19:40Z

My board is currently somewhere between Paris and Berlin...

PeterKietzmann · 2015-06-01T13:20:34Z

Is it with you :-) ?

OlegHahm · 2015-06-01T13:24:00Z

Apparently I outran it. ;-)

jfischer-no · 2015-06-02T23:02:20Z

updated and rebased on master, suitable, but still WIP

PeterKietzmann · 2015-06-03T06:47:01Z

Ok. Just give me a sign when it's not WIP any more!

jfischer-no · 2015-06-12T10:44:39Z

@PeterKietzmann I think I'm almost done, I do not want imlementieren the proximity measurement now, IR diode was not mounted at our Boards.
The init function do not need any parameters, the sensor should be self adjusted by autgain function.
I am considering to divide the tcs37727_t in tcs37727_t and tcs37727_data_t, there also insert color_rgb_t. What do you mean?

PeterKietzmann · 2015-06-12T15:43:01Z

@jfischer-phytec-iot I will have a deeper look inside in the next week, ok? In general: Do as much as you like and think is needed. Perhaps I will adapt small things afterwards which you need to test then. <- that was my thought when you opened this PR and said that this driver should just me a minimal implementation

PeterKietzmann · 2015-06-15T06:59:00Z

drivers/include/tcs37727.h

+    int cc;                 /**< IR compensated channels clear */
+    int lux;                /**< Lux */
+    int ct;                 /**< Color temperature */
+} tcs37727_t;


I'm wondering if you need all of this parameters for the device descriptor.

@PeterKietzmann once again:
The init function do not need any parameters, the sensor should be self adjusted by autogain function.
I am considering to divide the tcs37727_t in tcs37727_t and tcs37727_data_t, there also insert color_rgb_t. What do you mean?

Ok, do it! Maybe I just need to see this...

PeterKietzmann · 2015-06-15T07:02:49Z

@jfischer-phytec-iot I'd suggest you adapt this PR with the discussed changes and shortly ping me then, so I can review and merge the minimal implementation.

jfischer-no · 2015-06-22T22:51:01Z

@PeterKietzmann ping

PeterKietzmann · 2015-06-23T19:32:46Z

@jfischer-phytec-iot there are some minor things that "may" be changed. I tend to merge this PR anyway, as it doesn't seem to have the highest priority ("basic functionality only"). I'll do a follow up PR to clean up some minor things, once I find more time.
@jfischer-phytec-iot please squash!
@OlegHahm, @haukepetersen can you quickly run the test application? If this succeeds I'll give my ACK.

PeterKietzmann · 2015-06-23T19:34:28Z

PS: Don't forget to use PCB version 1426.1 (can be found under the Phytec logo, left to the micro USB connector)

jfischer-no · 2015-06-23T21:41:26Z

@PeterKietzmann "basic functionality only" is not more up to date. We may like to discuss about "minor things" :-) I would try to make the autogain function independently of tcs37727_read.

PeterKietzmann · 2015-06-24T06:33:03Z

@jfischer-phytec-iot is it urgent? If so, just say it! Otherwise I'd like to delay this PR for some days..

jfischer-no · 2015-06-24T07:40:19Z

@PeterKietzmann It is not urgent. A little time to mature will do the driver well :-)

PeterKietzmann · 2015-07-06T08:48:13Z

drivers/tcs37727/tcs37727.c

+            case 1:
+                reg_again = TCS37727_CONTROL_AGAIN_4;
+                val_again = 4;
+                break;


I don't understand the relation to https://github.com/RIOT-OS/RIOT/pull/3135/files#diff-c13e75366b4489aac1fd522db3c5b4a5R82. There you set TCS37727_CONTROL_AGAIN_4 and dev->again = 4;. Here you read this value by val_again = dev->again; and when the switch case is 4 (like you initialized) you set the parameters TCS37727_CONTROL_AGAIN_16 and val_again = 16;

The function increments or decrements the gain value dev->again.

reg_again and val_again are temporary values.

dev->again is required for the calculations in tcs37727_read. reg_again will be a bitmap and should be written to register.

s/reg_again/tmp_reg_again and s/val_again/tmp_val_again ?

Ok, after comparing to the manual I got it.

PeterKietzmann · 2015-07-08T08:19:16Z

@jfischer-phytec-iot except the question about the clipping of the gain value, are you willing to commit something else? Otherwise I think you can already squash you commits.

jfischer-no · 2015-07-10T13:18:47Z

@PeterKietzmann Yes, I will do the "TODO" :-)

PeterKietzmann · 2015-08-25T15:52:32Z

Just for the protocol: I talked with @jfischer-phytec-iot and he currently has some other projects with higher priority. He will adapt this PR once he finds time again.

PeterKietzmann · 2015-09-25T10:05:47Z

@jfischer-phytec-iot how is it going on your side? Any plans to spend further work on this PR?

jfischer-no · 2015-10-20T11:38:08Z

@PeterKietzmann It works well and provides useful results. I have removed WIP and would like to merge it (I am planning to extend it but it takes a few more weeks).

PeterKietzmann · 2015-10-22T08:08:29Z

tests/driver_tcs37727/main.c

+
+#include <stdio.h>
+
+#include "vtimer.h"


Please change to xtimer

PeterKietzmann · 2015-10-22T08:10:35Z

@jfischer-phytec-iot fine with me. Could you please address my latest minor comments, check your code quickly, rebase and squash and give me a hint then?!

The driver supports lux and ct calculation and autogain funktion.

jfischer-no · 2015-10-22T22:08:00Z

@PeterKietzmann ping

PeterKietzmann · 2015-10-23T07:30:10Z

Fine with me. Let's wait for T

PeterKietzmann · 2015-10-23T08:05:29Z

ACK and go

Driver for TCS3772 Color Light-to-Digital Converter

jfischer-no added the Area: drivers Area: Device drivers label May 31, 2015

PeterKietzmann reviewed Jun 1, 2015
View reviewed changes

jfischer-no added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 1, 2015

PeterKietzmann reviewed Jun 1, 2015
View reviewed changes

miri64 assigned PeterKietzmann Jun 2, 2015

PeterKietzmann reviewed Jun 15, 2015
View reviewed changes

PeterKietzmann reviewed Jul 6, 2015
View reviewed changes

OlegHahm modified the milestones: Release 2015.09, Release NEXT MAJOR Sep 2, 2015

jfischer-no added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Oct 20, 2015

PeterKietzmann reviewed Oct 22, 2015
View reviewed changes

tests/driver_tcs37727/main.c

#include <stdio.h>

#include "vtimer.h"

Copy link

Member

PeterKietzmann Oct 22, 2015

Choose a reason for hiding this comment

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

Please change to xtimer

jfischer-no added 2 commits October 22, 2015 12:05

drivers/tcs37727: initial commit for tcs37727 driver

c6cec83

The driver supports lux and ct calculation and autogain funktion.

tests/driver_tcs37727: initial commit for tcs37727 driver test

a3e187e

jfischer-no added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Oct 22, 2015

PeterKietzmann added a commit that referenced this pull request Oct 23, 2015

Merge pull request #3135 from jfischer-phytec-iot/pr@tcs37727

835c2db

Driver for TCS3772 Color Light-to-Digital Converter

PeterKietzmann merged commit 835c2db into RIOT-OS:master Oct 23, 2015

jfischer-no deleted the pr@tcs37727 branch October 23, 2015 09:03

Driver for TCS3772 Color Light-to-Digital Converter #3135

Driver for TCS3772 Color Light-to-Digital Converter #3135

Conversation

jfischer-no commented May 31, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PeterKietzmann commented Jun 1, 2015

jfischer-no commented Jun 1, 2015

jfischer-no commented Jun 1, 2015

PeterKietzmann commented Jun 1, 2015

jonas-rem commented Jun 1, 2015

OlegHahm commented Jun 1, 2015

PeterKietzmann commented Jun 1, 2015

OlegHahm commented Jun 1, 2015

jfischer-no commented Jun 2, 2015

PeterKietzmann commented Jun 3, 2015

jfischer-no commented Jun 12, 2015

PeterKietzmann commented Jun 12, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PeterKietzmann commented Jun 15, 2015

jfischer-no commented Jun 22, 2015

PeterKietzmann commented Jun 23, 2015

PeterKietzmann commented Jun 23, 2015

jfischer-no commented Jun 23, 2015

PeterKietzmann commented Jun 24, 2015

jfischer-no commented Jun 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PeterKietzmann commented Jul 8, 2015

jfischer-no commented Jul 10, 2015

PeterKietzmann commented Aug 25, 2015

PeterKietzmann commented Sep 25, 2015

jfischer-no commented Oct 20, 2015

Choose a reason for hiding this comment

PeterKietzmann commented Oct 22, 2015

jfischer-no commented Oct 22, 2015

PeterKietzmann commented Oct 23, 2015

PeterKietzmann commented Oct 23, 2015