Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleanup and refactoring of Cipulot keyboards folder #21577

Closed
wants to merge 7 commits into from
Closed

Cleanup and refactoring of Cipulot keyboards folder #21577

wants to merge 7 commits into from

Conversation

Cipulot
Copy link
Contributor

@Cipulot Cipulot commented Jul 21, 2023 •
edited
Loading

Description

This commit includes cleanups and changes in includes of the via_apc.c file for affected keyboards, after the move of the file to a dedicated common folder in commit #21233.

Here's a mode detailed list of the changes:

  • cleanup of empty layers in the keymap.c files
  • removal of features in info.json matching features in the default info.json
  • delete of duplicated files in the via keymap folders of via_apc.c file and subsequent inclusion path update in rules.mk
  • addition of OPT flags to increase scan rate through optimization
  • fix of keycodes order in some keymaps

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

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).

@Cipulot
Cleanup and refactoring.
589b105 
This commit includes cleanups and changes in includes of the 'via_apc.c` file for affected keyboards, after the move of the file to a dedicated `common` folder in commit [qmk#21233](qmk#21233).

Here's a mode detailed list of the changes:
- cleanup of empty layers in the `keymap.c` files
- removal of features in `info.json` matching features in the default `info.json`
- delete of duplicated files in the `via` keymap folders of `via_apc.c` file and subsequent inclusion path update in `rules.mk`
@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Jul 21, 2023
@Cipulot Cipulot changed the title Cleanup and refactoring. Cleanup and refactoring of Cipulot keyboards folder Jul 21, 2023
@dunk2k
Copy link
Contributor

dunk2k commented Jul 22, 2023

Could this PR be targeted at the develop branch?
as per PR Checklist:
"keyboard updates and refactors (eg. to data driven) must go through develop to reduce master -> develop merge conflicts"

@Cipulot
Copy link
Contributor Author

Cipulot commented Jul 22, 2023 •
edited
Loading

@dunk2k I didn't targeted this to develop simply because the base change was applied to main in the merge 21233.

It would create confusion to have multiple boards with the replicated code within the keymaps folder and the latest one being the only one with the latest changes requested to reduce code duplication.
Second opinions on the best target for this are more than welcome form other maintainers. Will act accordingly.

Cipulot added 2 commits July 27, 2023 23:59
@Cipulot
More cleanup
880ee54 
removed unused parameter in function proto.
@Cipulot
Fix CI build
188a854
@Cipulot Cipulot changed the base branch from master to develop July 27, 2023 22:14
@Cipulot
Copy link
Contributor Author

Cipulot commented Jul 27, 2023

Retargeted to develop, since more cleanup was made, now it configures more with the requirements specified in the PR checklist.

Cipulot added 3 commits July 28, 2023 00:45
@Cipulot
Merge branch 'develop' into cipulot_cleanup_and_refactoring
dcd85af
@Cipulot
OPT addition
8ff52ee 
Addition of `OPT = 2` for boards that HAVE RGB and `OPT = 3` for boards that DO NOT HAVE RGB.

This because of known issues with having `OPT = 3` enabled with RGB and ARM, causing it to glitch out.

The optimization flag proved to increase scan rate by a range of 15%-19% where RGB is not used and by ~5% where it is used.
@Cipulot
Fix HHKB style keycode order
911c189 
Small order fix for HHKB keymaps on my boards. Previously I swapped xGUI and xALT.
Not critical but still a QOL improvement.
@Cipulot
Copy link
Contributor Author

Cipulot commented Aug 14, 2023

Closing this since I've noticed that some of the EC code can be reworked and these changes are no longer needed, will do a separate and more comprehensive refactoring down the line (work already started on that).

@Cipulot Cipulot closed this Aug 14, 2023
@Cipulot Cipulot deleted the cipulot_cleanup_and_refactoring branch August 14, 2023 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesshonor lesshonor lesshonor approved these changes

Assignees
No one assigned
Labels
keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Cipulot @dunk2k @lesshonor