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

Draytronics Keyboards Refactor & Daisy V2 MacroPad Addition #23873

Closed
wants to merge 19 commits into from

Conversation

ghostseven
Copy link
Contributor

Description

Refactoring of the Draytronics keyboards to make sure the configuration is up to date and the readme.md files are suitable. Adjustment of "tap_keycode_delay" in Elise (V1/V2) keyboards to resolve issues related to KVM and key repeating on wake.

Addition of a Daisy V2 MacroPad with rotary encoder and OLED display on a STM32 base.

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

@ghostseven ghostseven marked this pull request as draft June 6, 2024 12:11
@ghostseven ghostseven marked this pull request as ready for review June 6, 2024 12:18
Copy link
Member

@waffle87 waffle87 left a comment

Choose a reason for hiding this comment

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

Keyboard updates need to target the develop branch.

keyboards/draytronics/daisy_v2/daisy_v2.c Outdated Show resolved Hide resolved
keyboards/draytronics/daisy_v2/daisy_v2.c Outdated Show resolved Hide resolved
keyboards/draytronics/daisy_v2/keyboard.json Outdated Show resolved Hide resolved
},
"diode_direction": "COL2ROW",
"encoder": {
"enabled": true,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"enabled": true,

keyboards/draytronics/daisy_v2/keyboard.json Outdated Show resolved Hide resolved
Comment on lines +12 to +16
qmk compile -kb draytronics/daisy_v2 -km default

Flashing example for this keyboard:

qmk flash -kb draytronics/daisy_v2 -km default
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
qmk compile -kb draytronics/daisy_v2 -km default
Flashing example for this keyboard:
qmk flash -kb draytronics/daisy_v2 -km default
make draytronics/daisy_v2:default
Flashing example for this keyboard:
make draytronics/daisy_v2:default:flash

keyboards/draytronics/daisy_v2/readme.md Show resolved Hide resolved
Make example for this keyboard (after setting up your build environment):

make draytronics/elise:default
qmk compile -kb draytronics/elise -km default
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this as it is preferred to have the make example in keyboard readme's.

keyboards/draytronics/elise_v2/config.h Show resolved Hide resolved
@@ -20,13 +20,13 @@ Entering DFU mode (to allow flashing):

Make example for this keyboard (after setting up your build environment):

make draytronics/elise_v2:default
qmk compile -kb draytronics/elise_v2 -km default
Copy link
Member

Choose a reason for hiding this comment

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

As above.

@waffle87 waffle87 changed the base branch from master to develop June 6, 2024 19:40
ghostseven and others added 13 commits June 6, 2024 20:58
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
Image moved to imgur as request and Bootloader instructions added.
Co-authored-by: jack <0x6a73@protonmail.com>
* add syenakeyboard elaruus

* add syenakeyboards elaruus

* add syenakeyboards elaruus

* add syenakeyboards elaruus

* add syenakeyboards/elaruus

* add syenakeyboards elaruus

* add syenakeyboards elaruus

* add syenakeyboards elaruus

* Update keyboards/syenakeyboards/elaruus/keyboard.json

Co-authored-by: jack <0x6a73@protonmail.com>

* Update keyboards/syenakeyboards/elaruus/keymaps/default/keymap.c

Co-authored-by: jack <0x6a73@protonmail.com>

* Update keyboards/syenakeyboards/elaruus/keymaps/via/keymap.c

Co-authored-by: jack <0x6a73@protonmail.com>

---------

Co-authored-by: Syenasweta <syenasweta@gmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
@github-actions github-actions bot added documentation via Adds via keymap and/or updates keyboard for via support labels Jun 8, 2024
@@ -88,7 +88,7 @@ https://github.com/qmk/qmk_firmware/pulls?q=is%3Apr+is%3Aclosed+label%3Akeyboard
- RGB Matrix Configuration
- Run `qmk format-json` on this file before submitting your PR. Be sure to append the `-i` flag to directly modify the file, or paste the outputted code into the file.
- `readme.md`
- must follow the [template](https://github.com/qmk/qmk_firmware/blob/master/data/templates/keyboard/readme)
- must follow the [template](https://github.com/qmk/qmk_firmware/blob/master/data/templates/keyboard/readme.md)
Copy link
Member

Choose a reason for hiding this comment

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

This should be reverted. Any changes here should be a separate PR

#include "daisy_v2.h"

enum my_keycodes {
ENCODER_PRESS = SAFE_RANGE
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ENCODER_PRESS = SAFE_RANGE
ENCODER_PRESS = QK_KB,

return false;
}
#ifdef OLED_ENABLE
key_pressed = record->event.pressed;
Copy link
Member

Choose a reason for hiding this comment

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

Not the best way to handle this. If you press two keys, and release on, this will be "false", which isn't true.

If anything, using a uint8_t instead of bool, and:

Suggested change
key_pressed = record->event.pressed;
if (record->event.pressed) {
key_pressed++;
} else {
if (key_pressed)
key_pressed--;
}

case ENCODER_PRESS:
if (record->event.pressed) {
// on tap
if (record->tap.count) {
Copy link
Member

Choose a reason for hiding this comment

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

If you're not manually setting tap.count, then this isn't populated, normally.

Copy link
Member

Choose a reason for hiding this comment

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

As much as some of these are cool, the maxtac and arasaka logos need to be removed.

@ghostseven
Copy link
Contributor Author

Sorry I am quite confused here it appears that I have done something very wrong here as there are files from the documentation and PR #23870. Do I need to close this and restart this whole PR?

@ghostseven ghostseven closed this Jun 9, 2024
@ghostseven
Copy link
Contributor Author

My apologies I am going to refactor this as changes I have made as requested here have caused issues I will resubmit at a later date. Thank you all for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants