g910-gkey-macro-support icon indicating copy to clipboard operation
g910-gkey-macro-support copied to clipboard

Published 20 hours ago • verified publisher icon JSubelj

m key support and profile switching. Closes #14

Open MR-R080T opened this issue 5 years ago • 3 comments

Provides support for M keys and allows any G or M key to switch the active config file

MR-R080T avatar Dec 26 '19 05:12 MR-R080T

Hey,

great work! I would just like to comment on some things:

  • Switching the color of logo: I do understand that some sort of indication would be nice but not everyone wants to get their color changed depending on the config type. I would propose using notify-send command to notify the user about the change.

  • Involvement of g810-led program: g810-led is not a dependency and I removed it, so that users don't need to have it and can use some other led changing program. If the program is not installed on the system then errors are logged, but not having a g810-led installed is not an error. I would propose checking if g810-led exists on the system and checking in the config or someway if the user really wants this behavior.

  • Changing logo to breathing: Why not just turn on/off m1-3/mr lights to the corresponding config/keypress?

  • Naming the configs: I would propose some sort of config naming inside the config. Currently, if you open the config.json, you can't really tell which config is used, is it m1 or mr or which. Having a config_name property in json file would resolve that and also you could send a notify-send command with the message: "Configuration changed to $config_name".

  • Changing readme.md: You did great with documenting the feature, but merging it would mean I would have to change the source git and delete the first line. It's not a bother to do that but it would be easier for me to just merge the pull request without editing it :)

Again you did really great work and thank you for that. If you could fix the commented parts I would be glad to merge your pull request.

Would love to hear from you,

Jan

JSubelj avatar Dec 28 '19 17:12 JSubelj

Since I like this PR, I worked on it. I changed the notification system to switch mX led if g910-leds is available (instead of colorizing the logo).

Here is the code: https://github.com/bcourtine/g910-gkey-macro-support/tree/swap-config I can create a new PR if it is good for you.

PS: sorry for the code quality. I am not used to develop in Python.

bcourtine avatar Nov 24 '20 16:11 bcourtine

Since I like this PR, I worked on it. I changed the notification system to switch mX led if g910-leds is available (instead of colorizing the logo).

Here is the code: https://github.com/bcourtine/g910-gkey-macro-support/tree/swap-config I can create a new PR if it is good for you.

PS: sorry for the code quality. I am not used to develop in Python.

MR-R080T does not seam to be active on GitHub anymore, so I think you should open a new PullRequest so this has a chance of being merged at all.

ModProg avatar Apr 10 '21 22:04 ModProg

I kind of picked that one up in #60 and added profile switching with a single config.json. Give it a try

suabo avatar Nov 09 '22 05:11 suabo