Helix (4 row) #585

maxdubrinsky · 2021-01-06T01:19:27Z

I've had this branch kicking around for several months, but now that there's a shield definition for the 5 row I figured now was as good a time as any.

Adds support for the 4-row Helix v2
Fixes a couple bugs with the setup script

If there's anything please let me know, I may have missed something through all of my rebasing/merging.

Nicell

Thanks for working on this. I believe the helix_4.conf file should have CONFIG_ ZMK_RGB_UNDERGLOW=y but commented out.

Also could you add this shield to the following files?

docs/docs/hardware.md: You should add the Helix 4 Row to this list

docs/static/setup.ps1 and docs/static/setup.sh: Helix 4 Row should be added as a split option for both files.

.github/workflows/build.yml

app/boards/shields/helix_4/Kconfig.defconfig

Nicell · 2021-01-06T21:11:01Z

app/boards/shields/helix_4/Kconfig.defconfig

+config DEVICE_POWER_MANAGEMENT
+	default y


This is already enabled in the main Kconfig I think? Should be able to remove it.

I wasn't sure. When I checked the full config output it was not set, and app/Kconfig has it under ZMK_SLEEP. Is that the one that should be default y?

Nicell · 2021-01-06T21:11:22Z

app/boards/shields/helix_4/Kconfig.defconfig

+config WS2812_STRIP
+	default y


This is neat. I like it.

app/boards/shields/helix_4/helix_4.dtsi

app/boards/shields/helix_4/helix_4_left.overlay

app/boards/shields/helix_4/helix_4_right.overlay

Nicell · 2021-01-06T21:17:27Z

docs/static/setup.sh

+    21 ) shield_title="Helix (5)" shield="helix"; split="y"; break;;
+    22 ) shield_title="Helix (4)" shield="helix_4"; split="y"; break;;


I'm wondering if this should be listed as "Helix" and "Helix 4 Row"? It seems to me that this variant is much less popular?

I'm not sure about which is more used, I only know one other person who uses one and he has both so 🤷‍♂️

Either way, how would you feel about merging this into one helix with multiple kscans and transforms? I'm not super familiar with devicetree syntax, so I'm not sure what config values would get set to differentiate between the 4 and 5 row variants of this board from a zmk-config.

You're right here. It's probably better to have one helix with multiple transforms. I'm thinking just an additional transform might be enough. The Kyria shield has an example of this with the default transform and 5 column transform. Could you look into this?

maxdubrinsky · 2021-01-07T00:21:00Z

Thanks for working on this. I believe the helix_4.conf file should have CONFIG_ ZMK_RGB_UNDERGLOW=y but commented out.

Also could you add this shield to the following files?

docs/docs/hardware.md: You should add the Helix 4 Row to this list

docs/static/setup.ps1 and docs/static/setup.sh: Helix 4 Row should be added as a split option for both files.

Definitely, the 4 and 5 row variants are kind of served from the same github repo, but I could specify both 4 and 5 row support. I will update the powershell script, is there something else that needs updating in the bash script?

I've noticed that those main conf files don't actually get included in the build process for splits, I'm assuming that's a bug?

that scene in phantom menace when they make it legal try some led stuff steal some functionality revert some stuff revert keymap; revert display changes add scripts and stuff remove less-standard keybind remove rgb since its brokenish sync with new keymap fix some config maybe this fixes rgb fix config move to 4 row folder

maxdubrinsky · 2021-01-20T04:46:17Z

Sorry for going silent on this, haven't been able to focus on keyboard firmware the past few weeks.

In testing I've found that simply treating my 4-row as a 5-row with a different keymap worked fine. The only difference is an additional row in the keymap transform and another gpio pin enabled in the kscan, not sure if that warrants another stack of config.

Up to you, but I'd be happy to close this/turn this change into a docs update to indicate my solution.

Nicell · 2021-01-29T20:38:00Z

@maxdubrinsky I'm thinking it might be worth it. All it should require is another keymap transform and another DTS file that changes the chosen matrix transform I believe.

petejohanson · 2021-09-30T03:51:18Z

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!

Hardware Metadata
Pro Micro shield DT naming changes
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:

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,
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 · 2022-01-30T20:59:43Z

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)!

innovaker added enhancement New feature or request shields PRs and issues related to shields labels Jan 6, 2021

innovaker requested a review from Nicell January 6, 2021 10:27

Nicell requested changes Jan 6, 2021

View reviewed changes

maxdubrinsky added 5 commits January 19, 2021 23:29

fix setup script

6caf956

remove configs

8a25434

fix some stuff

0150764

make requested changes

ea3e47c

maxdubrinsky force-pushed the helix branch from 48fdb31 to ea3e47c Compare January 20, 2021 04:30

Nicell closed this Jan 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Helix (4 row) #585

Helix (4 row) #585

maxdubrinsky commented Jan 6, 2021

Nicell left a comment

Nicell Jan 6, 2021

maxdubrinsky Jan 6, 2021

Nicell Jan 6, 2021

Nicell Jan 6, 2021

maxdubrinsky Jan 7, 2021

Nicell Jan 7, 2021 •

edited

Loading

maxdubrinsky commented Jan 7, 2021

maxdubrinsky commented Jan 20, 2021

Nicell commented Jan 29, 2021

petejohanson commented Sep 30, 2021

Nicell commented Jan 30, 2022

		21 ) shield_title="Helix (5)" shield="helix"; split="y"; break;;
		22 ) shield_title="Helix (4)" shield="helix_4"; split="y"; break;;

Helix (4 row) #585

Helix (4 row) #585

Conversation

maxdubrinsky commented Jan 6, 2021

Nicell left a comment

Choose a reason for hiding this comment

Nicell Jan 6, 2021

Choose a reason for hiding this comment

maxdubrinsky Jan 6, 2021

Choose a reason for hiding this comment

Nicell Jan 6, 2021

Choose a reason for hiding this comment

Nicell Jan 6, 2021

Choose a reason for hiding this comment

maxdubrinsky Jan 7, 2021

Choose a reason for hiding this comment

Nicell Jan 7, 2021 • edited Loading

Choose a reason for hiding this comment

maxdubrinsky commented Jan 7, 2021

maxdubrinsky commented Jan 20, 2021

Nicell commented Jan 29, 2021

petejohanson commented Sep 30, 2021

Hardware Metadata

The Problem

The Fix

Next Steps

Pro Micro shield DT naming changes

Split Shield Advertising Changes

Getting Help

Nicell commented Jan 30, 2022

Nicell Jan 7, 2021 •

edited

Loading