klippain
klippain copied to clipboard
Add check to prevent install script from interrupting a print on localhost
This uses the Moonraker API on localhost to see if a print is currently running.
The logic is as follows:
- It's already confirmed that the Klipper service is installed (going on faith that Moonraker is too)
- If the Moonraker API responds to a
server/info
request with a200
HTTP status, we can assume it can answer aprint_stats
request too - Demand that the status be '
standby
'
If the Klipper service isn't running, the API checks will fail, but I'm allowing that to happen silently, because a print can't be running anyway.
Yes, I lost a print because I was dumb.
in which use case is this relevant? If a print is running, moonraker updates are disabled. if you install it, you get asked what happens, and a user should not install something while printing (imho). Same for ERCF, which also restarts klipper service without "warning". Just want to understand :)
I had a bad experience installing klippain. After reading the documentation, I believed my configuration files would be moved to a different directory and new configuration files would replace them. I expected that I would initiate a 'firmware restart' to cause the new config to be read the first time, not that the klipper service would be manipulated by the script. I admit complete fault on my part here. I'm the idiot who piped curl to bash without reading it line-by-line first.
I wanted to save the next person from my mistake.
This could be addressed in documentation ("hey, this is going to restart klipper no matter what!"), but my proposed solution helps people who read the docs and people who don't.
Thanks for the PR.
This looks pretty good for me. I just need to think a little bit at what could happens if the moonraker instance is in another computer. Even if this case should be not standard at all, it can potentially block Klippain install, so I would prefer to detect it and if it's the case, continue the installation with a warning.
My personal opinion on this: It is too complex for a single point of time check where it would be ONLY needed if someone does install of a software that does a config "reset" while printing. Adding an additional note to the dialog asking that this installs klippain should be sufficient, without impacting the complexity. Independant of the localhost topic Frix wrote about, it could also get changes within the codeblock that makes it no longer work. This needs to cover all possibilities of set up so it does not result in a permanent fail for all klippain installs, as an error in that case would break the whole klippain install for all users. Relation of usage to risk does not fit here.
Ok, so I added another warning for now in the install script: 2514b894b7f881bc60c6d5c3d777ac37cab57811 I'm going to put this PR on hold and see how we can merge this in a smart way later. This warning should be enough for now.