qmk_firmware icon indicating copy to clipboard operation
qmk_firmware copied to clipboard

Fix functions when `NO_ACTION_TAPPING` is defined

Open drashna opened this issue 3 years ago • 5 comments

Description

Changes so that when NO_ACTION_TAPPING is defined, that layer function act as expected. Eg, that MO and TG still work.

  • LT's work as just their keycode
  • MT's don't work at all (can't figure out where they're trigger when tapping is disabled)
  • One Shot Layers work as MO
  • One Shot mods don't work (yet)
  • TT doesn't work.
  • Tap Dance works (with a fix), considering disabling outright as it is heavily "tapping" dependant
  • File formatted

Types of Changes

  • [x] Core
  • [x] Bugfix
  • [x] Enhancement/optimization
  • [ ] Documentation

Issues Fixed or Closed by This PR

  • Fixes #5735

Checklist

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

drashna avatar Jan 12 '21 18:01 drashna

Sorry about that, GitHub decided to delete the develop branch from upstream when we merged it, despite being told otherwise. Reopened.

tzarc avatar Feb 27 '21 20:02 tzarc

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.

stale[bot] avatar Apr 18 '21 23:04 stale[bot]

Oooh, it looks like this may be the solution to my perennial Crkbd firmware size issues. :) When I went to rebase my local keymap branch so I could send a PR, I realized I was over size with avr-gcc 5.4.0. Downloading and using an 8.5.0 toolchain brought me under size, but just barely (even with everything I could find that I didn't want disabled in rules.mk and config.h:

Checking file size of crkbd_rev1_bcat_split_3x6_3.hex                                               [WARNINGS]

 * The firmware size is approaching the maximum - 28570/28672 (99%, 102 bytes free)

With this PR and NO_ACTION_TAPPING enabled, I have a ton more bytes for OLED and RGB fun:

Checking file size of crkbd_rev1_bcat_split_3x6_3.hex                                               [OK]
 * The firmware size is fine - 26818/28672 (93%, 1854 bytes free)

Thanks for working on this fix! (I don't use any real tapping support, but I would be a sad person without MO keys.) I can give this PR a try on my keyboards this week if more testing is helpful.

bcat avatar Oct 04 '21 17:10 bcat

Thank you for fixing this. I just applied some of the changes locally and saw a huge size reduction, similar to what @bcat reported.

What’s the status on landing this to master? Any way we can pitch in?

rileyjshaw avatar Nov 30 '21 21:11 rileyjshaw

Yay! Thanks for giving this an update @drashna. Excited for it to land 🥳🎈

rileyjshaw avatar May 26 '22 18:05 rileyjshaw

Yay! Thanks for giving this an update drashna. Excited for it to land 🥳🎈

Sorry, I meant to get back to this sooner. On the plus side, I've learned a lot since. MT and others now work when tapping is disabled. Though, a lot more preprocessor stuff...

Need to add a bunch of tests to ensure that the behavior is correct and that it's not accidentally broken, later on.

drashna avatar Jan 10 '23 11:01 drashna

Epic, thanks @drashna!

rileyjshaw avatar Jan 10 '23 14:01 rileyjshaw

Epic!!!!!! Congrats on landing this and a huge thank you to @drashna!

rileyjshaw avatar Feb 19 '23 02:02 rileyjshaw