[Bug] encoder_update can undersample and discard valid/recoverable encoder events

[IBNobody]

Describe the Bug

The way that the encoder_read and encoder_update is written, slow matrix scan rates can cause these functions to miss partially malformed encoder events that a human could have recognized as being valid. (i.e. "I know encoder event took place, and I know what direction the encoder was turned.")

For an EC11-style encoder with a resolution of 4, encoder_update needs to see 4 specific transitions of the encoder's A and B lines in order to detect an encoder event and to determine encoder direction.

Clockwise - Negative Points

• A falling with B high: (1,1) to (0, 1)
• B falling with A low: (0,1) to (0, 0)
• A rising with B low: (0,0) to (1, 0)
• B rising with A high: (1, 0) to (1, 1)

Counter-clockwise - Positive Points

• B falling with A high: (1,1) to (1, 0)
• A falling with B low: (1,0) to (0, 0)
• B rising with A low: (0,0) to (0, 1)
• A rising with B high: (0, 1) to (1, 1)

Each detected transition above counts as a point, and a resolution 4 encoder needs 4 points, either negative or positive, to be counted as a full encoder event. (Due to how the encoder is set up, you never get a mix of positive and negative points.) Resolution 8 encoders need 2 full sets of 4 points. Resolution 2 encoders only need 2 points.

When a keyboard's matrix scan rate is low, the encoder_read function may undersample an EC11-style encoder's data lines.

Instead of seeing 4 transitions, it may only see 2 or 3 events. And it might also detect some events that have no scores.

• A and B rising: (0,0) to (1,1)
• A and B falling: (1,1) to (0,0)
• A rising while B falling: (0,1) to (1,0)
• A falling while B rising: (1,0) to (0,1)
• A and B not changing: (0,0) to (0,0)
• A and B not changing: (1,0) to (1,0)
• A and B not changing: (0,1) to (0,1)
• A and B not changing: (1,1) to (1,1)

I think this is a bug because we're throwing out encoder events that we could have caught.

Potential solution

Instead of scoring based on which events were detected, look at the order in which the pulse events are detected and keep track of which events were seen.

A pulse is a combination of ...

...a start event such as...

• A falling with B high: (1,1) to (0, 1)
• B falling with A high: (1,1) to (1, 0)
• A and B falling: (1,1) to (0,0)

...a middle event such as...

• A and B both low: (don't care, don't care) to (0,0)

... and a stop event such as...

• B rising with A high: (1, 0) to (1, 1)
• A rising with B high: (0, 1) to (1, 1)
• A and B rising: (0,0) to (1,1)

(If the detected start was "A and B falling" and the detected stop was "A and B rising", discard that pulse because you can't tell direction.)

[IBNobody]

btw, this straddles the line between bug and feature. It's a bug for me because the current kb I have IS undersampling my encoder, but that under-sampling is also being caused by non-optimized code. If the mods want to reclassify this, go for it.

[sharpenedblade]

I think this should be closed, #16932 has been merged.