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

duckboard compatibility #634

Closed
wants to merge 60 commits into from
Closed

duckboard compatibility #634

wants to merge 60 commits into from

Conversation

jackh2000
Copy link

This is written and tested firmware for the doodboard duckboard with a nice!nano. I have verified the features on my duckboard, and gotten reports from other's using my firmware for their duckboards from the doodboard discord server.

@Nicell Nicell self-requested a review January 26, 2021 16:42
@Nicell Nicell added enhancement New feature or request shields PRs and issues related to shields labels Jan 26, 2021
@dkmurray
Copy link

app/prj.conf needs to be removed in order for nice!nano v2 to build for any shield.

@petejohanson
Copy link
Contributor

app/prj.conf needs to be removed in order for nice!nano v2 to build for any shield.

I've been consider whether to zap that completely. Can you give me a link to a failed build to show what happens w/ it in place?

@dkmurray
Copy link

dkmurray commented Jul 29, 2021

It's specifically the prj.conf in this fork, the compare shows it as a new file with 3 lines. When the fork is built currently, all n!n v2 builds will fail with:

-- Configuring done
CMake Error at /workspaces/zmk/zephyr/cmake/extensions.cmake:416 (add_library):
  No SOURCES given to target: drivers__spi
Call Stack (most recent call first):
  /workspaces/zmk/zephyr/cmake/extensions.cmake:393 (zephyr_library_named)
  /workspaces/zmk/zephyr/drivers/spi/CMakeLists.txt:3 (zephyr_library)


CMake Generate step failed.  Build files cannot be regenerated correctly.
FATAL ERROR: command exited with status 1: /usr/bin/cmake -DWEST_PYTHON=/usr/bin/python3 -B/workspaces/zmk/app/build -S/workspaces/zmk/app -GNinja -DBOARD=nice_nano_v2 -DSHIELD=duckboard

Tried for duckboard, corne_left, and romac. When prj.conf is removed, the corne and romac build (the duckboard requires a nice_nano_v2.overlay file as well)

(note: this is all using the docker container of the duckboard fork in VSCode)

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

A few items, and then there are several build artifacts that snuck into this. Please remove those by doing an interactive rebase and editing the commits, so we avoid growing the git repo unnecessarily with those in the history.

Thanks!

@@ -0,0 +1,5 @@
/CMakeCache
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be necessary. Please remove the file.

@@ -0,0 +1,65 @@
# This is the CMakeCache file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Accidental addition, it seems.

config ZMK_KEYBOARD_NAME
default "duckboard"

config ZMK_USB
Copy link
Contributor

Choose a reason for hiding this comment

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

These defaults should be coming from the controller selected, not from the shield.

@@ -0,0 +1,51 @@
# Copyright (c) 2020 The ZMK Contributors
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep these files focused on settings specific to this shield, not general ZMK configuration items.

@@ -0,0 +1,3 @@
CONFIG_ZMK_RGB_UNDERGLOW=y
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this file

* Author: Jack Hartstein
*/

#include "duckboard.dtsi"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there's no left/right, etc nor need to re-use any common settings, please remove the duckboard.dtsi file and incorporate those DT items into this main overlay file.

@petejohanson
Copy link
Contributor

Thank you, contributor, for your patience with how long review and merge of boards/shields has taken!

There are three recent refactors/changes to boards and shields that require some attention, and then we can finally get this PR merged!

  1. Hardware Metadata
  2. Pro Micro shield DT naming changes
  3. Split changes for BLE advertising

Hardware Metadata

The Problem

When first developing the process around contributing new shields/boards to ZMK, we failed to recognize that several key files (setup scripts, documentation page of supported hardware, and GH Action build.yml file) required changes, often in the same spot, for every PR. This resulted in immediate merge conflicts for every other PR after one was merged, which is a headache for contributors.

The Fix

By adding discrete metadata files that are located with the boards/shields in question, and using that metadata to generate setup scripts, website hardware list, etc., users can contributing new hardware descriptions without the need to change the same files that other contributors are changing.

Next Steps

First, refer to https://zmk.dev/docs/development/hardware-metadata-files to familiarize yourself with the new metadata file format.

Next, you have two options for fixing up your PR:

  1. If comfortable with git rebase, perform an interactive rebase and revert any changes to build.yml, hardware.md, or the setup scripts setup.sh/setup.ps1, and then add the new metadata YAML file. Then force push your branch. Or,
  2. Create a new branch from an up-to-date main, copy in the files for your new hardware, add the metadata file, then commit and push the new branch. Then, edit your open PR to point to your new branch.

Pro Micro shield DT naming changes

In #876, we have simplified the DT naming for the "nexus node" we expose for pro-micro compatible boards, deprecating the use of pro_micro_a, and renaming pro_micro_d node to simply pro_micro. For pro-micro boards and shields, you'll need to adjust your DT to use the proper names.

Please see https://zmk.dev/docs/development/new-shield#shield-overlays for the updated docs on this.

Split Shield Advertising Changes

In addition, if this is a split PR, please see #658 where we have changed our conventions to remove the the name from the right sides, to prevent users attempting to pair with them and causing split sync issues. This also includes removing the " Left" suffix from the naming on the left side. See the changes in that PR for examples of what to change with your split shield.

Getting Help

If you have any questions about any of these changes, please comment here and tag @zmkfirmware/boards-shields or ask in the #boards-shields Discord channel.

@Nicell
Copy link
Member

Nicell commented Jan 30, 2022

Hello! We're closing this PR due to there being no activity since the metadata update to ZMK in September of last year. If you are still interested in updating this PR, please let us know, and we can reopen it and review your changes (hopefully a lot faster this time)!

@Nicell Nicell closed this Jan 30, 2022
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