mmone / OctoprintKlipperPlugin

A plugin for a better integration of Klipper into OctoPrint.
GNU General Public License v3.0
83 stars 62 forks source link

Klipper log file path is hardcoded #28

Open fpoussin opened 5 years ago

fpoussin commented 5 years ago

Octoprint 1.3.9 OctoKlipper 0.2.5

Currently the log file path is hardcoded: https://github.com/mmone/OctoprintKlipperPlugin/blob/769b8a53ed5fa38202aaaeaaa107b176749a8ff1/octoprint_klipper/__init__.py#L284

There is a need to be able to configure the path from the UI.

Thanks!

schnello commented 5 years ago

Same issue here. Klipper is running on a machine with Ubuntu. The installscript set the path for the log file to KLIPPER_LOG=/var/log/klippy.log.

schnello commented 5 years ago

I prepare an commit: image I hope this meets expectations as to how it should be implemented

schnello commented 5 years ago

@mmone Could you please review the commit. Please note: It works at my system but I'm not a python expert. https://github.com/schnello/OctoprintKlipperPlugin/commit/4445edbd5bdeba50b3f12a4d28b5fdd364c8fad0 https://github.com/schnello/OctoprintKlipperPlugin/commit/c459fe2ba5025b1401ca40e1e0b099159f31e6a2

codejedi365 commented 5 years ago

@schnello, at first glance your commit looks on track. I recommend you submit a pull request if you want a review of a fix action. It’s the GitHub way for crowdsourcing code improvements!

Also, unfortunately Mmone has been dormant on this repo for awhile now but if you submit the pull request to either @jameseleach, @codejedi365, or @dushyantahuja, we will likely incorporate your changes soon as we want to kept this plugin alive. Thanks for your help!

schnello commented 5 years ago

@codejedi365 Thank you for the information. Should i now create a pull request for this repro and add as cc @jameseleach or should i create a pull request to the fork?

jameseleach commented 5 years ago

The proper way is to create a fork, make the changes in your fork, and then submit a pull request. This will prompt the maintainer of the project (or branch) you forked from to integrate those changes.

The problem, currently, with forking from the main branch is that @mmone has been inactive and not reviewing and accepting pull requests generated against the main branch.

If, instead, you fork from my branch I’ll test the code and integrate it with my branch and submit a new pull request to he main branch. If it comes to pass that @mmone reviews my branch in the future and accepts the pull request all those changes, including yours, will become part of the main branch.

If not, people can at least use my branch with your enhancement, relatively easily.

More details here: Forking a repo: https://help.github.com/en/articles/fork-a-repo Creating a pull request: https://help.github.com/en/articles/creating-a-pull-request-from-a-fork

schnello commented 5 years ago

Thanks a lot. I will create a pull request to your fork