Skip to content

Add color property to the TCS34725 class (closes #8). #13

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

Merged
merged 2 commits into from
Oct 4, 2018
Merged

Add color property to the TCS34725 class (closes #8). #13

merged 2 commits into from
Oct 4, 2018

Conversation

process1183
Copy link
Contributor

I built the module and docs locally using the instructions in the README, as well as running pylint (10.00/10). I then tested my changes with an Adafruit Feather M0 Express and the Adafruit TCS34725 sensor board using a modified copy of the example tcs34725_simpletest.py. Sample output from this script:

Detected color: #19190C
sensor.color = 1644812 #19190C
Temperature: 4508.97K Lux: 721.692

Detected color: #440705
sensor.color = 4458245 #440705
Temperature: 2961.51K Lux: 1572.29

Detected color: #101010
sensor.color = 1052688 #101010
Temperature: 8890.37K Lux: 267.161

@caternuson caternuson requested a review from a team October 4, 2018 04:19
Blue = 255 (0x0000ff), SlateGray = 7372944 (0x708090)
"""
r, g, b = self.color_rgb_bytes
return (r << 16) + (g << 8) + b
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for using + instead of | here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep- I didn't think of it :)

Since I'm already doing bitwise stuff, I guess it would be more consistent if I used | instead of +. Shall I go ahead and change it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please.

@caternuson
Copy link
Contributor

Thanks for doing this. Looks good. Just have the question above in the review.

For consistency, switch from '+' to '|' since bitwise operations are already being used.
#13 (comment)
@caternuson caternuson merged commit f82f166 into adafruit:master Oct 4, 2018
@process1183 process1183 deleted the colorproperty branch October 5, 2018 00:01
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 5, 2018
Updating https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15 to 0.5.2 from 0.5.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_ADS1x15#16 from iBug/patch-1
  > ignore the board module imports in .pylintrc
  > Merge pull request adafruit/Adafruit_CircuitPython_ADS1x15#14 from adafruit/pypi_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_TCS34725 to 3.1.1 from 3.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_TCS34725#13 from process1183/colorproperty
  > Merge pull request adafruit/Adafruit_CircuitPython_TCS34725#12 from VladMihai28/issue11
  > ignore the board module imports in .pylintrc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants