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.

Sign up for GitHub

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

Jump to bottom

feat(boards) Kinesis Advantage 360 pro #1454

Merged
merged 18 commits into from
Nov 28, 2023
Merged

feat(boards) Kinesis Advantage 360 pro #1454

merged 18 commits into from
Nov 28, 2023

Conversation

ReFil
Copy link
Contributor

@ReFil ReFil commented Sep 13, 2022 •
edited

This PR adds support for the kinesis advantage 360 pro split, an nrf52840 based board
https://kinesis-ergo.com/keyboards/advantage360

I've tested this on real production hardware and everything works as expected, I'm not 100% sure about the metadata file though as I've never done one before

Board/Shield Check-list

  • This board/shield is tested working on real hardware
  • Definitions follow the general style of other shields/boards upstream (Reference)
  • .zmk.yml metadata file added
  • 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)
  • General consistent formatting of DeviceTree files
  • Keymaps do not use deprecated key defines (Check using the upgrader tool)
  • &pro_micro used in favor of &pro_micro_d/a if applicable (N/A)
  • If split, no name added for the right/peripheral half
  • Kconfig.defconfig file correctly wraps all configuration in conditional on the shield symbol
  • .conf file has optional extra features commented out
  • 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)

@ReFil
feat(boards) Advantage 360 pro
4e0262c
@ReFil
chore(lighting): fix deprecation
4d58241 
Removed deprecated macros, replace with correct Zephyr 3 implementation
@ypsilon-takai
Copy link

I've merged just this PR into main branch, have successfully built firmwares and install them into my adv360. But LED indicators are not lit.

I found some PRs which you issued on this PR list.
Is there any PRs which have to merge into source code to work LED indicators correctly?

@ReFil
Copy link
Contributor Author

ReFil commented Sep 20, 2022

LED indicators need a lot of custom RGB code to make them work, right now they're just set up as underglow defaulting to off. They won't work as intended for the foreseeable future. If you want the indicators to work properly you need to use the custom fork

@ccarter
Copy link

ccarter commented Sep 21, 2022

My setup that works with adv360-beta branch gives the following. I might just be doing something silly, I dunno. Any tips appreciated.

devicetree error: /__w/zmk-config-adv360/zmk-config-adv360/config/adv360.keymap:56 (column 47): parse error: expected number or parenthesized expression

@ReFil
Copy link
Contributor Author

ReFil commented Sep 21, 2022

My setup that works with adv360-beta branch gives the following. I might just be doing something silly, I dunno. Any tips appreciated.

devicetree error: /__w/zmk-config-adv360/zmk-config-adv360/config/adv360.keymap:56 (column 47): parse error: expected number or parenthesized expression

The adv360-beta branch has features that are not present in zmk main, including behaviours. At a guess check the last layer and replace the &rgb_ug RGB_MEFS_CMD 5 with &none

@ccarter
Copy link

ccarter commented Sep 21, 2022

My setup that works with adv360-beta branch gives the following. I might just be doing something silly, I dunno. Any tips appreciated.
devicetree error: /__w/zmk-config-adv360/zmk-config-adv360/config/adv360.keymap:56 (column 47): parse error: expected number or parenthesized expression

The adv360-beta branch has features that are not present in zmk main, including behaviours. At a guess check the last layer and replace the &rgb_ug RGB_MEFS_CMD 5 with &none

Ya, that got me further. Now I'm getting.
devicetree error: 'color-mapping' is marked as required in 'properties:' in /__w/zmk-config-adv360/zmk-config-adv360/zephyr/dts/bindings/led_strip/worldsemi,ws2812-spi.yaml, but does not appear in <Node /soc/spi@40004000/ws2812@0 in '/__w/zmk-config-adv360/zmk-config-adv360/zephyr/misc/empty_file.c'>

@ReFil
Copy link
Contributor Author

ReFil commented Sep 21, 2022

My setup that works with adv360-beta branch gives the following. I might just be doing something silly, I dunno. Any tips appreciated.
devicetree error: /__w/zmk-config-adv360/zmk-config-adv360/config/adv360.keymap:56 (column 47): parse error: expected number or parenthesized expression

The adv360-beta branch has features that are not present in zmk main, including behaviours. At a guess check the last layer and replace the &rgb_ug RGB_MEFS_CMD 5 with &none

Ya, that got me further. Now I'm getting.
devicetree error: 'color-mapping' is marked as required in 'properties:' in /__w/zmk-config-adv360/zmk-config-adv360/zephyr/dts/bindings/led_strip/worldsemi,ws2812-spi.yaml, but does not appear in <Node /soc/spi@40004000/ws2812@0 in '/__w/zmk-config-adv360/zmk-config-adv360/zephyr/misc/empty_file.c'>

You can't use a config branch for the adv360-beta branch with this. It simply won't work. Use this branch instead. The devicetree is all configured correctly

https://github.com/PolarityWorks/zmk-config-adv360/tree/zmk-main

@ccarter
Copy link

ccarter commented Sep 21, 2022

My setup that works with adv360-beta branch gives the following. I might just be doing something silly, I dunno. Any tips appreciated.
devicetree error: /__w/zmk-config-adv360/zmk-config-adv360/config/adv360.keymap:56 (column 47): parse error: expected number or parenthesized expression

The adv360-beta branch has features that are not present in zmk main, including behaviours. At a guess check the last layer and replace the &rgb_ug RGB_MEFS_CMD 5 with &none

Ya, that got me further. Now I'm getting.
devicetree error: 'color-mapping' is marked as required in 'properties:' in /__w/zmk-config-adv360/zmk-config-adv360/zephyr/dts/bindings/led_strip/worldsemi,ws2812-spi.yaml, but does not appear in <Node /soc/spi@40004000/ws2812@0 in '/__w/zmk-config-adv360/zmk-config-adv360/zephyr/misc/empty_file.c'>

You can't use a config branch for the adv360-beta branch with this. It simply won't work. Use this branch instead. The devicetree is all configured correctly

https://github.com/PolarityWorks/zmk-config-adv360/tree/zmk-main

very good. Thanks for your help. Good work BTW, and the keyboard is amazing IMO.

@ReFil
Copy link
Contributor Author

ReFil commented Sep 21, 2022

My setup that works with adv360-beta branch gives the following. I might just be doing something silly, I dunno. Any tips appreciated.
devicetree error: /__w/zmk-config-adv360/zmk-config-adv360/config/adv360.keymap:56 (column 47): parse error: expected number or parenthesized expression

The adv360-beta branch has features that are not present in zmk main, including behaviours. At a guess check the last layer and replace the &rgb_ug RGB_MEFS_CMD 5 with &none

Ya, that got me further. Now I'm getting.
devicetree error: 'color-mapping' is marked as required in 'properties:' in /__w/zmk-config-adv360/zmk-config-adv360/zephyr/dts/bindings/led_strip/worldsemi,ws2812-spi.yaml, but does not appear in <Node /soc/spi@40004000/ws2812@0 in '/__w/zmk-config-adv360/zmk-config-adv360/zephyr/misc/empty_file.c'>

You can't use a config branch for the adv360-beta branch with this. It simply won't work. Use this branch instead. The devicetree is all configured correctly

https://github.com/PolarityWorks/zmk-config-adv360/tree/zmk-main

very good. Thanks for your help. Good work BTW, and the keyboard is amazing IMO.

Thanks for the support. It's much appreciated 😊

@ReFil ReFil mentioned this pull request Feb 26, 2023
Open
@yanshay yanshay mentioned this pull request Mar 4, 2023
Closed
@yanshay
Copy link

yanshay commented Mar 5, 2023 •
edited

Isn't this PR missing from the left and right .defconfig files at least the following which are found in these files in https://github.com/KinesisCorporation/Adv360-Pro-ZMK ?

CONFIG_USB_DEVICE_VID=0x29EA
CONFIG_USB_DEVICE_PID=0x0362
CONFIG_USB_DEVICE_MANUFACTURER="Kinesis Corporation"
CONFIG_BT_DIS_MANUF="Kinesis Corporation"

@douglascamata
Copy link

Any chance of us seeing this merged?

@elisiano
Copy link

howdy!
I'm also stuck on an another branch until this gets merged.

Is this PR missing something to be able to be merged?

@ReFil
Copy link
Contributor Author

ReFil commented Apr 10, 2023

Since the zephyr 3.2 update I need to rebase and bugfix before this can be merged. I'll work on it asap

@elisiano
Copy link

Since the zephyr 3.2 update I need to rebase and bugfix before this can be merged. I'll work on it asap

I didn't realize that, thank you ❤️

@urob urob mentioned this pull request Apr 28, 2023
Open
@ReFil ReFil mentioned this pull request Nov 17, 2023
Closed
petejohanson
petejohanson requested changes Nov 20, 2023
View reviewed changes
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 minor things here and there. The formatting definitely should be cleaned up for consistency.

app/boards/arm/adv360pro/Kconfig.defconfig Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/Kconfig.defconfig Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.dtsi Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.dtsi Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.dtsi Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.dtsi Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro_right.dts Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro_right.dts Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/board.cmake Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.dtsi Show resolved Hide resolved
caksoylar
caksoylar reviewed Nov 20, 2023
View reviewed changes
Copy link
Contributor

@caksoylar caksoylar left a comment

Choose a reason for hiding this comment

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

Can you add a README to the board folder similar to the Glove80 one https://github.com/zmkfirmware/zmk/blob/main/app/boards/arm/glove80/readme.md ? I assume you will still be maintaining a fork with the extra features, like Glove80.

app/boards/arm/adv360pro/adv360pro_left.keymap Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro_right.keymap Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/Kconfig.defconfig Show resolved Hide resolved
app/boards/arm/adv360pro/Kconfig.defconfig Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/Kconfig.defconfig Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.dtsi Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.dtsi Outdated Show resolved Hide resolved
app/boards/arm/adv360pro/adv360pro.keymap Show resolved Hide resolved
@ReFil
Copy link
Contributor Author

ReFil commented Nov 21, 2023

Just retested it because quite a few things changed through the review process and everything seems to still be working

caksoylar
caksoylar approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@caksoylar caksoylar left a comment

Choose a reason for hiding this comment

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

Thanks, readme looks good!

petejohanson
petejohanson approved these changes Nov 28, 2023
View reviewed changes
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.

Thanks for sticking with this. LGTM!

@petejohanson petejohanson merged commit 69f7bfb into zmkfirmware:main Nov 28, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronjensen aaronjensen aaronjensen left review comments

@petejohanson petejohanson petejohanson approved these changes

@caksoylar caksoylar caksoylar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@ReFil @ypsilon-takai @ccarter @yanshay @douglascamata @elisiano @aaronjensen @petejohanson @caksoylar