Skip to content

Channel Selection, Bugfix #97

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

Closed
wants to merge 3 commits into from
Closed

Channel Selection, Bugfix #97

wants to merge 3 commits into from

Conversation

Bl4d3s
Copy link
Contributor

@Bl4d3s Bl4d3s commented Feb 24, 2017

channels are now selecteable. Bugfix, which caused flickering when in singe color mode, Bugfix which caused slow fading between colors.

Syntax:
hueplus=COM-Port,channel,leds
ex.: hueplus=COM4,0,8

channel:
0 -> both channels
1 -> channel 1
2-> channel 2
when 1/2 selected: the other channel will display the last color/stays out/gets no updates

leds:
fans * 8, always the number of one channel (the one with more fans)

For LED-Strip support: I dont know exactly if it is working as I dont own these. I can implement it if somebody sends me a dump session with a serial monitor (eg.: http://www.eltima.com/products/serial-port-monitor/ (free trial)). Just send in CAM some static colors and document which and how many LEDs you have connected. If you can vary colors and number of LEDs and send me the different dumps it would be easier, but one would be better then nothing :)

Copy link
Contributor

@d-rez d-rez left a comment

Choose a reason for hiding this comment

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

I have a few comments if I may

LPSTR next = NULL;

source = strtok_s(ledstring, ",", &next);
strcpy(led_string, ledstring);
Copy link
Contributor

Choose a reason for hiding this comment

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

TABs instead of spaces

strcpy(led_string, ledstring);

LPSTR source = NULL;
LPSTR channels = NULL;
Copy link
Contributor

@d-rez d-rez Feb 25, 2017

Choose a reason for hiding this comment

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

What's up with this alignment? :) You tried to align the first two and then gave up with the rest

@@ -275,16 +293,16 @@ void LEDStrip::SetLEDsHuePlus(COLORREF pixels[64][256])
{
unsigned char *serial_buf;

serial_buf = new unsigned char[250]; //Size of Message always 5 XX Blocks (Mode Selection) + 3 XX for each LED (1 color)
serial_buf = new unsigned char[125]; //Size of Message always 5 XX Blocks (Mode Selection) + 3 XX for each LED (1 color)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you're repeating 125 (previously 250) multiple times, maybe it's worth to create a const for that?

@@ -182,6 +182,7 @@
<ClCompile Include="..\KeyboardVisualizerCommon\LogitechSDK.cpp" />
<ClCompile Include="..\KeyboardVisualizerCommon\MSIKeyboard.cpp" />
<ClCompile Include="..\KeyboardVisualizerCommon\net_port.cpp" />
<ClCompile Include="..\KeyboardVisualizerCommon\PoseidonZRGBKeyboard.cpp" />
Copy link
Contributor

@d-rez d-rez Feb 25, 2017

Choose a reason for hiding this comment

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

I don't think this should be merged to master yet, along with the other Poseidon include

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just thought could be nice cause the project can't be build unless you manually add the poseidonz files to the project.

Regarding the other things. I will update my code :)

@@ -281,12 +281,13 @@ boolean parse_command_line(char * command_line)
printf(" - (ex.ledstrip=udp:192.168.1.5,1234,30)\r\n");
printf(" xmas - COM port, ex. xmas=COM2\r\n");
printf(" hueplus - HUE+ config:\r\n");
printf(" - hueplus=port,num_leds\r\n");
printf(" - hueplus=port,channel,num_leds\r\n");
printf(" - channel: 0 -> both channels, 1 -> channel 1, 2 -> channel 2\r\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

TABs instead of spaces

@CalcProgrammer1
Copy link
Owner

I think you have it backwards, I use spaces not tabs, with a tab width of 4 spaces. Just a tip, when you work on a project not your own, try to adapt to the project's tab/space style in your editor before you start working. I've contributed to other projects and they're often strict about this. I don't mind cleaning up pulls before committing them though, so if you just ignore the alignment and submit a PR I'll clean the tab/space issues. I am used to the 4 spaces for tab from work, and because space lines up the same on any editor/website. I'm going to merge your commits now.

As for the Poseidon headers, I wrote that code in Linux and never got around to adding to the Windows project, though it doesn't work in Windows yet but it doesn't hurt anything. I added them in the 3.04 release commit so that's not a big deal either way here.

@Bl4d3s
Copy link
Contributor Author

Bl4d3s commented Feb 25, 2017

Okay, thank you. I'll do it in the future.

@CalcProgrammer1
Copy link
Owner

UGH freaking intel...my overclocked xeon failed me with a nice BSOD as I made the commit and it seems to have trashed my git folder...gotta re-clone and re-merge. RYZEN PLS HURRY.

CalcProgrammer1 pushed a commit that referenced this pull request Feb 25, 2017
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.

3 participants