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.

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

A fancy keymap for the wt65_xt. #19375

Merged
merged 5 commits into from
Dec 30, 2022
Merged

A fancy keymap for the wt65_xt. #19375

merged 5 commits into from
Dec 30, 2022

Conversation

yonatanzunger
Copy link
Contributor

TODO: Extract a lot of this logic and make it more widely usable for polyglot keyboards!

Description

Adding a custom keymap. Testing out several ideas to generalize later!

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

TODO: Extract a lot of this logic and make it more widely usable for
polyglot keyboards!
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#pragma once
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is nothing here, then this file can (and should) be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

*/
#include QMK_KEYBOARD_H
#include <assert.h>
#include "wt65_xt.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't needed. QMK_KEYBOARD_H is generated as this, already.

Suggested change
#include "wt65_xt.h"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

_MAC = 1,
_OS_MODES_MAX = 2,
};
static int os_mode = _MAC;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to use an unsigned 8 bit int, as it may save a bit of space. (also better to be specific)

Suggested change
static int os_mode = _MAC;
static uint8_t os_mode = _MAC;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea; done.

Also fix unicode mode selection, oops.
#define NON_LANGUAGE_LAYERS ~(((1UL << _LAST_LANGUAGE_LAYER) - 1) - ((1UL << _FIRST_LANGUAGE_LAYER) - 1))

// Update the current layer state and return the layer we're in.
int update_layer(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth converting these to uint8_t from int.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, done.

@drashna drashna requested a review from a team December 27, 2022 21:12
@tzarc tzarc merged commit 6bd76c9 into qmk:master Dec 30, 2022
sbhal pushed a commit to sbhal/qmk_firmware that referenced this pull request Dec 30, 2022
@yonatanzunger yonatanzunger deleted the wilba branch December 30, 2022 22:17
jasonisgraham pushed a commit to jasonisgraham/qmk_firmware that referenced this pull request Jan 2, 2023
omikronik pushed a commit to omikronik/qmk_firmware that referenced this pull request Jan 22, 2023
ideas32 pushed a commit to ideas32/qmk_firmware that referenced this pull request Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants