UltimateHackingKeyboard / firmware

Ultimate Hacking Keyboard firmware
Other
419 stars 66 forks source link

Optimize switch release debouncing #283

Closed mondalaci closed 4 years ago

mondalaci commented 4 years ago

We've received several reports about chatter, and we used to combat it in a brute force fashion by increasing the debouncing interval quite significantly. In the meantime, it turned out that chatter can have many causes, and the value we used was unreasonably high. I've just changed the debouncing interval to 5 ms in 13e7c43f59655c01f0ad47f75f47aa320f4e2455 which should be enough for MX switches.

I'm wondering though whether the switches should be debounced not only upon pressing but upon release, too. Some research should be done about this, and then we could set release debouncing interval to 0 if it's viable.

kareltucek commented 4 years ago

I was thinking about debouncing algorithm too during last two months... I soldered some of my switches badly, which led to chatter due to connection losses during some phases of the press, and later (unsuccessfully) tried to solve it by making the algorithm more robust.

Some ideas:

mondalaci commented 4 years ago

Sharp insights, and I don't have all the answers. I see no reason not to use immediate activation / deactivation, though. Doing otherwise would unnecessarily introduce delay upon press / release which doesn't make sense in general, and especially hurtful for competitive gaming.

How did you measure your bounces?

kareltucek commented 4 years ago

I see no reason not to use immediate activation / deactivation, though. Doing otherwise would unnecessarily introduce delay upon press / release which doesn't make sense in general.

This was meant with respect to releases only.

The intuition is that slight movement of the switch during pressed phase might introduce brief skips even during "pressed" phase, especially in case of corroded or otherwise damaged switches. However, after taking one switch apart and looking at the mechanism, I agree that the switches are designed to prevent this type of problems.

So yes, I agree with you. Should probably be considered in case some weird evidence shows up, but that's all.

How did you measure your bounces?

As explained above. If anything is unclear, please ask more concretely :-).

uhk-fw-8.7.1-debounce_v1.tar.gz

static inline void preprocessKeyState(key_state_t *keyState) {
    keyState->previous = keyState->current;

    static uint8_t bounces = 0;
    static bool bounceDebug = false;
    static uint32_t bounceTimestamp = 0;
    static bool lastSkipped = false;
    static key_state_t* debugKey = NULL;

    uint8_t debounceTime = keyState->previous ? DebounceTimePress : DebounceTimeRelease;
    if (keyState->debouncing && (uint8_t)(CurrentTime - keyState->timestamp) > debounceTime) {
        keyState->debouncing = false;
        if(debugKey == keyState) {
            bounceDebug = false;
            debugKey = NULL;
        }
    }

    if (keyState->debouncing && keyState->current != keyState->next) {
        bounces++;
        ShowNumber(keyState->previous ? 1 : 2); //forgotten, ignore it.
        if(bounceDebug == false) {
            bounceDebug = true;
            debugKey = keyState;
        }
        if(debugKey == keyState) {
            keyState->timestamp = CurrentTime;
            lastSkipped = true;
        }
    }

    if(bounceDebug && lastSkipped && debugKey == keyState) {
        uint32_t time = CurrentTime - bounceTimestamp ;
        LedDisplay_SetIcon(LedDisplayIcon_Adaptive, keyState->current);
        ShowNumber(time); 
        keyState->timestamp = CurrentTime;
        lastSkipped = false;
    }

    if (!keyState->debouncing && keyState->current != keyState->next) {
        //LedDisplay_UpdateText();
        bounceTimestamp = CurrentTime;
        keyState->timestamp = CurrentTime;
        keyState->debouncing = true;
        keyState->current = keyState->next;
    }
}
kareltucek commented 4 years ago

(Adaptive mode led indicates that shown number relates to "press", otherwise it is either false positive (in case of times above 25ms) or bounce on release.) The number is updated only when bounce happens.

mondalaci commented 4 years ago

I ended up reverting my change because the 5 ms debouncing interval introduced chatter. 5 ms shouldn't cause chatter based on MX switch specifications, but there are probably some outlier switches when it comes to manufacturing, and some switches may degrade throughout their lifetime.

Given that the firmware activates key presses instantly upon pressing switches, I don't consider this to be an issue after all, so I'm closing it.