Support Modifiers + Logging Cleanup

[lmiphay]

Hi,

This pull request replaces the previous pull request:

Allow a single G13 keypress be mapped to multiple key events at uinput

This request has all of the changes in that previous request, plus a logging cleanup.

Thanks,

Paul

[james-fowler]

Apologies, I'm a bit new to GitHub and I didn't notice this before I ended up duplicating some of the effort on my own. I haven't done nearly as much with logging yet, but I have support for multiple keys and quite a bit more (multiple profiles, writing text to the LCD, ... ). Haven't submitted a pull request of my own yet because I'm still working on the code.

[lmiphay]

No problem - will be interested to take a look at what you have when you are finished.

In the meantime I have done a little more work around the race condition between sending bind commands and uploading a new bitmap image (the parser gets confused if they come too quickly together).

Paul

[james-fowler]

That race condition fix sounds useful. I should probably start thinking about putting in a pull request for my fork, as I've cleaned up, renamed, and reorganized quite a bit, so it might get tedious to merge anything else that gets done before then. My support for modifiers goes a bit further than yours, but I'd like to try and incorporate the logging cleanup you've done. Feel free to take a look - I might never be "finished", but it's currently working fine. I've got some scripts tying it together with some hacks I've done in Synergy, which automatically change backlight color and keymappings when I move the mouse from one system to another.

[TechOtter]

I was just wondering about modifiers. Any news on if the PR will be merged?

[james-fowler]

I think most, if not all, of what you had was functionally duplicated in my branch, which has now been merged to ecraven/g13 master. Again, my apologies for not paying attention - it would have been much better to start with your branch.

Multiple key support / modifers are definitely in there. The latest version should be 100% compatible with your multiple key bind command format. The KEY_ prefix is now optional, but still accepted. There area also a few other new tricks - all in the readme. I don't know whether the newer code is still vulnerable to the race condition - if you've got a repeatable test and can check it out, that would be great.

Logging had been updated, similar to what you did but with a few twists. It uses Boost logging, although indirectly through the G13_OUT and G13_LOG macros. There is also a command line option to adjust the logging level, and a pipe command to adjust it after startup.

The fix for 91-g13.rules is not included yet. It would be good to get that and the changes from #3 merged in. I can do the merge, but I'm not familiar enough with all the linux permissions and security bits to be confident I can test it properly.

Is there any other functionality in your branch that hasn't been added to the latest ecraven/g13 master?

[lmiphay]

Slightly crossed signals here:

TechOtter> I was just wondering about modifiers. Any news on if the PR will be merged?

See the response from James which describes the current level of support for handling modifiers and as far as I know covers all functionality I catered for in the original PR. If you see an anything missing, let us know here and I am sure we can add/fix it.

james-fowler> ...it would have been much better to start with your branch.

As I mentioned before - no problems on my side, and I am just happy to see this project move forward.

I'll get around to updating shortly and create a small PR for the udev rules file.

The race condition in the original tree was pretty easy to repo (haven't checked HEAD to see if its still there) - just send a image file in over the pipe and immediately send some bind key commands (unless the read from the pipe exactly matched the size expected it wouldn't be recognised as a bitmap image file).

And just to round things off I currently have another issue on startup when on boot, it seems the uinput isn't ready when the g13 daemon tries to open it:

Problem writing uinput event: result=-1, error=Bad file descriptor

Again this is the original tree plus my patches - again haven't had a chance to check with your changes. It may be gone, which would be great.

Paul

[TechOtter]

Thanks for the info. Apparently my browser had cached an old version of the readme so I did see the new features that have been added.

[james-fowler]

lmiphay> I'll get around to updating shortly and create a small PR for the udev rules file.
Might be good to look at merging in #3 while we're at it.

lmiphay> The race condition...
Moved this to support request #19

lmiphay> And just to round things off I currently have another issue on startup when on boot, it seems the uinput isn't ready ....
Moved this to support request #20

lmiphay> As I mentioned before - no problems on my side, and I am just happy to see this project move forward.
So are you OK with closing this pull request?

[lmiphay]

Yes fine by me - closed.