Skip to content
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

Jump to bottom

Add KATAR_PRO_XT #739

Merged
merged 5 commits into from Oct 1, 2021
Merged

Add KATAR_PRO_XT #739

merged 5 commits into from Oct 1, 2021

Conversation

michaelni
Copy link

@michaelni michaelni commented Aug 18, 2021 •
edited

A limitation ATM is that the wheel LED can be adjusted in brightness
but not color in SW mode. For HW mode the locations for the DPI colors
has been found.

As my goal with writing this was just to be able to switch the LED off
that solves what i need.

Signed-off-by: Michael Niedermayer michael@niedermayer.cc

@michaelni michaelni force-pushed the katar_pro_xt branch 2 times, most recently from 6cd27c9 to c56369f Compare August 22, 2021 08:03
@michaelni michaelni changed the title [WIP] Add KATAR_PRO_XT Add KATAR_PRO_XT Aug 22, 2021
tatokis
tatokis requested changes Aug 29, 2021
View changes
Copy link
Member

@tatokis tatokis left a comment

Choose a reason for hiding this comment

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

Unfortunately, as it stands, this can not be merged in its current state.
To get RGB working, count the number of separate LEDs on the device (including each DPI LED), and add that number in

ckb-next/src/daemon/led_mouse_bragi.c

Lines 24 to 26 in 761ba48

switch(kb->product){
LED_CASE(P_IRONCLAW_W_U, 6);
LED_CASE(P_HARPOON_WL_U, 2);

You will need to rebase and force push in your fork.

Then, once you get the number of LEDs there, look at the mouse section in the keymap and try the scancodes one by one from the top until you figure out and note down which is which.

ckb-next/src/daemon/keymap.c

Lines 272 to 284 in 761ba48

// RGB mouse zones
{ "front", LED_MOUSE, KEY_NONE },
{ "back", LED_MOUSE + 1, KEY_NONE },
{ "dpi", LED_MOUSE + 2, KEY_NONE }, // SW DPI light
{ "wheel", LED_MOUSE + 3, KEY_NONE },
{ "thumb", LED_MOUSE + 4, KEY_NONE },
{ "side", LED_MOUSE + 5, KEY_NONE },
{ "dpi0", LED_MOUSE + 6, KEY_NONE }, // HW DPI light levels (0 = sniper, 1..5 = modes)
{ "dpi1", LED_MOUSE + 7, KEY_NONE },
{ "dpi2", LED_MOUSE + 8, KEY_NONE },
{ "dpi3", LED_MOUSE + 9, KEY_NONE },
{ "dpi4", LED_MOUSE + 10, KEY_NONE },
{ "dpi5", LED_MOUSE + 11, KEY_NONE },

Once you do, create a keymap patch like these ones, so that the scancodes correspond to the correct LEDs. Make sure to NULL out the remaining entries so that there are no duplicate scancodes.

ckb-next/src/daemon/keymap_patch.c

Lines 30 to 56 in 761ba48

keypatch icwpatch[] = {
{ 237, "back", LED_MOUSE, KEY_NONE },
{ 238, "wheel", LED_MOUSE + 1, KEY_NONE },
{ 239, "front", LED_MOUSE + 2, KEY_NONE },
{ 240, "dpi0", LED_MOUSE + 3, KEY_NONE },
{ 241, "dpi1", LED_MOUSE + 4, KEY_NONE },
{ 242, "dpi2", LED_MOUSE + 5, KEY_NONE },
{ 243, NULL, 0, KEY_NONE },
{ 244, NULL, 0, KEY_NONE },
{ 245, NULL, 0, KEY_NONE },
{ 246, NULL, 0, KEY_NONE },
};
#define ICWPATCH_LEN sizeof(icwpatch)/sizeof(*icwpatch)
keypatch harpoonwlpatch[] = {
{ 237, "dpi", LED_MOUSE, KEY_NONE },
{ 238, "back", LED_MOUSE + 1, KEY_NONE },
{ 239, NULL, 0, KEY_NONE },
{ 240, NULL, 0, KEY_NONE },
{ 241, NULL, 0, KEY_NONE },
{ 242, NULL, 0, KEY_NONE },
{ 243, NULL, 0, KEY_NONE },
{ 244, NULL, 0, KEY_NONE },
{ 245, NULL, 0, KEY_NONE },
{ 246, NULL, 0, KEY_NONE },
};
#define HARPOONWLPATCH_LEN sizeof(harpoonwlpatch)/sizeof(*harpoonwlpatch)

If the colours change but (for example) red doesn't actually control the red channel, and instead controls another one, you got the number of LEDs wrong in the function above.

This is all I can think of at the moment. Thank you for looking into this!

src/daemon/led_mouse_bragi.c Outdated
@@ -24,6 +24,20 @@ int updatergb_mouse_bragi(usbdevice* kb, int force){

uchar pkt[BRAGI_JUMBO_SIZE] = {0};

if (kb->vendor == V_CORSAIR && kb->product == P_KATAR_PRO_XT) {
Copy link
Member

Choose a reason for hiding this comment

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

ckb-next does not currently handle hardware brightness; It is implemented in software.
Hardware brightness needs to be set to 100%.

I'd suggest changing this so that it sets it to full brightness on all bragi devices.

Copy link
Author

Choose a reason for hiding this comment

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

ok, ive set it to 1000 fixed, ive added a commit on top of that, which you may want to ommit but being able to switch the hw brightness off is useful

src/daemon/led_mouse_bragi.c Outdated Show resolved Hide resolved
src/daemon/keymap.c Outdated
@@ -609,6 +609,7 @@ void process_input_urb(void* context, unsigned char* buffer, int urblen, ushort
} else {
kb->input.rel_x += (buffer[5] << 8) | buffer[4];
kb->input.rel_y += (buffer[7] << 8) | buffer[6];
kb->input.whl_rel_y = (signed char)buffer[8];
Copy link
Member

Choose a reason for hiding this comment

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

I think this might cause issues with other bragi devices. I need to look into it.

Copy link
Member

Choose a reason for hiding this comment

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

So, having finally looked at how my mouse behaves, in software mode, for every scroll event, it spits out two packets.

[I] ckb1 Input Recv: 00 02 00 00 ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
[I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff

The bottom one is the one that this change receives the scroll information from, which is a hardware mode packet.
The scroll information should be read from the software mode packet, which is the first one in this case.
Does your mouse not emit that packet? Does it differ in some way from mine?

Copy link
Author

Choose a reason for hiding this comment

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

My katar pro xt with my patchset on top of the master branch from where it was based on does not generate the 1st packet for the scroll wheel, it does generate these for the buttons though. I had ckb-next running so i presume it was in software mode

Sep 29 21:27:47 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 29 21:27:47 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 29 21:27:48 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 29 21:27:48 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 29 21:27:48 : [I] ckb1 Input Recv: 00 00 00 00 01 00 00 00 00
Sep 29 21:27:49 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 29 21:27:49 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 29 21:27:50 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 29 21:27:50 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 29 21:27:51 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 29 21:27:51 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 29 21:27:51 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 29 21:27:52 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 29 21:27:56 : [I] ckb1 Input Recv: 00 02 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:27:56 : [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:27:57 : [I] ckb1 Input Recv: 00 02 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:27:57 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 00
Sep 29 21:27:57 : [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:27:57 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 00
Sep 29 21:27:57 : [I] ckb1 Input Recv: 00 00 00 00 01 00 00 00 00
Sep 29 21:27:58 : [I] ckb1 Input Recv: 00 02 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:27:58 : [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:27:59 : [I] ckb1 Input Recv: 00 02 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:27:59 : [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:28:01 : [I] ckb1 Input Recv: 00 02 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:28:01 : [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 29 21:28:03 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 29 21:28:04 : [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 29 21:28:08 : [I] ckb1 Input Recv: 00 00 00 00 01 00 00 00 00

in fact i tried zgrep 'ckb1 Input Recv' /var/log/syslog* | grep -v ': .. .. .. .. .. .. .. .. ..$' | grep -v repeated | grep ff and this has 0 matches.
Of course i know very little about the codebase and the protocol so its quite possible iam missing something

Copy link
Member

Choose a reason for hiding this comment

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

This seems frustrating. Do you also not get both packets when you press a button?
For example, when I left click in software mode, I get:

[I] ckb1 Input Recv: 00 02 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
[I] ckb1 Input Recv: 01 00 00 00 00 00 00 00 00

but in hardware mode I get only:

[I] ckb1 Input Recv: 01 00 00 00 00 00 00 00 00

I had ckb-next running so i presume it was in software mode

As long as the GUI is running and controlling the lights, it's in software mode. Quitting the GUI using either the tray icon or the button under settings should switch the device back to hardware mode.

Copy link
Author

Choose a reason for hiding this comment

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

It seems it doesnt follow the expected behavior.
I also rebased the code locally on top of current master to make sure iam not missing some bugfix

wheel and button test
Sep 30 22:51:16  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 01 00 00 00 00
Sep 30 22:51:17  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:20  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:51:20  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:21  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 30 22:51:22  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:51:22  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:22  ckb-next-daemon[2842705]: message repeated 3 times: [ [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00]
Sep 30 22:51:22  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 ff ff 00 00 00
Sep 30 22:51:22  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:23  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 30 22:51:23  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 00
Sep 30 22:51:23  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:23  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:51:24  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:24  ckb-next-daemon[2842705]: message repeated 2 times: [ [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00]
Sep 30 22:51:24  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 30 22:51:25  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:51:25  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:25  ckb-next-daemon[2842705]: message repeated 8 times: [ [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00]
Sep 30 22:51:25  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 ff ff 00 00 00
Sep 30 22:51:25  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00
Sep 30 22:51:25  ckb-next-daemon[2842705]: message repeated 4 times: [ [I] ckb1 Input Recv: 00 00 00 00 00 00 01 00 00]
Sep 30 22:51:25  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 30 22:51:26  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 01 00 00 00 00 00 00 00 00
Sep 30 22:51:27  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 00
Sep 30 22:51:27  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 01 00 00 00 00 00 00 00 00
Sep 30 22:51:27  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 00
Sep 30 22:51:28  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 02 00 00 00 00 00 00 00 00

ckb-next is started, GUI is vissible from here on
wheel and button test is repeated

Sep 30 22:52:18  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 ff ff 00
Sep 30 22:52:48  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:52:49  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 30 22:52:49  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:52:50  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 30 22:52:51  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:52:51  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 ff
Sep 30 22:52:52  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 00 00 00 00 00 00 00 01
Sep 30 22:52:52  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:52:53  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:52:53  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:52:53  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:52:55  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:52:55  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:52:56  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:52:56  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Sep 30 22:53:04  ckb-next-daemon[2842705]: [I] ckb1 Input Recv: 00 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Copy link
Member

Choose a reason for hiding this comment

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

Please try running it outside of systemd.
You should be able to sudo systemctl stop ckb-next-daemon and manually sudo ckb-next-daemon.

I don't think this is the case, but it might be rate limiting the logs somehow and maybe we're missing some of them, but it seems unlikely. Just want to be sure.

Copy link
Author

@michaelni michaelni Oct 1, 2021 •
edited

Choose a reason for hiding this comment

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

Tried without systemd, same behavior

Copy link
Member

Choose a reason for hiding this comment

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

Understood. I guess for now we can ignore the scroll data on the other packet and see how it goes.

@michaelni michaelni requested a review from tatokis August 29, 2021 15:03
@tatokis
Copy link
Member

tatokis commented Sep 30, 2021

While merging some of the functions, it created conflicts with this PR. (led_mouse_bragi.c and led_keyboard_bragi.c have been merged into led_bragi.c.)
I rebased it against the latest master here https://github.com/ckb-next/ckb-next/tree/katar_pro_xt.

While doing so, I noticed that the brightness packets are sent on every single RGB frame update. This doesn't seem correct.

I think the initial brightness should be set during device init, and then if we detect that the lights are off, then set it to 0 in the updatergb function.
Finally, after that, when the lights stop being off, we set it back to 1000.
This should be achievable by comparing lastlight and newlight, after the "don't do anything if the lighting hasn't changed" check.

ckb-next/src/daemon/led_bragi.c

Lines 53 to 63 in 7766ea5

uchar pkt[BRAGI_JUMBO_SIZE] = {0};
int brightness = 1000;
// Since the blank pkt is used to check if the lights are off, we need to make sure it's sufficiently large
static_assert(LED_MOUSE <= sizeof(pkt), "pkt is not large enough to check if all zones are off");
// Switch LEDs off if its all black, because being able to just switch them off even in hw mode is really nice
if (!(memcmp(newlight->r + led_offset, pkt, CPY_SZ(r)) ||
memcmp(newlight->g + led_offset, pkt, CPY_SZ(g)) ||
memcmp(newlight->b + led_offset, pkt, CPY_SZ(b))))
brightness = 0;
bragi_set_property(kb, BRAGI_BRIGHTNESS, brightness);

@michaelni
daemon/bragi_common: Use ushort in bragi_set_property()
7cc428b 
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
@michaelni
bragi_proto: Add information about additional fields
4e61295 
These where found by writing into them and looking at what happens on a
KATAR PRO XT

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
@michaelni
Add KATAR_PRO_XT
f3f1eea 
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
@michaelni
daemon/led_bragi: Set or Clear brightness depending on LED changes
2fbc65d 
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
@michaelni
Copy link
Author

michaelni commented Sep 30, 2021 •
edited

While merging some of the functions, it created conflicts with this PR. (led_mouse_bragi.c and led_keyboard_bragi.c have been merged into led_bragi.c.) I rebased it against the latest master here https://github.com/ckb-next/ckb-next/tree/katar_pro_xt.

While doing so, I noticed that the brightness packets are sent on every single RGB frame update. This doesn't seem correct.

I think the initial brightness should be set during device init, and then if we detect that the lights are off, then set it to 0 in the updatergb function. Finally, after that, when the lights stop being off, we set it back to 1000. This should be achievable by comparing lastlight and newlight, after the "don't do anything if the lighting hasn't changed" check.

ckb-next/src/daemon/led_bragi.c

Lines 53 to 63 in 7766ea5

uchar pkt[BRAGI_JUMBO_SIZE] = {0};
int brightness = 1000;
// Since the blank pkt is used to check if the lights are off, we need to make sure it's sufficiently large
static_assert(LED_MOUSE <= sizeof(pkt), "pkt is not large enough to check if all zones are off");
// Switch LEDs off if its all black, because being able to just switch them off even in hw mode is really nice
if (!(memcmp(newlight->r + led_offset, pkt, CPY_SZ(r)) ||
memcmp(newlight->g + led_offset, pkt, CPY_SZ(g)) ||
memcmp(newlight->b + led_offset, pkt, CPY_SZ(b))))
brightness = 0;
bragi_set_property(kb, BRAGI_BRIGHTNESS, brightness);

Should be fixed, the new code should only set the brightness when force is on or the value differs from the last

ckb-next/src/daemon/led_bragi.c

Lines 54 to 66 in 2fbc65d

// Since the blank pkt is used to check if the lights are off, we need to make sure it's sufficiently large
static_assert(LED_MOUSE <= sizeof(pkt), "pkt is not large enough to check if all zones are off");
// Switch LEDs off if its all black, because being able to just switch them off even in hw mode is really nice
int newon = memcmp( newlight->r + led_offset, pkt, CPY_SZ(r)) ||
memcmp( newlight->g + led_offset, pkt, CPY_SZ(g)) ||
memcmp( newlight->b + led_offset, pkt, CPY_SZ(b));
int laston = memcmp(lastlight->r + led_offset, pkt, CPY_SZ(r)) ||
memcmp(lastlight->g + led_offset, pkt, CPY_SZ(g)) ||
memcmp(lastlight->b + led_offset, pkt, CPY_SZ(b));
if (newon != laston || force)
bragi_set_property(kb, BRAGI_BRIGHTNESS, newon ? 1000 : 0);

tatokis
tatokis approved these changes Oct 1, 2021
View changes
@tatokis tatokis merged commit 783e3eb into ckb-next:master Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tatokis tatokis tatokis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@michaelni @tatokis