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

UniGo66 keyboard added #4913

Merged
merged 16 commits into from
Jan 27, 2019
Merged

UniGo66 keyboard added #4913

merged 16 commits into from
Jan 27, 2019

Conversation

dededecline
Copy link
Contributor

Description

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.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document. (https://docs.qmk.fm/#/contributing)
  • 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).

noroadsleft
noroadsleft requested changes Jan 22, 2019
View reviewed changes
keyboards/sirius/unigo66/rules.mk Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/info.json Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/danielhklein/keymap.c Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/danielhklein/keymap.c Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/unigo66.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/unigo66.h Show resolved Hide resolved
keyboards/sirius/unigo66/README.md Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/danielhklein/config.h Outdated Show resolved Hide resolved
drashna
drashna requested changes Jan 22, 2019
View reviewed changes
keyboards/sirius/unigo66/config.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/config.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/danielhklein/keymap.c Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/config.h Outdated Show resolved Hide resolved
keyboards/sirius/unigo66/keymaps/danielhklein/keymap.c Show resolved Hide resolved
@noroadsleft
Copy link
Member

Putting this here to get more eyes on it because I'm a bit out of my depth here.

@danielhklein sent me this Gist via Discord on Tuesday, saying, in part:

Hey, on the PR for the UniGo66, adding the #include "config_common.h" breaks build, I'm not 100% sure why but I have suspicions
I think it's because of the weird custom_matrix implementation, but honestly I wouldn't know where to begin in debugging or rewriting that

I know that the error is because one file is trying to define variables that are already defined, specifically these:

qmk_firmware/tmk_core/protocol/usb_hid/arduino-1.0.1/cores/arduino/binary.h

Line 4 in 0ba9620

#define B0 0

qmk_firmware/tmk_core/protocol/usb_hid/arduino-1.0.1/cores/arduino/binary.h

Line 12 in 0ba9620

#define B1 1

which are already defined here:

qmk_firmware/quantum/config_common.h

Lines 80 to 81 in 0ba9620

#define B0 PINDEF(B, 0)
#define B1 PINDEF(B, 1)

I don't know quite enough to say whether including config_common.h is required for every build, and as I don't have the keyboard in question, I can't test it.

@dededecline
Copy link
Contributor Author

@noroadsleft thanks for posting that here. I actually have some updated files for the config and the matrix, I will be testing them tonight but I suspect they may resolve these issues. So before anyone dives too deep, just wanted to say that this may already be solved.

@dededecline
Copy link
Contributor Author

All suggested changes have been accepted other than the inclusion of #include config_common.h. in discussion with @drashna on Discord, this should be okay due to the configuration of UniGo66

noroadsleft
noroadsleft requested changes Jan 27, 2019
View reviewed changes
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

All the files in keyboards/unigo66/ should be removed.

@dededecline
Copy link
Contributor Author

dededecline commented Jan 27, 2019 •
edited
Loading

@noroadsleft I don't think the keyboards/unigo66 files still exist. They don't show up in my repo at latest commit, and from the PR's "Files Changed" section they also are not present

edit: confirmed, they've been removed

@drashna
Copy link
Member

drashna commented Jan 27, 2019

Thanks!

@drashna drashna merged commit 9ae800f into qmk:master Jan 27, 2019
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Jan 27, 2019
@Shinichi-Ohki
Merge branch 'master' of https://github.com/qmk/qmk_firmware
b7f634d 
* 'master' of https://github.com/qmk/qmk_firmware:
  Fix Command feature: use get_mods() instead of keyboard_report->mods (qmk#4955)
  [Keymap] Small improvements to Maxr1998's Contra keymap (qmk#4952)
  [Keymap] Minor updates to my dz60 keymap (qmk#4943)
  [Keyboard] UniGo66 keyboard added (qmk#4913)
  [Keymap] Move Iris via support to Via keymap (qmk#4893)
  Adds a default value for IS_COMMAND for COMMAND feature (qmk#4301)
  [Keyboard] drop unused i2c files (qmk#4948)
  [Keymap] Add Maxim keymap for Fourier (qmk#4534)
  use built-in arm stuff
  [Keymap] Add userspace files for vosechu (qmk#4912)
zer09 pushed a commit to zer09/qmk_firmware that referenced this pull request Feb 9, 2019
@zer09
[Keyboard] UniGo66 keyboard added (qmk#4913)
fb79523 
* UniGo66 keyboard added

* UniGo66 keyboard added

* case correction of unigo66 files

* create sirius folder

* Update keyboards/sirius/unigo66/rules.mk

Co-Authored-By: danielhklein <[email protected]>

* Update keyboards/sirius/unigo66/keymaps/danielhklein/keymap.c

Co-Authored-By: danielhklein <[email protected]>

* Update keyboards/sirius/unigo66/keymaps/default/config.h

Co-Authored-By: danielhklein <[email protected]>

* Update keyboards/sirius/unigo66/keymaps/danielhklein/config.h

Co-Authored-By: danielhklein <[email protected]>

* debugging

* correct keymap to layout

* readme

* remove common config

* suggested changes to config.h

* default keymap cleanup
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
[Keyboard] UniGo66 keyboard added (qmk#4913)
c972a0d 
* UniGo66 keyboard added

* UniGo66 keyboard added

* case correction of unigo66 files

* create sirius folder

* Update keyboards/sirius/unigo66/rules.mk

Co-Authored-By: danielhklein <[email protected]>

* Update keyboards/sirius/unigo66/keymaps/danielhklein/keymap.c

Co-Authored-By: danielhklein <[email protected]>

* Update keyboards/sirius/unigo66/keymaps/default/config.h

Co-Authored-By: danielhklein <[email protected]>

* Update keyboards/sirius/unigo66/keymaps/danielhklein/config.h

Co-Authored-By: danielhklein <[email protected]>

* debugging

* correct keymap to layout

* readme

* remove common config

* suggested changes to config.h

* default keymap cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vomindoraan vomindoraan vomindoraan left review comments

@noroadsleft noroadsleft noroadsleft requested changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
awaiting changes keyboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dededecline @noroadsleft @drashna @vomindoraan