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

Refactor cospad to current standards and enable support for backlight keycodes #5582

Merged
merged 1 commit into from Apr 24, 2019

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Apr 8, 2019

Description

Refactor staryu to current standards;

  • Use of '#pragma once
  • Cleaned up default keymap

Rotated LAYOUT_gamepad_6x4 back to portrait in the Configurator to match the name and macro.

Enable support for backlight keycodes;

Types of Changes

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

Checklist

  • My code follows the code style of this project.
  • 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).

@zvecr
Copy link
Member Author

zvecr commented Apr 8, 2019

@Njiallu This change should allow you to update your detrus keymap to use the standard backlight functions or keycodes.

@noroadsleft
Copy link
Member

Rotated LAYOUT_gamepad_6x4 back to portrait in the Configurator to match the name and macro.

This was a deliberate choice because for gamepad usage, it's supposed to be landscape:

Realistically, the layout macro should be 6 columns and 4 rows (physically; even though it says 6x4); I didn't edit the layout macro when I added the Configurator data for that layout.

@zvecr
Copy link
Member Author

zvecr commented Apr 8, 2019

@noroadsleft I feel the name and layout Configurator should all correlate. From a conceptual point, rotating a board (where it makes sense) is something that could be entirely done in the Configurators UI.

Given 1 keymap already exists in the repo, a good short term compromise could be to add both 6x4 and 4x6. However i will also back out the change if its the consensus.

@noroadsleft
Copy link
Member

I feel the name and layout Configurator should all correlate.

That's fair. I left the name because I would have had to edit additional files. I was comfortable doing that, but it wasn't relevant to my goal, so I left it alone. The macro could just leave off the layout dimensions (LAYOUT_gamepad, without specifying 6x4/4x6). The layout and corresponding keymap were submitted with that name.

From a conceptual point, rotating a board (where it makes sense) is something that could be entirely done in the Configurators UI.

That's exactly what I did Notice the first four keys are all "x": 5; landscape for gamepad puts the top (in portrait) to the right, so the "first row" is on the right-edge of the board.

If I had to do it again, I probably would rotate the layout macro and regenerate the info.json object.

#define LAYOUT_gamepad( \
	k50, k40, k30, k20, k10, k00, \
	k51, k41, k31, k21, k11, k01, \
	k52, k42, k32, k22, k12, k02, \
	k53, k43,   k23,    k13, k03  \
) \
{ \
	{k00, k01, k02, k03},   \
	{k10, k11, k12, k13},   \
	{k20, k21, k22, k23},   \
	{k30, k31, k32, KC_NO}, \
	{k40, k41, k42, k43},   \
	{k50, k51, k52, k53}    \
}

That would require editing the detrus keymap though.

@mechmerlin
Copy link
Contributor

mechmerlin commented Apr 8, 2019

The rotated numpad is one of the reasons why I really wanted this board, unfortunately they don't actually sell that case anymore. By the time I decided I wanted one, it was sold out and the new and "improved" v2 was deemed the cospad case.

On that note, depending on rotation, bootmagic lite key will change position. Can we change its location based on the LAYOUT_macro in use? @drashna

@drashna
Copy link
Member

drashna commented Apr 8, 2019

@mechmerlin No, we can't. The best we can do is have a define that controls the bootmagic config.

Copy link
Contributor

@mechmerlin mechmerlin left a comment

Choose a reason for hiding this comment

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

Thanks!

@mechmerlin mechmerlin merged commit e6f4173 into qmk:master Apr 24, 2019
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Apr 26, 2019
* 'master' of https://github.com/qmk/qmk_firmware: (99 commits)
  [Keyboard] Add a new keyboard ADKB96 (qmk#5685)
  test commit
  add RGBLIGHT_SPLIT_SET_CHANGE_MODEHSVS; to rgblight_update_dword()
  add RGBLIGHT_SPLIT_SET_CHANGE_MODEHSVS; to eeconfig_update_rgblight_default()
  Refactor cospad to current standards and enable support for backlight keycodes (qmk#5582)
  [Keymap] update (mouse emulation, rev 6 compatibility) (qmk#5696)
  [Keyboard] fix project zen rev1 bootloader declaration (qmk#5695)
  [FIX] Misspelled RGB_YELLOW (qmk#5692)
  [Keymap] Fix broken Shift-Insert binding (qmk#5689)
  [Keyboard] forgot to omit k05 from the electrical matrix in hhkb layout (qmk#5684)
  [Keyboard] Fix red an green leds location (qmk#5698)
  Translate docs into Chinese (qmk#5693)
  [Keymap] Fix my userspace RGB bug (qmk#5686)
  Boston meetup 2019 (qmk#5611)
  [Keymap] Update to Drashna Keymaps (qmk#5594)
  fix LIB_SRC and QUANTUM_LIB_SRC for ARM (qmk#5623)
  RGB Matrix Animations: Three/six new reactive effects (wide, cross, nexus) (qmk#5602)
  Fix qmk#3566 use an hardware timer for software PWM stability (qmk#3615)
  added info.json for ymd96 (qmk#4982)
  Define RGB colors (qmk#5300)
  ...
foosinn pushed a commit to foosinn/qmk_firmware that referenced this pull request May 6, 2019
Timbus pushed a commit to Timbus/qmk_firmware that referenced this pull request Jun 23, 2019
JeffreyPalmer pushed a commit to JeffreyPalmer/qmk_firmware that referenced this pull request Feb 27, 2020
@zvecr zvecr deleted the feature/cospad_refactor branch April 28, 2020 00:14
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.

None yet

4 participants