mkuf / prind

print in docker - Deploy a containerized Klipper Stack for your 3D Printer
GNU General Public License v3.0
324 stars 82 forks source link

Add kipper_mcu service #72

Closed xBiohazardx closed 1 year ago

xBiohazardx commented 1 year ago

I would like to request the implementation of a new feature in Prind that allows Klipper to directly control the GPIOs and buses of the RPi where it is installed as a secondary MCU. According to this documentation from klipper: https://www.klipper3d.org/RPi_microcontroller.html

mkuf commented 1 year ago

Thanks for your Request.

From my understanding this would require two new components:

The mentioned service needs to be set up in a way so that the process can access the gpio resources from within the container.
A quick google search revealed three possible ways to achieve this: https://stackoverflow.com/a/48234752
The correct way to make it work has to be researched, maybe by analyzing the klipper code or by simple trial and error starting with the least permissions granted to the container.

I currently do not own a Raspberry Pi where I could test the implementation, this is also why this Note is part of the input shaper calibration docs. https://github.com/mkuf/prind/blob/21fd23fe978c6a1627c376a25e042d590e659d93/README.md?plain=1#L158-L165

If you and/or others are willing to assist in testing I could try implement this.

-Markus

xBiohazardx commented 1 year ago

Thank you for your fast reply

I only know docker basics so I could not implement it myself, or else I would have opened a pull request. But I can help and test for this implementation as needed. I agree on the implementation steps you mentioned. Getting the mcu service running would be a good first step.

mkuf commented 1 year ago

So, I had some time at hand to try this.
I just pushed a new branch, that should in theory make it possible to run the klipper_mcu process within docker.

  1. Check out the code of the feature branch https://github.com/mkuf/prind/tree/72-mcu-process
  2. add the following lines to your printer.cfg
    [mcu localhost]
    serial: /opt/printer_data/run/klipper_mcu.tty
  3. Start the stack
    docker compose --profile mainsail -f docker-compose.yaml -f docker-compose.extra.mcu-process.yaml -f docker-compose.override.yaml up -d

Starting the stack will take a long time, as the image for the mcu process has to be built from scratch.
Once the functionality is verified, this can be optimized and a klipper_mcu image can be provided by the buildsystem.

Looking forward to your feedback.

-Markus

xBiohazardx commented 1 year ago

Thank you for this fast implementation. I now had some time to test this feature.

For it to work, I had to add the gpio group to the Dockerfile and add the klipper user to it. Without this the mcu process did not have access to the gpios. (I can open a pull request with this change if you want to, but I am not sure if this will work on a system other than raspbian...)

With this change I was able to use the gpios of the Raspberry. I sadly could not test I2C functionality because I am still waiting on some hardware for that.

mkuf commented 1 year ago

You can just share the Code here and I can add it to the feature branch. :)
Did you have to match any group ids? As far as I can see, there is no gpio group in the klipper images and creating one does not guarantee that it matches the hosts gpio group.

On the other hand, we might just want to run the mcu process as root, as the official systemd service provided by klipper does the same: https://github.com/Klipper3d/klipper/blob/master/scripts/klipper-mcu.service

mkuf commented 1 year ago

I just updated the Dockerfile to use root instead of the unprivileged klipper user.
Would you try again? @xBiohazardx

You'll have to update your repo and run a build before bringing up the stack again.

docker compose --profile mainsail -f docker-compose.yaml -f docker-compose.extra.mcu-process.yaml -f docker-compose.override.yaml build

-Markus

xBiohazardx commented 1 year ago

Yes I did match the group id with the one from the host system. The problem with this running as root is, that the created serial device now also needs root access:

lrwxrwxrwx 1 root root 10 Jun  2 07:47 printer_data/run/klipper_mcu.tty -> /dev/pts/1

And clippy now can't access it because of these permissions.

mkuf commented 1 year ago

Thanks for the feedback.
I'm not sure why access is denied, as the klipper user is assigned the tty group, which has access to /dev/pty/1.
The numeric group ids between the klipper and the mcu image match, so in theory the klipper user should be able to access the device.

root@984734cb906d:/opt# ls -al printer_data/run/
total 8
drwxr-xr-x 2 root root 4096 Jun  2 19:33 .
drwxr-xr-x 3 root root 4096 Jun  2 19:33 ..
lrwxrwxrwx 1 root root   10 Jun  2 19:33 klipper_mcu.tty -> /dev/pts/1
root@984734cb906d:/opt# ls -al /dev/pts/1
crw-rw---- 1 root tty 136, 1 Jun  2 19:33 /dev/pts/1
root@984734cb906d:/opt#

I'm occupied for the next few days but I'll investigate this further sometime next week.

Other thoughts:
If we can't get it to work as root, it might be possible to automate the group creation via an entrypoint script.
The numeric group id of the hosts gpio group can then be set via an environment variable. By default it should match the ids used by raspbian. If it does not match a users host system, the variables may be manually set by the user via the docker-compose.overwrite.yaml

xBiohazardx commented 1 year ago

I am not sure, but it does seem like that the klipper user is not in the tty group:

root@49f6f0288a63:/opt# ls -la /opt/printer_data/run/
total 4
drwxrwxrwt 2 root    root     100 Jun  6 07:03 .
drwxr-xr-x 6 klipper klipper 4096 May 30 02:35 ..
srwxr-xr-x 1 klipper klipper    0 Jun  6 07:03 klipper.sock
lrwxrwxrwx 1 klipper klipper   10 Jun  6 07:03 klipper.tty -> /dev/pts/1
lrwxrwxrwx 1 root    root      10 Jun  6 07:03 klipper_mcu.tty -> /dev/pts/0
root@49f6f0288a63:/opt# ls -la /dev/pts/0
crw-rw---- 1 root tty 136, 0 Jun  6 07:03 /dev/pts/0
root@49f6f0288a63:/opt# ls -la /dev/pts/1
crw-rw---- 1 klipper tty 136, 1 Jun  6 07:03 /dev/pts/1
root@49f6f0288a63:/opt# groups klipper
klipper : klipper dialout
mkuf commented 1 year ago

You're right, I mixed that up with the dialout group.
I just added klipper user to the tty group in the feature branch https://github.com/mkuf/prind/commit/4870bbc13efe9c956ff3c72acec13513ea47047a

As this is not yet in the main branch, I added a the build instruction for the klipper image itself to docker-compose.extra.mcu-process.yaml, so you'll have to run the built command again prior to testing.

xBiohazardx commented 1 year ago

Yeah now it seems to be working. As mentioned before, I need to wait on some hardware in order to be able to test i2c communication. Do you want to wait for this in order to merge? Honestly I don't see a reason why i2c or spi shouldn't be working.

mkuf commented 1 year ago

Thats great to hear.
I'm with you in that this can be added to main without verifying i2c and spi.

I'll have to do some refactoring of the dockerfile and find a solution to build a small runtime image for the mcu_process that can be pushed to the registry. When this is done I'd ask you to test again, then it can be merged to main.

Thanks for your efforts in testing this so far :)

mkuf commented 1 year ago

Progress is documented in the PR: https://github.com/mkuf/prind/pull/79

mkuf commented 1 year ago

The codebase is now in a state that it can be tested and images have been uploaded to dockerhub.

The serial device for the klipper_mcu process has changed, so you'll probably need to update that in your config: https://github.com/mkuf/prind/blob/656f65e1d777dbaa82eda874060ac12d540c6d0f/config/printer.cfg#L10

Other than that, stop your stack and start it again with the hostmcu profile e.g.

docker compose --profile mainsail --profile hostmcu up -d

Depending on when you get to testing, the latest tag for klipper might have already been updated. So it might be necessary to use mkuf/klipper:645a1b8 for the klipper service instead of latest, as this is the only image where the klipper user is part of the tty group.

You can set the image like this in your override file:

services:
  klipper:
    image: mkuf/klipper:645a1b8
xBiohazardx commented 1 year ago

Okay I finally could test this and can confirm that it is working. Thank you for this fast implementation.

mkuf commented 1 year ago

Thank you for your contribution.
I just added the required docs and merged the PR.

I'll close this, once the new Images are build and the new release is tagged.

mkuf commented 1 year ago

Progress may be observed here: https://github.com/mkuf/prind/actions/runs/5297761833/jobs/9589727601

mkuf commented 1 year ago

The Image build finished and v1.7.0 has been released.