Macro: fix and redesign

[tmk]

At this moment macro uses add_weak_mods for all mdofiers. This is intented to prevent macro from breaking real/physical key state when the modifier in macro is released. But add_mods is needed for some macro usage, for example, when you want a macro which behaves as real modifier key.

Later I'll change macro action behaviour:
KEY_DOWN: uses register_code even for modifier
KEY_UP: uses register_code even for modifier

And add macro actions:
ADD_WEAK_MOD: uses weak_add_mods
DEL_WEAK_MOD: uses weak_del_mods
ADD_MOD:
DEL_MOD:
ADD_KEY:
DEL_KEY:
SEND: uses send_keyboard_report

tmk_keyboard/common/action_macro.c

Line 44 in a5d4a1f

add_weak_mods(MOD_BIT(macro));

[tmk]

use case #136

[tmk]

And this use case.
e852582#commitcomment-13926250

Users may want to change modifier state in their macro definition.

• store modifier state
• restore modifier state
• clear modifier state

[tmk]

This commit e852582 probably fixed #136.

[p3lim]

Implemented storing, restoring and clearing modifiers.
This does not take arguments though, which is probably wanted/needed.

From a1260b5fbc1813229738d81109af2c8dd68101c1 Mon Sep 17 00:00:00 2001
From: Adrian L Lange <mail@p3lim.net>
Date: Fri, 23 Oct 2015 20:38:50 +0200
Subject: [PATCH] Add support for storing, restoring and clearing modifiers in
 macros

---
 common/action_macro.c | 13 +++++++++++++
 common/action_macro.h | 12 ++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/common/action_macro.c b/common/action_macro.c
index ffaf125..34e22e5 100644
--- a/common/action_macro.c
+++ b/common/action_macro.c
@@ -34,6 +34,8 @@ void action_macro_play(const macro_t *macro_p)
     macro_t macro = END;
     uint8_t interval = 0;

+    uint8_t mod_storage;
+
     if (!macro_p) return;
     while (true) {
         switch (MACRO_READ()) {
@@ -66,6 +68,17 @@ void action_macro_play(const macro_t *macro_p)
                 interval = MACRO_READ();
                 dprintf("INTERVAL(%u)\n", interval);
                 break;
+            case MOD_STORE:
+                mod_storage = get_mods();
+                break;
+            case MOD_RESTORE:
+                set_mods(mod_storage);
+                send_keyboard_report();
+                break;
+            case MOD_CLEAR:
+                clear_mods();
+                send_keyboard_report();
+                break;
             case 0x04 ... 0x73:
                 dprintf("DOWN(%02X)\n", macro);
                 register_code(macro);
diff --git a/common/action_macro.h b/common/action_macro.h
index aedc32e..4cf2216 100644
--- a/common/action_macro.h
+++ b/common/action_macro.h
@@ -64,6 +64,9 @@ enum macro_command_id{
     /* 0x74 - 0x83 */
     WAIT                = 0x74,
     INTERVAL,
+    MOD_STORE,
+    MOD_RESTORE,
+    MOD_CLEAR,

     /* 0x84 - 0xf3 (reserved for keycode up) */

@@ -82,6 +85,9 @@ enum macro_command_id{
 #define TYPE(key)       DOWN(key), UP(key)
 #define WAIT(ms)        WAIT, (ms)
 #define INTERVAL(ms)    INTERVAL, (ms)
+#define STORE()         MOD_STORE
+#define RESTORE()       MOD_RESTORE
+#define CLEAR()         MOD_CLEAR

 /* key down */
 #define D(key)          DOWN(KC_##key)
@@ -93,6 +99,12 @@ enum macro_command_id{
 #define W(ms)           WAIT(ms)
 /* interval */
 #define I(ms)           INTERVAL(ms)
+/* store modifier(s) */
+#define SM()            STORE()
+/* restore modifier(s) */
+#define RM()            RESTORE()
+/* clear modifier(s) */
+#define CM()            CLEAR()

 /* for backward comaptibility */
 #define MD(key)         DOWN(KC_##key)
-- 
2.6.2.windows.1

[p3lim]

With that patch, I'm able to write my macro like this:

return (record->event.pressed ? MACRO(T(F20), CM(), SM(), T(SLSH), RM(), T(O), END) : MACRO_NONE);

Instead of how you suggested in e852582#commitcomment-13938301

[tmk]

Ah, nice. It is almost exactly what I had in mind.
I like to make STORE() atomic, which only stores modifier state.

diff --git a/tmk_core/common/action_macro.c b/tmk_core/common/action_macro.c
index ffaf125..34e22e5 100644
--- a/tmk_core/common/action_macro.c
+++ b/tmk_core/common/action_macro.c
@@ -34,6 +34,8 @@ void action_macro_play(const macro_t *macro_p)
     macro_t macro = END;
     uint8_t interval = 0;

+    uint8_t mod_storage;
+
     if (!macro_p) return;
     while (true) {
         switch (MACRO_READ()) {
@@ -66,6 +68,17 @@ void action_macro_play(const macro_t *macro_p)
                 interval = MACRO_READ();
                 dprintf("INTERVAL(%u)\n", interval);
                 break;
+            case MOD_STORE:
+                mod_storage = get_mods();
+                break;
+            case MOD_RESTORE:
+                set_mods(mod_storage);
+                send_keyboard_report();
+                break;
+            case MOD_CLEAR:
+                clear_mods();
+                send_keyboard_report();
+                break;
             case 0x04 ... 0x73:
                 dprintf("DOWN(%02X)\n", macro);
                 register_code(macro);

[p3lim]

So you'd have to store > clear > rest of macro > restore > end?

[tmk]

yep.

[p3lim]

I'd be fine with that.

Updated my patch in case you want to use it.