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

feat(shields): Add Fifi Shield Support #1008

Closed
wants to merge 11 commits into from
Closed

Conversation

hmngwy
Copy link

@hmngwy hmngwy commented Oct 31, 2021

Board/Shield Check-list

  • This board/shield is tested working on real hardware
  • Definitions follow the general style of other shields/boards upstream (Reference)
  • .zmk.yml metadata file added
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • General consistent formatting of DeviceTree files
  • Keymaps do not use deprecated key defines (Check using the upgrader tool)
  • &pro_micro used in favor of &pro_micro_d/a if applicable
  • If split, no name added for the right/peripheral half
  • .conf file has optional extra features commented out

.github/workflows/build.yml Outdated Show resolved Hide resolved
app/boards/shields/fifi/Kconfig.defconfig Show resolved Hide resolved
app/boards/shields/fifi/Kconfig.shield Outdated Show resolved Hide resolved
app/boards/shields/fifi/fifi.dtsi Outdated Show resolved Hide resolved
app/boards/shields/fifi/fifi.dtsi Outdated Show resolved Hide resolved
app/boards/shields/fifi/fifi.keymap Outdated Show resolved Hide resolved
app/boards/shields/fifi/fifi_left.overlay Outdated Show resolved Hide resolved
app/boards/shields/fifi/fifi_right.overlay Outdated Show resolved Hide resolved
@Nicell Nicell added enhancement New feature or request shields PRs and issues related to shields labels Jan 30, 2022
hmngwy and others added 7 commits February 21, 2022 09:59
Co-authored-by: Nick Winans <nick@winans.codes>
Co-authored-by: Nick Winans <nick@winans.codes>
Co-authored-by: Nick Winans <nick@winans.codes>
Co-authored-by: Nick Winans <nick@winans.codes>
Co-authored-by: Nick Winans <nick@winans.codes>
Co-authored-by: Nick Winans <nick@winans.codes>
@hmngwy
Copy link
Author

hmngwy commented Feb 22, 2022

@Nicell added the missing copyright lines, updated to 2022, ditched the changes to build.yml

Copy link
Member

@Nicell Nicell left a comment

Choose a reason for hiding this comment

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

Please remove the fifi_left.conf and fifi_right.conf files! Otherwise I think this definition looks good. Thanks. You can re-request my review after updating.

@petejohanson
Copy link
Contributor

@hmngwy Please feel free to re-open this once you have a chance to address the item from @Nicell and this is ready for review again.

Thanks!

@hmngwy
Copy link
Author

hmngwy commented Feb 21, 2023

Hi @petejohanson, rebased with main and then reapplied my changes, but I'm getting the below:

FATAL ERROR: command exited with status 1: /usr/local/bin/cmake -DWEST_PYTHON=/usr/bin/python3 -B/__w/zmk/zmk/build -S/__w/zmk/zmk/app -GNinja -DBOARD=nice_nano -DSHIELD=fifi
Error: Failed to build or upload nice_nano fifi undefined

https://github.com/hmngwy/zmk/actions/runs/4234720633/jobs/7357427250#step:10:140

I'm not quite sure what I'm doing wrong, I checked the New Keyboard Shields guide as well if anything there is new since I slept on this PR, and it seems what I added here seems to still comply, however the error says otherwise and I can't tell what I'm missing.

If you're not too busy, a quick look would be appreciated. Thanks!

@caksoylar
Copy link
Contributor

caksoylar commented Feb 21, 2023

@hmngwy You should be building with fifi_left and fifi_right shields separately, right? I.e. you need to add the siblings property in the metadata file, at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request shields PRs and issues related to shields
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants