Skip to content

Commit

Permalink
Fix qmk#156: clear weak mods on every key press
Browse files Browse the repository at this point in the history
- new macro_mods bit field for mods applied by macros
- weak_mods now only used for ACT_{L,R}MODS (i.e. LSFT, RSFT, LCTL etc.)
- clear the _weak_ mods on every key *pressed* such that LSFT etc.
  can no more interfere with the next key
  • Loading branch information
DidierLoiseau committed Mar 8, 2016
1 parent 7d3ebd7 commit b7a81f0
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 2 deletions.
5 changes: 5 additions & 0 deletions tmk_core/common/action.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ void process_action(keyrecord_t *record)
#endif
dprintln();

if (event.pressed) {
// clear the potential weak mods left by previously pressed keys
clear_weak_mods();
}
switch (action.kind.id) {
/* Key and Mods */
case ACT_LMODS:
Expand Down Expand Up @@ -500,6 +504,7 @@ void clear_keyboard(void)
void clear_keyboard_but_mods(void)
{
clear_weak_mods();
clear_macro_mods();
clear_keys();
send_keyboard_report();
#ifdef MOUSEKEY_ENABLE
Expand Down
4 changes: 2 additions & 2 deletions tmk_core/common/action_macro.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void action_macro_play(const macro_t *macro_p)
MACRO_READ();
dprintf("KEY_DOWN(%02X)\n", macro);
if (IS_MOD(macro)) {
add_weak_mods(MOD_BIT(macro));
add_macro_mods(MOD_BIT(macro));
send_keyboard_report();
} else {
register_code(macro);
Expand All @@ -51,7 +51,7 @@ void action_macro_play(const macro_t *macro_p)
MACRO_READ();
dprintf("KEY_UP(%02X)\n", macro);
if (IS_MOD(macro)) {
del_weak_mods(MOD_BIT(macro));
del_macro_mods(MOD_BIT(macro));
send_keyboard_report();
} else {
unregister_code(macro);
Expand Down
9 changes: 9 additions & 0 deletions tmk_core/common/action_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ static inline void del_key_bit(uint8_t code);

static uint8_t real_mods = 0;
static uint8_t weak_mods = 0;
static uint8_t macro_mods = 0;

#ifdef USB_6KRO_ENABLE
#define RO_ADD(a, b) ((a + b) % KEYBOARD_REPORT_KEYS)
Expand All @@ -55,6 +56,7 @@ static int16_t oneshot_time = 0;
void send_keyboard_report(void) {
keyboard_report->mods = real_mods;
keyboard_report->mods |= weak_mods;
keyboard_report->mods |= macro_mods;
#ifndef NO_ACTION_ONESHOT
if (oneshot_mods) {
#if (defined(ONESHOT_TIMEOUT) && (ONESHOT_TIMEOUT > 0))
Expand Down Expand Up @@ -118,6 +120,13 @@ void del_weak_mods(uint8_t mods) { weak_mods &= ~mods; }
void set_weak_mods(uint8_t mods) { weak_mods = mods; }
void clear_weak_mods(void) { weak_mods = 0; }

/* macro modifier */
uint8_t get_macro_mods(void) { return macro_mods; }
void add_macro_mods(uint8_t mods) { macro_mods |= mods; }
void del_macro_mods(uint8_t mods) { macro_mods &= ~mods; }
void set_macro_mods(uint8_t mods) { macro_mods = mods; }
void clear_macro_mods(void) { macro_mods = 0; }

/* Oneshot modifier */
#ifndef NO_ACTION_ONESHOT
void set_oneshot_mods(uint8_t mods)
Expand Down
7 changes: 7 additions & 0 deletions tmk_core/common/action_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ void del_weak_mods(uint8_t mods);
void set_weak_mods(uint8_t mods);
void clear_weak_mods(void);

/* macro modifier */
uint8_t get_macro_mods(void);
void add_macro_mods(uint8_t mods);
void del_macro_mods(uint8_t mods);
void set_macro_mods(uint8_t mods);
void clear_macro_mods(void);

/* oneshot modifier */
void set_oneshot_mods(uint8_t mods);
void clear_oneshot_mods(void);
Expand Down

9 comments on commit b7a81f0

@eltang
Copy link

@eltang eltang commented on b7a81f0 Mar 9, 2016

Choose a reason for hiding this comment

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

I actually have a much more complete solution for qmk#156 that I am going to reveal after Jack finishes shipping out all the Massdrop orders. It makes it possible for layouts like Programmer Dvorak to be implemented seamlessly.

@DidierLoiseau
Copy link
Owner Author

Choose a reason for hiding this comment

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

@Eric-L-T Couldn't you already share it already? I guess you didn't test it yet but that would be more convenient for discussing it ;-)

I suppose this also relates to qmk#148, which discusses Programmer Dvorak.

@eltang
Copy link

@eltang eltang commented on b7a81f0 Mar 9, 2016

Choose a reason for hiding this comment

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

I've actually done a lot of testing and my findings have allowed me to make my solution much better than it originally was. It's kind of hard to explain. Do you mind waiting?

@eltang
Copy link

@eltang eltang commented on b7a81f0 Mar 9, 2016

Choose a reason for hiding this comment

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

@DidierLoiseau Jack wrote me that he doesn't have enough time to look at what I've done at the moment because he's busy fulfilling the last of the Massdrop orders.

@DidierLoiseau
Copy link
Owner Author

Choose a reason for hiding this comment

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

@Eric-L-T we should continue the discussion on PR qmk#188, Jack has already posted a comment there. It will become confusing otherwise :-)

I wonder why he asked that though, there are lots of people contributing ATM, and even if he does not have the time now I think it's always useful to create PR's as soon as they are ready and have a visible backlog.

@eltang
Copy link

@eltang eltang commented on b7a81f0 Mar 10, 2016

Choose a reason for hiding this comment

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

What's your rationale for creating macro_mods? I read through qmk#156 and I didn't quite understand.

@eltang
Copy link

@eltang eltang commented on b7a81f0 Apr 2, 2016

Choose a reason for hiding this comment

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

I saw that this solution is causing some issues such as qmk#221. Do you want me to send you my solution in a pull request? It sorta depends on qmk#182, though.

@DidierLoiseau
Copy link
Owner Author

Choose a reason for hiding this comment

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

@eltang I think you should definitely make your PR against @jackhumbert's repo, you will get more feedback there. There are already many ongoing PR now so I don't see why yours should wait. Even if Jack does not have the time to review it now it won't hurt to have it already.

Anyway I'd be glad to have a look at your changes even if you don't create a PR yet. Is it in your modifier-release-fix branch?

@eltang
Copy link

@eltang eltang commented on b7a81f0 Apr 6, 2016

Choose a reason for hiding this comment

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

The code's not on GitHub yet. I'll try to upload it in a bit.

Please sign in to comment.