Magic+D should not enable all debug flags

[DidierLoiseau]

Following PR #217 by @IBNobody, it appears that pressing Magic+D enables all debug flags:

        case MAGIC_KC(MAGIC_KEY_DEBUG):
            debug_enable = !debug_enable;
            if (debug_enable) {
                print("\ndebug: on\n");
                debug_matrix   = true;
                debug_keyboard = true;
                debug_mouse    = true;
            } else {
                print("\ndebug: off\n");
                debug_matrix   = false;
                debug_keyboard = false;
                debug_mouse    = false;
            }
            break;

This was not previously the case, and is likely a mistake as you probably don't want this. Indeed, all the debug of dprint*() and debug*() only rely on the debug_enable flag. The 3 other ones are intended for specific trace logging and produce a huge amount of output, even when you don't press any key.

Additionally, when all flags are enabled together, the keyboard becomes very slow and almost unresponsive (at least the ErgoDox EZ), and it is even difficult to disable them (you have to maintain the shortcut for several seconds).

Could we thus remove the enabling of these 3 flags when debug_enable is set to true?

Workaround: enable and then disable keyboard debug with Magic+K twice, which leaves debug_enable to true.

[IBNobody]

That's fine, but why would it affect the EZ more?

On Fri, Sep 23, 2016, 4:08 PM DidierLoiseau notifications@github.com wrote:

Following PR #217 https://github.com/jackhumbert/qmk_firmware/pull/217 by @IBNobody https://github.com/IBNobody, it appears that pressing Magic +D enables all debug flags:

    case MAGIC_KC(MAGIC_KEY_DEBUG):
        debug_enable = !debug_enable;
        if (debug_enable) {
            print("\ndebug: on\n");
            debug_matrix   = true;
            debug_keyboard = true;
            debug_mouse    = true;
        } else {
            print("\ndebug: off\n");
            debug_matrix   = false;
            debug_keyboard = false;
            debug_mouse    = false;
        }
        break;

This was not previously the case, and is likely a mistake as you probably don't want this. Indeed, all the debug of dprint() and debug() only rely on the debug_enable flag. The 3 other ones are intended for specific trace logging and produce a huge amount of output, even when you don't press any key.

Additionally, when all flags are enabled together, the keyboard becomes very slow and almost unresponsive (at least the ErgoDox EZ), and it is even difficult to disable them (you have to maintain the shortcut for several seconds).

Could we thus remove the enabling of these 3 flags when debug_enable is set to true?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jackhumbert/qmk_firmware/issues/779, or mute the thread https://github.com/notifications/unsubscribe-auth/AFl50Jk6RBqzT8vZLzc9ek2wLpzDv1SEks5qtD_AgaJpZM4KFZKd .

[DidierLoiseau]

@IBNobody It's just what I have and I don't know whether this slowness is due to the limitations of HID or the CPU, so it might depend on the keyboard.

[fredizzimo]

I agree, the different debug outputs should be toggle able like they were before.

I'm pretty sure, but I haven't verified that the slowness comes from the fact that the hid protocol can't send more than 64 bytes every 1 ms. So once it oversteps that it might first start to buffer, and then eventually have to wait until more data can be sent.

Another reason why the EZ might be affected more, is this pull request https://github.com/jackhumbert/qmk_firmware/pull/343, which causes the keyboard loop to run faster than it should. It also causes the debouncing algorithm to not work as intended. It was partially reverted in https://github.com/jackhumbert/qmk_firmware/pull/474, with some further fixes here https://github.com/jackhumbert/qmk_firmware/commit/3577e26fd9916ceab58779ec6323d43da54eb3b5 based on the comments I wrote here https://github.com/jackhumbert/qmk_firmware/commit/8e88d55bfd7c88cb15845e0c6415e4e892532861.

The same changes were not made for the EZ, as can be seen in https://github.com/jackhumbert/qmk_firmware/blob/master/keyboards/ergodox/ez/matrix.c, specially the comments _"this should be waitms(1) but has been left as-is at EZ's request". I think there was some further discussion somewhere, but I can't seem to find it, maybe @ezuk or @jackhumbert remember?

Anyway the TL;DR version is the following, the EZ currently runs the scan loop faster than other keyboards.

[DidierLoiseau]

@jackhumbert it looks like your commit 78767bf is not what I was suggesting in this issue: I was suggesting that enabling debug should not enable all debug flags. What you changed is that now disabling debug does no more disable all debug flags. I think the disable part could be kept as-is, it is the enable part that should be changed.