Skip to content
This repository was archived by the owner on Apr 21, 2025. It is now read-only.

Conversation

@frickler24
Copy link
Collaborator

This is the follower PR to #25: Handling delays between key actions while playing a macro.
Commits 77bf3bd .. 0da9e55 and 8296895 .. b675b32 belong to the daemon. They implement the handling of different delays on each change in keys (press and release). The implementation was done by stephenhouser for the ckb repo, I just cherry-picked them.
7751703 .. a51a4e6 implement the GUI (Bind tab) and 793fe12 fixes a debug error.

stephenhouser and others added 30 commits June 17, 2017 12:52
While recording a macro, between each two key changes
the delay is recorded.
The format is +/-KEY=macrotime_in_usec.
While recording a macro, between each two key changes
the delay is recorded.
The format is +/-KEY=macrotime_in_usec.

The correct form ist adjusted when clicking "Stop".
Some corrections and comments for the first steps in
the new GUI element for Keystroke-Delays.
This implementation adds some UI Elements to the Binding tab.

When recording a macro, the timing (the time between changes the
Keyboard receives) are noted with the key values.

When saved with the detailled timing information, you may choose between
- running the macro witghout any delay
- running it with the standard delay
    - 0ms for the first 20 changes (normally 10 Keys),
    - then 30ms for up to 100 keys (200 status-changes)
    - and then 200ms for any further change)
- running the key delays, which had been recorded.

After recording (or after saving) you may change the timing values.
This might be useful, if you want to generate slow keystrokes, but you
do not want unneccessary long wait times (e.g. when sending longer macro
content to a virtual machine or a remote desktop tool, the maximum speed
is often too high. So include a short delay and all is fine).
There are some contraints in using the TAB area for configuration.
because of too many elements neccessary for the macro GUI,
The font size had been set to 12px fix.

If you find that is to small, you can change it by installing an
own stylesheet file and calling ckb with
ckb -stylesheet=<stylesheet-file>

The complete TAB area (better the surrounding areas) should be
redesigned...
When using long macros with very long delays
(eg delays recorded while you typed very slowly)
and there is parallel a color-update necessary,
you can get an error in usb.c
(device closed as the keyboard resets itself).

Reason for this is misuse of the usb protocol
(sending two commands/request to the KB instead of
alternating request/reply)
because of missing synchronization between the two threads.

So we need a sync mechanism between these threads.
The mechanism is an additional mutex (mmutex).

The disadvantage of that synchronization is that
updating the color information is delayed
until all macros have been processed
(this also applies to type-ahead with several macros).
In the case of rather short macros or those with small delays,
this is not noticeable; in the case of wave motions,
the effect is occasionally recognizable.

And yes, we should rewrite ckb-daemon...
As a user mentioned, the colorization of the keyboard was stopped
while a macro with delays is played.

This was because the synchronisation block in input.c was over the whole
play-loop for the macro.
Now we have an unlock / lock sequence around all usleep calling for delay.

This has only minimal impact on the timeing behavior.
Test was a long running macro inclusive startings
"time cat" at the beginning and sending CTRL-D at the end.
Colorization was "shimmer" all over the keyboard,
so heavy load between GUI and daemon.

With this long macro before this commit, time-cmd gives:
real    0m40.087s   (between 40.080 and 40.087 in different runs)
user    0m0.000s
sys     0m0.000s

after changing the blocking for this critical section we get:
real    0m40.116s
user    0m0.000s
sys     0m0.000s
The first implementation for macros with delay management
has stopped the colorization while playing the macro content.

This Version handles the macro playing different:
It starts a single thread for each macro when
the appropriate key is pressed.
All tasks are self-syncing via a linked list (FIFO).

One Todo is left: When playing macros, all other keyboard input
except modifyer keys is ignored. A \todo is set as comment
where to heal this if anyone finds out...

With the new coding some comments for existing code
and changes for better readability are made and typos
have been fixed.
The shellscript migmac.sh was initially used to migrate
existing macros in the config files from 4- to 5-element strings.

Because the algorithm now handles older and newer macro strings as well,
the script is no longer needed.
@frickler24 frickler24 merged commit 0f6242e into mattanger:testing Jun 17, 2017
@frickler24 frickler24 deleted the macrodelay branch June 17, 2017 12:50
@ghost
Copy link

ghost commented Jun 18, 2017

Thank you for cleaning this out from other commits and cross-merges, much appreciated.

pthread_mutex_unlock(mmutex(kb)); ///< use this unlock / relock for enablling the parallel running colorization
if (action->delay != UINT_MAX) { ///< local delay set
if (action->delay != UINT_MAX && action->delay) { ///< local delay set
usleep(action->delay);
Copy link

Choose a reason for hiding this comment

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

usleep() is deprecated since POSIX.1-2001. We should use clock_nanosleep() instead.

static ptlist_t* pt_tail = 0;

///
/// \brief macro_pt_enqueue Save the new thread in the single linked list (FIFO).
Copy link

Choose a reason for hiding this comment

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

This is probably okay, but if we would use the queues somewhere else, I guess it'd be better to use something like BSD's queue for a generic solution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had looked for a solution like this, but not long enough (I just found other self implementations). So thank you for the link.
Because Tatokis asked me to merge the implementation before 20th june, I will change it as a PR after this date.

int res;
if(kb->fwversion >= 0x120 && !is_recv){
if (kb->fwversion >= 0x120 && !is_recv) {
struct usbdevfs_bulktransfer transfer;
Copy link

Choose a reason for hiding this comment

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

There's a neat way to initialize structs:

struct usbdevfs_bulktransfer transfer = {0};

instead of memset().

The C Standard guarantees this will initialize all members to their default values (e.g. integers to 0, pointers to NULL). Also, using just empty braces = {}; is also possible, although it's a GNU extension. But since it is already used in device.c (triple dots to set array boundaries), it should cause no problem, I suppose clang (even Apple's one) implements them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, a fine function, I didn't know this struct initializer.
I changed it, but I believe it may give conflicts with other branches from @tatokis.

const char* ep_str = udev_device_get_sysattr_value(dev, "bNumInterfaces");
#ifdef DEBUG
ckb_info("Connecting %s at %s%d\n", kb->name, devpath, index);
ckb_info("claiming interfaces. name=%s, serial=%s, firmware=%s; Got >>%s<< as ep_str\n", name, serial, firmware, ep_str);
Copy link

Choose a reason for hiding this comment

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

Not sure whether we need to print a serial code. Since we are concerned about security and even sign the FIRMWARE, I guess it shouldn't be printed in the logs that people publish here afterwards not realizing they are publishing it as it might be misused.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a very good comment, I cleared the serial code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants