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(docs) Adds a feature page about dongles #2401

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

RasmusKoit
Copy link

Contains a quick setup for using a dongle or adding a third split as central

Related to discussion started by user caksoylar under issue #948

@RasmusKoit RasmusKoit marked this pull request as ready for review August 3, 2024 17:05
@RasmusKoit RasmusKoit requested a review from a team as a code owner August 3, 2024 17:05
Copy link
Contributor

@Nick-Munnich Nick-Munnich left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! There has been some discussion on simplifying the dongle setup, but I think there's enough demand for them that it would be useful to document the current state of things.

I think it would be good to scrub all references to split keyboards from the page, and focus on dongles entirely. A separate page can handle the discussion on split keyboards, That would let this page be streamlined a bunch.

My current line of thought is to have this PR be included in a "Blueprints" category following the idea started in #1952, rather than under the features category. I plan on putting some time into that soon, and would like to "eat" this PR (still giving you author credits ofc) for that purpose when I get around to it. Your thoughts?

docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
@RasmusKoit
Copy link
Author

I think it would be good to scrub all references to split keyboards from the page, and focus on dongles entirely. A separate page can handle the discussion on split keyboards, That would let this page be streamlined a bunch.

My current line of thought is to have this PR be included in a "Blueprints" category following the idea started in #1952, rather than under the features category. I plan on putting some time into that soon, and would like to "eat" this PR (still giving you author credits ofc) for that purpose when I get around to it. Your thoughts?

I wanted to include some of the split information, because there could be builds that implement 3 splits or more (using a numpad as a third split comes to mind) so that is why I did not wish to box it in as much. If we rename and change this to just dongles, then yes, it does make sense to just streamline it.

I do not mind if this is moved under somewhere else nor care if I get any credit for writing. I am just trying to give back to the ZMK community for all the free help I am getting.

@RasmusKoit
Copy link
Author

I also think that now it makes sense to rename the file from splits to dongles, just want avoid any renaming until the doc review is completed

Copy link
Contributor

@Nick-Munnich Nick-Munnich left a comment

Choose a reason for hiding this comment

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

As an aside, I think you can rename the file as a part of this pass. It won't be an issue review-wise.

docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
docs/docs/features/splits.mdx Outdated Show resolved Hide resolved
@Nick-Munnich Nick-Munnich added the documentation Improvements or additions to documentation label Aug 4, 2024
@caksoylar
Copy link
Contributor

Hi @RasmusKoit, thank you very much for taking the initiative to tackle this. I think it's a good idea to have a guide such as this one to describe the why and how of "dongle" keyboards. However I think it is a bit awkward to have this without first having an explainer page on split keyboards in general. This would also save you from repeating any information in there.

My understanding from here/Discord is @Nick-Munnich had similar thoughts and this page right now is being geared more towards being a recipe/blueprint? If so, I think that's a good approach. This pushed me a bit to write a split keyboards feature page at #2406 so we have a good base to add on to.

Appreciate the contribution!

@RasmusKoit RasmusKoit changed the title feat(docs) Adds a feature page about splits and dongles feat(docs) Adds a feature page about dongles Aug 6, 2024
@RasmusKoit
Copy link
Author

@Nick-Munnich @caksoylar I've improved the document again and would like another review please

Copy link
Contributor

@Nick-Munnich Nick-Munnich left a comment

Choose a reason for hiding this comment

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

Another pass, I think we're getting somewhere that's going to look great. I didn't mention it explicitly, but I'd like it if you could change all the "Board" to "Keyboard" - avoiding confusion as ZMK uses "Board" as a different terminology.

docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
@Nick-Munnich
Copy link
Contributor

Also just going to add that this closes #2044.

RasmusKoit and others added 3 commits August 11, 2024 21:28
Co-authored-by: Nicolas Munnich <98408764+Nick-Munnich@users.noreply.github.com>
Co-authored-by: Nicolas Munnich <98408764+Nick-Munnich@users.noreply.github.com>
some restructure to cfg files
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved
@caksoylar
Copy link
Contributor

I think a note that this requires defining a custom shield and a pointer to new shield guide as a prerequisite is needed on this page.

Copy link

@englmaxi englmaxi left a comment

Choose a reason for hiding this comment

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

In my opinion, it should be mentioned somewhere that the number of available profiles will be reduced if CONFIG_BT_MAX_CONN and CONFIG_BT_MAX_PAIRED are not adjusted accordingly (ZMK_BLE_PROFILE_COUNT = CONFIG_BT_MAX_PAIRED - CONFIG_ZMK_SPLIT_BLE_CENTRAL_PERIPHERALS).

One small thing: The definitions in zephyr and zmk are not indented within the if/endif blocks. I would remain consistent with the new-shield guide where there are also no indents.

docs/docs/features/dongle.mdx Outdated Show resolved Hide resolved

```dts title="my_keyboard_dongle.overlay"
// import the default shield configuration
#include "my_keyboard.dtsi"

Choose a reason for hiding this comment

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

If the common shield configuration is imported and different boards are used for the dongle than for the shields, then this can lead to problems if board-specific aliases (e.g. &pro_micro, &xiao_d) are used.
I therefore had to delete unused nodes with these aliases (e.g. /delete-node/ &kscan0;), but perhaps there is a nicer solution.

RasmusKoit and others added 8 commits August 18, 2024 15:45
Co-authored-by: Nicolas Munnich <98408764+Nick-Munnich@users.noreply.github.com>
Co-authored-by: Nicolas Munnich <98408764+Nick-Munnich@users.noreply.github.com>
Co-authored-by: Nicolas Munnich <98408764+Nick-Munnich@users.noreply.github.com>
Co-authored-by: Nicolas Munnich <98408764+Nick-Munnich@users.noreply.github.com>
Co-authored-by: Nicolas Munnich <98408764+Nick-Munnich@users.noreply.github.com>
Co-authored-by: 5Y1VN <86794811+5y1vn@users.noreply.github.com>
Co-authored-by: Maximilian Engl <43675074+englmaxi@users.noreply.github.com>
Fixes pwr choice config
improves wording
Comment on lines +105 to +106
config ZMK_SPLIT_BLE_CENTRAL_PERIPHERALS
default 2

Choose a reason for hiding this comment

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

I was not able to overwrite the default like this. During my testing, it only worked inside the .conf file. Have you tested this successfully?

Copy link
Author

Choose a reason for hiding this comment

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

i have not as i also used a conf

https://github.com/RasmusKoit/pipar/blob/master/config/boards/shields/pipar/pipar_dongle.conf , at some point i shifted from real world examples to what is supposed to be more correct

@jeffsf
Copy link

jeffsf commented Aug 27, 2024

At least as I understand the functionality, displays on both the splits become "peripheral" and are unable to display, for example, active layer. If so, that seems like a significant disadvantage.

Edit: Thinking through this, it might be an advantage with, for example, a dongle at the bottom of a monitor that can display information in an arguably easier to glance at location than on the keyboard.

config ZMK_SPLIT
default y


Copy link

@jeffsf jeffsf Aug 27, 2024

Choose a reason for hiding this comment

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

In my opinion, this [power increase] is optional and unrelated to using a dongle. I agree it may be helpful and perhaps merits a text reference to documentation of BT_CTLR_TX_PWR_PLUS_8 such as https://zmk.dev/docs/troubleshooting/connection-issues

:::note
Depending on how the dongle is used, there are some additional [latency considerations](./split-keyboards.md#latency-considerations) to keep in mind.
On average, the latency increase is around 3.75ms, in worst case scenario, the latency increase can be up to 7.5ms compared to a unibody keyboard.
:::
Copy link

Choose a reason for hiding this comment

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

From a product liability perspective, I'd say "in some situations, ..." as poor Bluetooth connectivity can push latency even higher. Is it worth calling out one of the more common scenarios?

@jasper-clarke
Copy link

Hey All,
Could I ask if this guide is functionally working?
I'd like to set this up myself since I have 3 nice!nano's

@RasmusKoit
Copy link
Author

Hey All, Could I ask if this guide is functionally working? I'd like to set this up myself since I have 3 nice!nano's

Hey @jasper-at-windswept for a functionally working build you can check https://github.com/RasmusKoit/pipar or https://github.com/RasmusKoit/miryoku_zmk

@jasper-clarke
Copy link

@RasmusKoit
That pipar config is great thank you.
I am having an issue though, for some reason for me having columns set to 0 like yours in my dongle overlay is causing the build to fail.
Is there any change you could help me with this?
https://github.com/jasper-at-windswept/zmk-config-corone

@RasmusKoit
Copy link
Author

Please keep in mind that pipar uses Xiao ble and not a pro micro, you can reference both this doc and the repo to find the right combination though.

Please let me know if you get stuck again

@jasper-clarke
Copy link

Hey @RasmusKoit
I have managed to get it to build but for some reason no matter what I do, the left side of my board remains the central peripheral despite setting it to not be.
I can tell it is the central peripheral because on the nice!view it shows the controller-to-PC status indicator instead of the peripheral image display.
Your help would be greatly appreciated!
https://github.com/jasper-at-windswept/zmk-config-corone

@RasmusKoit
Copy link
Author

please flash each part with the reset firmware first!

then you should get it working

@jasper-clarke
Copy link

@RasmusKoit
I already have done that but it did not fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants