zestyping / openpixelcontrol

A simple stream protocol for controlling arrays of RGB lights.
http://openpixelcontrol.org/
353 stars 103 forks source link

Added gamma correction to tcl server. #14

Open devries opened 11 years ago

devries commented 11 years ago

I decided to try to address the gamma correction issue with TCL pixels the way I had in the elinux-tcl library. I basically create a lookup table for each color (right now that's not really used, but it could potentially come in handy) and I create the gamma correction table before the server starts.

I'm working on some other arduino based LED projects, so I don't have a test harness set up for this at the moment, but I hope it's not buggy.

zestyping commented 11 years ago

Hi Chris, thanks for writing this. Could you make the gamma values configurable on the command line? I think the default should be 1, so that existing lighting projects aren't affected by this change.

Also, some style nits: one space after commas and semicolons, one space before and after low-precedence binary operators (+, -, and anything of lower precedence). Thanks!

devries commented 11 years ago

OK, makes sense. I will work on the changes in my repo and submit them to you, but probably not before burning man.

hunson-abadeer commented 11 years ago

I tried this out with the latest build of OPC. It compiled fine on my mac, but threw errors on the BeagleBoneBlack. This is my first post on GitHub, so do let me know if there is a better/formal process for posting things of this nature.

gcc -o bin/tcl_server src/tcl_server.c src/opc_server.c src/spi.c src/tcl_server.c: In function 'set_gamma': src/tcl_server.c:60:36: warning: incompatible implicit declaration of built-in function 'pow' [enabled by default] /tmp/ccFHNAgO.o: In function set_gamma': tcl_server.c:(.text+0x2c0): undefined reference topow' tcl_server.c:(.text+0x364): undefined reference to pow' tcl_server.c:(.text+0x404): undefined reference topow' collect2: error: ld returned 1 exit status make: *\ [bin/tcl_server] Error 1

... update ...

I had to add the -lm flag to gcc on the BBB. Huh, wonder why it isn't necessary in OSX.

devries commented 11 years ago

Re: the compiling errors, I need to include the math.h library and add -lm to link to it in the gcc line.

I saw that you typedefed u8 to unsigned 8-bit int. I used it on several of the values I added, but I notice I didn't modify it on the casting calls.

I'll set up a test bed after burning man, but right now I don't have a system I can debug on, so I'll defer the changes.

zestyping commented 11 years ago

No rush! Thanks!

On Thu, Aug 15, 2013 at 6:56 AM, Christopher De Vries < notifications@github.com> wrote:

Re: the compiling errors, I need to include the math.h library and add -lm to link to it in the gcc line.

I saw that you typedefed u8 to unsigned 8-bit int. I used it on several of the values I added, but I notice I didn't modify it on the casting calls.

I'll set up a test bed after burning man, but right now I don't have a system I can debug on, so I'll defer the changes.

— Reply to this email directly or view it on GitHubhttps://github.com/zestyping/openpixelcontrol/pull/14#issuecomment-22704203 .

zestyping commented 11 years ago

Thanks @solidgold! I merged another pull request that added the missing -lm flag.

mbustosorg commented 8 years ago

hi all, this is exactly what I was looking to implement. Is there anything I can do to help it along?

zestyping commented 7 years ago

@devries @mbustosorg I made this configurable using the environment variable TCL_GAMMA. The merged change is in the branch devries-master https://github.com/zestyping/openpixelcontrol/tree/devries-master . Would either of you be able to test that branch on a Beaglebone? Once it's confirmed that it works, I can merge it.

devries commented 7 years ago

Unfortunately I don't have a working Beaglebone at the moment to do any testing. I had a similar routine in another piece of code I wrote, and it had an issue writing a 257th byte to a 256 byte long buffer. I think this one was simpler so it might not include that bug.

mbustosorg commented 7 years ago

I have one available that I can test with this weekend.

mbustosorg commented 7 years ago

Hey sorry for the delay. The gamma correction works. Please add #include <math.h> to remove the compiler issue. Please also check availability of the env TCL_GAMMA before trying to do the atof.

Let me know if you need anything else.