Ergo S-1 OE

[wizarddata]

Board/Shield Check-list

• [x ] This board/shield is tested working on real hardware
• [x ] Definitions follow the general style of other shields/boards upstream (Reference)
• [x ] .zmk.yml metadata file added
• [x ] 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)
• [x ] General consistent formatting of DeviceTree files
• [x ] Keymaps do not use deprecated key defines (Check using the upgrader tool)
• [x ] &pro_micro used in favor of &pro_micro_d/a if applicable
• [x ] If split, no name added for the right/peripheral half
• x[ ] Kconfig.defconfig file correctly wraps all configuration in conditional on the shield symbol
• [x ] .conf file has optional extra features commented out
• [x ] Keyboard/PCB is part of a shipped group buy or is generally available in stock to purchase (OSH/personal projects without general availability should create a zmk-config repo instead)

[wizarddata]

I've received some help, this PR should be in a much better state now.

Looking at it again, though, should this be a zmk-config repo instead? The case is available for purchase along with assembly instructions, but there isn't a custom PCB involved in this project. I'm not quite sure which makes the most sense.

[DEV-OGRE]

zmk-config repositories are created by the end user to compile their modified keymap configurations that are based around a core shield (like this one).

While the Ergo S-1 in a literal sense is 'handwired', i.e, each wire/diode is soldered individually to each switch, the pinout and switch layout is static every time, there is only one Ergo S-1. Unlike say a Dactyl, where the end user can customize the amount of rows, thumb clusters types, etc.

Being that the Ergo S-1 is a concave keywell (harder to produce flexible pcb) with pre-defined data pins, and most importantly, one static design, I think it makes sense to have it as a shield personally. The build guide has the user wiring the rows and columns identically each time, and the shield has been coded around that. There's no ambiguity or remapping GPIO pins to accommodate different 'versions' of the Ergo S-1, as there is only one version of it.

[DEV-OGRE]

zmk-config repositories are created by the end user to compile their modified keymap configurations that are based around a core shield (like this one).

In retrospect, that's technically not true, my bad, config's can also encompass entire boards and definitions, my main point still stands, the build guide and design still is centralized around one configuration, one wiring diagram, one design.

[caksoylar]

I reviewed it and left comments assuming this shield supports underglow, however I couldn't see anything related to LEDs in the project page. If it indeed isn't supported, you can remove the boards/nice_nano.overlay file and ignore my comments on the .conf and .yml files.

Also, you might want to update the copyright years.

[caksoylar]

I think this needs to be updated for pinctrl changes: https://zmk.dev/blog/2023/04/06/zephyr-3-2#move-to-pinctrl-driver. See another shield like https://github.com/zmkfirmware/zmk/blob/main/app/boards/shields/corne/boards/nice_nano.overlay as an example.

Also, you might want to duplicate this file (at least) for nice_nano_v2.overlay since that is commonly used these days and this file won't add support for v2.

[caksoylar]

You might want to add lines like below to let the user enable underglow:

# Uncomment the following lines to enable the RGB Underglow
# CONFIG_ZMK_RGB_UNDERGLOW=y
# CONFIG_WS2812_STRIP=y

[caksoylar]

Suggested change

	&bt BT_CLR &kp F1 &kp F2 &kp F3 &kp F4 &kp F5 &trans &kp KP_NUM&kp EQL &kp SLASH &kp STAR &trans
	&bt BT_CLR &kp F1 &kp F2 &kp F3 &kp F4 &kp F5 &trans &kp KP_NUM &kp EQUAL &kp SLASH &kp STAR &trans

Did this compile before with the missing space? Also EQL -> EQUAL since the former is deprecated.

[caksoylar]

Suggested change

	- keys
	- keys
	- underglow

[lesshonor]

Looking at it again, though, should this be a zmk-config repo instead?

I'm going to leave immediately-actionable suggestions in good faith and humor—but the answer to this question is prima facie "yes", and you should probably close this out.

Lack of any feedback for 14 months aside, the PR checklist effectively rules out end-user-assembled handwires unless you squint real hard...even if said end-user has to closely follow a specific set of instructions. Dactyls work the same way.

All that said; should you leave this PR open, perhaps squashing your commits and rewriting the message so it adheres to Conventional Commit guidelines will make the prospect of merging more attractive. It's a guess on my part, but it couldn't hurt.

[lesshonor]

Suggested change

label = "KSCAN";

This attribute is deprecated.

[lesshonor]

Migration to the pinctrl subsystem as part of the Zephyr 3.2 upgrade, and the &spi3 bus per #1746.

Suggested change

	&spi1 {
	compatible = "nordic,nrf-spim";
	status = "okay";
	mosi-pin = <6>;
	// Unused pins, needed for SPI definition, but not used by the ws2812 driver itself.
	sck-pin = <5>;
	miso-pin = <7>;
	&pinctrl {
	spi3_default: spi3_default {
	group1 {
	psels = <NRF_PSEL(SPIM_MOSI, 0, 6)>;
	};
	};
	spi3_sleep: spi3_sleep {
	group1 {
	psels = <NRF_PSEL(SPIM_MOSI, 0, 6)>;
	low-power-enable;
	};
	};
	};
	&spi3 {
	compatible = "nordic,nrf-spim";
	status = "okay";
	pinctrl-0 = <&spi3_default>;
	pinctrl-1 = <&spi3_sleep>;
	pinctrl-names = "default", "sleep";