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

[Keyboard] Add Moondrop Dash75 #462

Merged
merged 2 commits into from
May 20, 2023
Merged

Conversation

silvinor
Copy link
Contributor

Description

Add the Moondrop Dash75 - an innovative 75% keyboard with integrated USB Hub and headphone DAC/Amp

Upstream

Types of Changes

  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)

Copy link
Contributor

@lesshonor lesshonor left a comment

Choose a reason for hiding this comment

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

So, the upstream version of moondrop/dash75/r1/config.h has #define SERIAL_NUMBER Dash 75.

...You know this will cause a problem with Vial, which is why it's not in this PR. But it is worth noting that this difference will cause conflicts whenever changes to that file are pulled from upstream, which someone (i.e. xyz) will need to continually monitor and resolve.

I'm not xyz, so maybe he doesn't mind -- but I don't think I'd accept any keymaps to the main repository that are guaranteed to require that kind of babysitting.

keyboards/moondrop/dash75/info.json Show resolved Hide resolved
@silvinor
Copy link
Contributor Author

silvinor commented May 18, 2023

So, the upstream version of moondrop/dash75/r1/config.h has #define SERIAL_NUMBER Dash 75.

One thing to consider on this keyboard is that it is targeted to the audiophile consumer - it is constructed with a embedded USB connected to a 4 port-hub, with two USB- Type-A receptacles exposed, then the next two ports have one connected to an embedded MOONRIVER 2 Dual CS43198 DAC, and the other to a rudimentary ATMega32U4 based keyboard.

The DAC identifies itself with an OEM PID/VID, and a SERIAL NUMBER descriptor as "Moonriver 2". The s/n on the QMK aligns this branding to the KB too. Like you said - this does not work with the Vial variant as the App fails to identify the KB as a Vial enabled KB, so was removed for the Vial variant of the code.

image
(Image extracted from alexotos' review https://youtu.be/6JPqyMEYuwI?t=533 )

@xyzz
Copy link
Contributor

xyzz commented May 20, 2023

Can you keep the "define" in moondrop/dash75/r1/config.h and then "undef" it in the vial config.h?

@silvinor
Copy link
Contributor Author

Can you keep the "define" in moondrop/dash75/r1/config.h and then "undef" it in the vial config.h?

Unfortunately not - the sequence of includes means that the SERIAL_NUMBER defined in vial code occurs before the inclusion of the kb or keymap config.h ... so the define hits an error from the onset.

Compiling: .build/obj_moondrop_dash75_r1/src/default_keyboard.c                                    In file included from <command-line>:
./keyboards/moondrop/dash75/r1/config.h:6: error: "SERIAL_NUMBER" redefined [-Werror]
 #define SERIAL_NUMBER "Dash 75"

I will change the code to use a #ifndef ... but that will still be different from QMK upstream anyway. (So deleting the def or wrapping it in a #ifndef will result in the same outcome.)

@xyzz xyzz merged commit be45071 into vial-kb:vial May 20, 2023
@silvinor silvinor deleted the moondrop-dash75-r1 branch May 21, 2023 00:30
lesshonor pushed a commit to lesshonor/vial-qmk that referenced this pull request May 21, 2023
* Add Moondrop Dash75

* Update config.h
lesshonor pushed a commit to lesshonor/vial-qmk that referenced this pull request Jun 5, 2023
* Add Moondrop Dash75

* Update config.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants