[Core] Don't send keyboard reports that propagate no changes to the host

[KarlK90]

Description

As the title suggest, with this change only keyboard reports that propagate changes to the host get send at all. This PR builds on to of #13789 so the change set is obviously a bit larger than necessary. Only ab2e2b2 and 5c9fb10 contain the necessary changes.
Note that the method used is a bit brute force as it doesn't fix the logic bugs under the hood.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

• None

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[stale]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[tzarc]

Approval in principle, though I want to get the tests and the like merged first.

[KarlK90]

The merge conflicts should resolve automatically once #15269 is merged.

[zvecr]

As the PR doesn't mention the reasoning why this change is required, could we get that updated. RIght now its not clear if this is for anything other than optimisation, and then how much risk there is with the intended change.

Note that the method used is a bit brute force as it doesn't fix the logic bugs under the hood.

Can we get something written on the PR as to what those roughly are. Just so we have something a little more concrete to point to.

[KarlK90]

Sure I could have been a bit more clear in this regard, also the commit messages don't say much.

The main arguments for me are latency and correctness. As sending a keyboard report is a blocking operation any report that is redundant increases the latency for a real change to be propagated to the host and is technically not needed. I would also argue that the behavior is not correct even if these reports do no harm in the first place. I stumbled upon this issue while writing unit-tests and was a bit baffled by the amount of reports generated under some circumstances.

The logic issues mentioned are i.e. functions like clear_keyboard_but_mods_and_keys that unconditionally send a keyboard report after clearing the keyboard state without checking the overall state of the keyboard. Or maybe it is the other way around that the callers don't check if the keyboard state really needs a "clean slate".

All in all I thought that in order to fix the issue it would be easier to have this diffing in place.

[tzarc]

Given the outstanding queries and the like I've deferred this to next cycle. I'd rather merge it on the first day and have 3 months to iron out issues rather than trying to scramble to find fixes on master.

The whole point for the develop merge branch was to minimise disruption, and in this respect a change like this with its introduced risk is not appropriate for a last-day merge.

[KarlK90]

Sure, seems like reasonable decision to me. Looking forward to getting it merged in the next cycle.