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

[Keyboard] Add Electronlab KLOR keyboard definitions #24372

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

Lefuneste83
Copy link

Electronlab KLOR keyboard definitions

-Electronlab built fork of KLOR keyboard definitions for use with RP2040 MCU boards

Description

This PR aims at bringing a complete set of features for the KLOR keyboard hardware, thus requiring the use of a RP2040 MCU due to space constraints on Atmega AVR chips.
Current version compiles without errors and enables all supported embedded hardware, as well as ability to chose any of the 4 layouts of the KLOR keyboard.

It is an initial PR and will likely require heavy scrutiny and revisions. I understand QMK aims at a minimal set of default features but the goal of this keyboard being to be feature rich, I have found many users struggling while trying to implement various functionalities. The goal of this PR is to bring a set of config files ready to be compiled with as clean as possible code following latest standard and syntax.

Types of Changes

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

Issues Fixed or Closed by This PR

Allows KLOR keyboard users to have access to a fully featured firmware which is currently impossible :

  • 3 year old legacy files provided by the project creator will not compile against latest QMK codebase due to change in syntax and architecture
  • Latest merged version under geistgeistgeist/klor will only bring a minimal set of hardware features as it is based on Atmega AVR definitions and firmware limitations

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.
  • I have updated the documentation accordingly.
  • [ X] I have read the CONTRIBUTING document.
  • 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).

	nouveau fichier : electronlab/klor/halconf.h
	nouveau fichier : electronlab/klor/keyboard.json
	nouveau fichier : electronlab/klor/keymaps/default/config.h
	nouveau fichier : electronlab/klor/keymaps/default/keymap.c
	nouveau fichier : electronlab/klor/keymaps/default/rules.mk
	nouveau fichier : electronlab/klor/klor.c
	nouveau fichier : electronlab/klor/klor.h
	nouveau fichier : electronlab/klor/lib/glcdfont.c
	nouveau fichier : electronlab/klor/lib/klorimation.h
	nouveau fichier : electronlab/klor/lib/klounds.h
	nouveau fichier : electronlab/klor/mcuconf.h
	nouveau fichier : electronlab/klor/readme.md
	nouveau fichier : electronlab/klor/rules.mk
	supprimé :        electronlab/klor/halconf.h
	supprimé :        electronlab/klor/keyboard.json
	supprimé :        electronlab/klor/keymaps/default/config.h
	supprimé :        electronlab/klor/keymaps/default/keymap.c
	supprimé :        electronlab/klor/keymaps/default/rules.mk
	supprimé :        electronlab/klor/klor.c
	supprimé :        electronlab/klor/klor.h
	supprimé :        electronlab/klor/lib/glcdfont.c
	supprimé :        electronlab/klor/lib/klorimation.h
	supprimé :        electronlab/klor/lib/klounds.h
	supprimé :        electronlab/klor/mcuconf.h
	supprimé :        electronlab/klor/readme.md
	supprimé :        electronlab/klor/rules.mk
@Lefuneste83 Lefuneste83 changed the title Electronlab KLOR keyboard definitions [Keyboard] Add Electronlab KLOR keyboard definitions Sep 7, 2024
Copy link
Contributor

@sigprof sigprof left a comment

Choose a reason for hiding this comment

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

This is a preliminary review which points out the most obvious places that should be fixed. In particular, I did not actually look at the content of the default keymap, because it violates one of the items in the PR Checklist:

  • default keymaps should be "pristine"

You may be able to add a "pristine" default keymap, and provide your fancy keymap as a “vendor” keymap. Also keep in mind that QMK Configurator currently generates a bare keymap that expands to just the keymaps array init and nothing else (and it would be a standalone keymap which does not inherit anything from default, so any code or configuration which is required for the keyboard to work must be at the keyboard level, not in the default keymap).

keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/klor.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/klor.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/mcuconf.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/readme.md Outdated Show resolved Hide resolved
keyboards/electronlab/klor/rules.mk Outdated Show resolved Hide resolved
@Lefuneste83
Copy link
Author

Thank you very much for the immensely helpful code review. This has helped me quite a lot progress in the migration to json definitions which still had some hidden secrets. Let me know of any further changes you deem necessary.

@Lefuneste83
Copy link
Author

Totally cleaned up the default keymap.c for ultra minimal mapping and created a vendor specific keymap directory with extra settings enabled

keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keyboard.json Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keymaps/default/rules.mk Outdated Show resolved Hide resolved
	modifié :         keyboards/electronlab/klor/keyboard.json
	modifié :         keyboards/electronlab/klor/keymaps/default/config.h
	modifié :         keyboards/electronlab/klor/keymaps/default/keymap.c
	supprimé :        keyboards/electronlab/klor/keymaps/default/rules.mk
	modifié :         keyboards/electronlab/klor/keymaps/electronlab/config.h
	modifié :         keyboards/electronlab/klor/keymaps/electronlab/keymap.c
	modifié :         keyboards/electronlab/klor/keymaps/electronlab/rules.mk
	modifié :         keyboards/electronlab/klor/keymaps/electronlab/keymap.c
	renommé :         keyboards/electronlab/klor/glcdfont.c -> keyboards/electronlab/klor/lib/glcdfont.c
keyboards/electronlab/klor/config.h Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keyboard.json Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keyboard.json Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/electronlab/klor/keyboard.json Show resolved Hide resolved
keyboards/electronlab/klor/klor.c Outdated Show resolved Hide resolved
keyboards/electronlab/klor/lib/glcdfont.c Show resolved Hide resolved
keyboards/electronlab/klor/lib/glcdfont.c Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants