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

adding haven80 hotswap & haven65 pcbs firmware to qmk #20081

Closed
wants to merge 82 commits into from
Closed

adding haven80 hotswap & haven65 pcbs firmware to qmk #20081

wants to merge 82 commits into from

Conversation

CMMS-Freather
Copy link
Contributor

@CMMS-Freather CMMS-Freather commented Mar 10, 2023

adds following keyboard firmware to qmk repo
haven80 hotswap
haven65 solder
haven65 hotswap

updated fuji65 via keymap
updated teahouse/ayleen via keymap and config.h to achieve same indicator only control feature in via

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

CMMS-Freather and others added 30 commits January 6, 2022 15:59
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
defined specific rgb animations want to enable
Co-authored-by: Joel Challis <git@zvecr.com>
Co-authored-by: Ryan <fauxpark@gmail.com>
Co-authored-by: Ryan <fauxpark@gmail.com>
add support for fuji75 solder
fuji75 hotswap
and kp60 hotswap pcb
added config.h for firmware compilation
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
@github-actions github-actions bot added keymap via Adds via keymap and/or updates keyboard for via support labels Mar 10, 2023
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.

Not sure why the entire teahouse/ayleen directory is being added again, after #19900 was merged?

keyboards/ah/haven80_hotswap/config.h Outdated Show resolved Hide resolved
keyboards/ah/haven80_hotswap/info.json Outdated Show resolved Hide resolved
keyboards/ah/haven80_hotswap/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/ah/haven80_hotswap/keymaps/vial/rules.mk Outdated Show resolved Hide resolved
keyboards/ah/haven80_hotswap/readme.md Outdated Show resolved Hide resolved
keyboards/ah/haven80_hotswap/readme.md Show resolved Hide resolved
CMMS-Freather and others added 5 commits March 10, 2023 16:53
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>
@CMMS-Freather
Copy link
Contributor Author

Not sure why the entire teahouse/ayleen directory is being added again, after #19900 was merged?

i am not too sure why either. While making the pr, i only selected files that will be used in the havne80_hotswap.

@CMMS-Freather CMMS-Freather changed the title adding haven80 hotswap pcb firmware to qmk adding haven80 hotswap & haven65 pcbs firmware to qmk Mar 12, 2023
@zvecr
Copy link
Member

zvecr commented Mar 15, 2023

i am not too sure why either. While making the pr, i only selected files that will be used in the havne80_hotswap.

Probably because you have workflow issues, most likely due to the reused the branch from a previous PR without fixing the history first.

This will have to be resolved your end before the PR can be accepted.

keyboards/ah/haven65_hotswap/config.h Outdated Show resolved Hide resolved
as @zvecr suggested, changing all the config.h
#define eeconfig_user_data_size 4 to
#define eeconfig_kb_data_size 4

alongside, updating Ayleen corresponding file to allow the same indicator only control feature in via
@CMMS-Freather
Copy link
Contributor Author

i am not too sure why either. While making the pr, i only selected files that will be used in the havne80_hotswap.

Probably because you have workflow issues, most likely due to the reused the branch from a previous PR without fixing the history first.

This will have to be resolved your end before the PR can be accepted.

this has been resolved as I had to update some new files for the ayleen to allow the same indicator only control in via. Newly commited files for ayleen will be included

@CMMS-Freather CMMS-Freather requested review from zvecr and waffle87 and removed request for waffle87 and zvecr March 16, 2023 00:34
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

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

  • Use of EECONFIG_KB_DATA_SIZE at the keymap level is just as bad as eeconfig_update_kb_datablock at the keyboard level.
    • My preference would be EECONFIG_USER_DATA_SIZE defined at a keymap level config.h and eeconfig_update_user_datablock calls
  • keyboards/teahouse/ayleen isnt a set of updates to existing files, as they exist already.
    • The Files Changed tab needs to correctly reflect this and trying to re-add them is wrong

image

move eeconfig related code to keymap level as suggested
@CMMS-Freather CMMS-Freather marked this pull request as draft March 16, 2023 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants