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

howto add a new feature from a remote zmk repo to kinessis #132

Closed
rtomaszewski opened this issue Apr 7, 2023 · 3 comments
Closed

howto add a new feature from a remote zmk repo to kinessis #132

rtomaszewski opened this issue Apr 7, 2023 · 3 comments

Comments

@rtomaszewski
Copy link

rtomaszewski commented Apr 7, 2023

I would like to add a missing feature for new behavior that is implemented here:

https://github.com/nickconway/zmk/tree/swapper-impl
zmkfirmware/zmk#997
zmkfirmware/zmk#1366

I have setup a local doker setup on my windows laptop. What is the best and easies way to add this feature and be able to compile it locally on my laptop?

Naively ive tried:
clone my fork Adv360-Pro-ZMK && cd Adv360-Pro-ZMK
git remote add otherrepo git@github.com:nickconway/zmk.git
git remote update
git merge swapper-impl --allow-unrelated-histories

The merge wants to create the "app" directory in my WSL which is not what the kinessis setup is. We are using docker and every time im build new firmware the compilation is happening inside the docker container.

Understanding this, do I have to build now a new container and try to run these github commands I provided or do I hack my way to login to the existing docker container I have and run these commands there.

Can somebody share some feedback, thanks.

@ReFil
Copy link
Collaborator

ReFil commented Apr 7, 2023

Hi, this is covered in the zmk documentation here: https://zmk.dev/docs/features/beta-testing

I'd caution against just merging the adv360_z3 and swapper branches together, that's just begging to cause the mother of all merge conflicts. The adv360_z3 branch has custom code to make the most out of the adv360 pro

@rtomaszewski
Copy link
Author

rtomaszewski commented Apr 13, 2023

question about the west.yml file

Thanks for the info. Just had a quick look at the link and Im not quite sure how I should combine the new feature and the kinesis firmware together.
Maybe my understanding is wrong buy if Im going to modify the west.yml file and point there to the new "remote" it would ignore the kinesis repo. Am i correct?

#west.yml

manifest:
  remotes:
    - name: zmkfirmware
      url-base: https://github.com/zmkfirmware
    - name: refil
      url-base: https://github.com/refil
  projects:
    - name: zmk                              
      remote: refil             << changing this and pointing to https://github.com/nickconway/zmk/tree/smart-interrupt
      revision: adv360-z3
      import: app/west.yml
  self:
    path: config

Instead what I managed to do is:

  1. Run manually the container
docker run -it -v /home/rado/Adv360-Pro-ZMK/firmware:/app/firmware -v /home/rado/Adv360-Pro-ZMK/config:/app/config  --entrypoint bash zmk
  1. Install some tools
apt-get install less vim
  1. Extract from the repo https://github.com/nickconway/zmk/tree/swapper-impl the relevant files

Cloning and looking at the history for the swapper-impl branch is not really helpful but the pull request zmkfirmware/zmk#1366 has the tab of changed files that shows what is really needed.
It was a lot more understandable after reading this howto as well
https://zmk.dev/docs/development/new-behavior

Ive saved the relevant files in the "config" dir that we are going to copy to the docker image in next step below

  1. Manually copy the extracted files to the running docker container from (1)
 cp -v config/zmk,behavior-tri-state.yaml /app/zmk/app/dts/bindings/
 cp -v config/behavior_tri_state.c /app/zmk/app/src/behaviors/

Modify the file: /app/CMakeLists.txt and add this line:

target_sources_ifdef(CONFIG_ZMK_BEHAVIOR_TRI_STATE app PRIVATE src/behaviors/behavior_tri_state.c)

Modify this file: /app/Kconfig as well and add:

DT_COMPAT_ZMK_BEHAVIOR_TRI_STATE := zmk,behavior-tri-state

config ZMK_BEHAVIOR_TRI_STATE
        bool
        default $(dt_compat_enabled,$(DT_COMPAT_ZMK_BEHAVIOR_TRI_STATE))
  1. Inside the container shell move back the the /app and run the build to test if everything compiles as expected:
root@889a7dd8bd92:/app# ./build.sh    
...
  1. Create a new image from the running container so we can use it now instead the default one
rado@DESKTOP-4OCP27V:~/zmk$ docker ps
CONTAINER ID   IMAGE     COMMAND   CREATED      STATUS      PORTS     NAMES
0d2ceeec80fd   zmk       "bash"    4 days ago   Up 4 days             musing_nash

rado@DESKTOP-4OCP27V:~/zmk$ docker commit --change='CMD ["./build.sh"]' 0d2ceeec80fd zmk-rado:v1
sha256:9f07485dc95dc02beac205984f95d677131d5c240fd82b8e59d54a3d152eaafa

rado@DESKTOP-4OCP27V:~/zmk$ docker images
REPOSITORY   TAG       IMAGE ID       CREATED         SIZE
zmk-rado     v1        9f07485dc95d   6 seconds ago   3.68GB
zmk          latest    89de7353c090   6 weeks ago     3.48GB
  1. Modify the kinesis: Makefile and change the "zmk" image to zmk-rado:v1
# We want to commetn 1st line becaus we are going to use our pre-build image
# $(DOCKER) build --tag zmk --file Dockerfile .

  $(DOCKER) run --rm -it --name zmk \
    -v $(PWD)/firmware:/app/firmware$(SELINUX1) \
    -v $(PWD)/config:/app/config:ro$(SELINUX2) \
    -e TIMESTAMP=$(TIMESTAMP) \
    zmk-rado:v1
  1. Run a test build from you laptop
rado@DESKTOP-4OCP27V:~/Adv360-Pro-ZMK$ make

@ReFil
Copy link
Collaborator

ReFil commented Apr 16, 2023

You shouldn't need to change what docker image is used, if you push your changes to a new zmk git repo and change the west.yml to point to the new repo you should be able to build using the standard docker image but it's good to see you've got it sorted, i'll close as completed

@ReFil ReFil closed this as completed Apr 16, 2023
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

No branches or pull requests

2 participants