search

bradyaturner / lights

A Ruby library & CLI for interacting with Philips Hue
MIT License
37 stars 7 forks source link

Added method for sending DELETE request #2

Closed bobbyduhbrain closed 10 years ago

bradyaturner commented 10 years ago

Hey Mike I made a few comments in the code. Other than that, I'd like to do a code review just to compare it with the original equations. One other thing is that normally I would want to see two separate pull requests, one for the delete function and one for colorconverter. Don't worry about splitting it this time. Also, you would really want to be doing this on a branch of your own code, not master. See this: https://www.atlassian.com/git/workflows#!workflow-feature-branch. So in this case you would have a branch for delete, and a branch for colorconverter, then a pull request for each branch.

Once you update the code, just push it to your master branch on github and shoot me a comment. The pull request isn't tied to a specific commit, so it will be updated when you push.

bradyaturner commented 10 years ago

Where did you get this equation? I found this one: http://www.everyhue.com/vanilla/discussion/comment/635, which I tried out in my Qt project, but it produces xy values for green (0,255,0), which are outside of the allowable range ([0,1]). I'm going to try your equation out and see how it compares.

bradyaturner commented 10 years ago

I tried a comparison of the two equations. For the one I found, I rounded xy values up to 0 or down to 1 if they were out of the range, as a workaround for green producing out of range values. I just compared the pure R, G, and B values set with both equations. I also compared purple (255,0,255), since its my favorite color on the bulbs.

The green produced by both equations is indistinguishable. Your equation produces a slightly brighter blue color than mine does. I'm not sure which one I prefer. The red produced by your equation is slightly brighter, and more of an orange. I definitely prefer the one produced by my equation. The two equations produce very different purples. Mine is closer to a hot pink, and yours is a lighter, pastel color. Honestly I'm pretty disappointed with both equations on this one, the Phillips app gets a very nice purple color.

I wanted to take some pictures of this to show you, but the camera isn't picking up the differences in color very well. Not sure what to do about this, but I'd like your input.

bradyaturner commented 10 years ago

We also want to check that rgb values are in [0,255].