Klipper gcode.py refactor fixes

pluuuk commented 4 years ago

List of Changes:

fixes #73 by renaming try_load_module to load_object.
refactors the gcode command definitions to be compatible with the new klippy/gcode.py GCodeCommand handling format
Moves the hook to the custom gcode from rr_gcode to gcode_reactor_callback, to simplify handling logic.
Refactors the SDcard custom gcode logic to tie in better with the existing virtual_sdcard functionality in klipper, this includes overriding the sdcard.get_file_list command to list the nested ./gcode/* files that dwc2 uses.
Removes the need to modify gcode.py for the response callbacks by overriding the default instance method with one that calls gcode_response - this should simplify installation

It seems to be working well with my printer, but it wouldn't hurt to do some additional testing.

pluuuk commented 4 years ago

Closing this for now while I port the command definitions to the new paradigm

pluuuk commented 4 years ago

I have done some light testing on this and is compatible with the latest version of klipper.

Szeker commented 4 years ago

@pluuuk Do you also have the callback patch for the latest gcode.py available? (I guess it needs to be updated also due to the changes in Klipper on gcode.py. )

Or is the patch necessary at all with the latest Klipper version?

pluuuk commented 4 years ago

I installed the patch as usual and it's working okay. I haven't investigated whether it's still needed. I can appreciate if some testing can be done independently to the custom gcodes, what I have tested so far seems to work well.

manu7irl commented 4 years ago

Can you make the babysteps working again command set_gcode_offset z_adjust={variable} move=1 The M290 command is not working.

On Mon, May 11, 2020, 12:07 PM pluuuk notifications@github.com wrote:

I installed the patch as usual and it's working okay. I haven't investigated whether it's still needed. I can appreciate if some testing can be done independently to the custom gcodes, what I have tested so far seems to work well.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Stephan3/dwc2-for-klipper/pull/75#issuecomment-626574582, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RGB45L55B7MTNILAPLRQ656JANCNFSM4M5NLL4A .

Szeker commented 4 years ago

@pluuuk Can you share your patched gcode.py? As it was changed quite a lot lately, I'm not sure where and how exactly to apply the necessary changes in the file.

Szeker commented 4 years ago

@pluuuk I made the changes manually in gcode.py, and it works, I can connect to the server. Printing / stability not yet tested.

manu7irl commented 4 years ago

Share your changes. It should be good to use a different method than sed to implement the ack into this file.... Maybe copying the entire file into the project then symlinking it inside klipper folder.

On Mon, May 11, 2020, 5:42 PM Szeker notifications@github.com wrote:

@pluuuk https://github.com/pluuuk I made the changes manually in gcode.py, and it works, I can connect to the server. Printing / stability not yet tested.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Stephan3/dwc2-for-klipper/pull/75#issuecomment-626745953, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RBN7MGA554OAJLSBS3RRAFEDANCNFSM4M5NLL4A .

Szeker commented 4 years ago

I don't know what is the best way to share it, but here is my patched gcode.py:

(In my Klipper repo:) https://github.com/Szeker/klipper/commit/2dfb5a0305f7a13cca37408b2468ebc12b3b1c8a

Anyhow, @BlackStump has a nice install script - including the gcode.py patch, just needs to be updated to the latest Klipper version.

https://github.com/BlackStump/dwc2-for-klipper-install

elliotf commented 4 years ago

I can confirm that this PR lets me use up-to-date klipper

pluuuk commented 4 years ago

@manu7irl , I think I've fixed the babystep command, but I have no way to test it, can you pull this latest version and test?

manu7irl commented 4 years ago

I don't know what is the best way to share it, but here is my patched gcode.py:

(In my Klipper repo:) Szeker/klipper@2dfb5a0

Anyhow, @BlackStump has a nice install script - including the gcode.py patch, just needs to be updated to the latest Klipper version.

https://github.com/BlackStump/dwc2-for-klipper-install

I made a new patch file for his script, and I added it to my own multisession script, at https://github.com/manu7irl/klipper-DWC2-installer and I have a working mirror at with all the changes needed it got also the dwc2_gcode_py.patch to patch it use my script https://github.com/manu7irl/dwc2-for-klipper

manu7irl commented 4 years ago

I will test it! I made a clone of your version in my own folder and I updated my multisession script with a .patch version of the gcode.py file. Available at: https://github.com/manu7irl/klipper-dwc2-installer

On Mon, May 11, 2020, 11:36 PM pluuuk notifications@github.com wrote:

@manu7irl https://github.com/manu7irl , I think I've fixed the babystep command, but I have no way to test it, can you pull this latest version and test?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Stephan3/dwc2-for-klipper/pull/75#issuecomment-626947311, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RFO6QECWTYJX4FBI4LRRBOTFANCNFSM4M5NLL4A .

manu7irl commented 4 years ago

I think there is a little typo here. It needs to be move=1 and not move1 As in here:


        gcmd2 = self.parse_params('SET_GCODE_OFFSET Z_ADJUST=' + str(mm_step) + ' MOVE=1')```

pluuuk commented 4 years ago

Good catch, just fixed it.

manu7irl commented 4 years ago

No problem. My python is crap, but I have a good hacker eye..

On Tue, May 12, 2020, 12:20 AM pluuuk notifications@github.com wrote:

Good catch, just fixed it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Stephan3/dwc2-for-klipper/pull/75#issuecomment-626971160, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RBK33SSUOZLTN2QCQTRRBTZ3ANCNFSM4M5NLL4A .

BlackStump commented 4 years ago

I have updated to @pluuuk changes, I have not tested if I get a chance I will today

pluuuk commented 4 years ago

I have added more changes to this PR, including a change in the code that removes the need to modify gcode.py, which should simplify installation. Installing is just a matter of simlinking the web_dwc2.py to klippy/extras, configuring the virtual sdcard and copying the dwc2 files over.

manu7irl commented 4 years ago

I have added more changes to this PR, including a change in the code that removes the need to modify gcode.py, which should simplify installation. Installing is just a matter of simlinking the web_dwc2.py to klippy/extras, configuring the virtual sdcard and copying the dwc2 files over.

OMG missed that one!... Removing the need to modifying gcode.py is great testing that today! And removing the patch lines form my script.

manu7irl commented 4 years ago

it is working for me, and my script is up-to-date

manu7irl commented 4 years ago

Can you make the babysteps working again command set_gcode_offset z_adjust={variable} move=1 The M290 command is not working. … On Mon, May 11, 2020, 12:07 PM pluuuk @.***> wrote: I installed the patch as usual and it's working okay. I haven't investigated whether it's still needed. I can appreciate if some testing can be done independently to the custom gcodes, what I have tested so far seems to work well. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#75 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RGB45L55B7MTNILAPLRQ656JANCNFSM4M5NLL4A .

pluuuk commented 4 years ago

Can you make the babysteps working again command set_gcode_offset zadjust={variable} move=1 The M290 command is not working. … On Mon, May 11, 2020, 12:07 PM pluuuk @_.***> wrote: I installed the patch as usual and it's working okay. I haven't investigated whether it's still needed. I can appreciate if some testing can be done independently to the custom gcodes, what I have tested so far seems to work well. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#75 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RGB45L55B7MTNILAPLRQ656JANCNFSM4M5NLL4A .

Babystepping is working now, I fixed it in an earlier commit.

Stephan3 commented 4 years ago

sorry guys. dont get me wrong but getting 47 postings with a patch of a patch and a fix of a fix will take me ages.

Stephan3 commented 4 years ago

https://github.com/pluuuk/dwc2-for-klipper/blob/b503aef371217d1fd6dc19f840a51180fc23dad0/web_dwc2.py#L1138

remove the = trust me

pluuuk commented 4 years ago

https://github.com/pluuuk/dwc2-for-klipper/blob/b503aef371217d1fd6dc19f840a51180fc23dad0/web_dwc2.py#L1138

remove the = trust me

~I'm not sure if the new parsing function (lifted directly from gcode.py) works without the equals sign. I originally didn't include it but some users reported issues with it.~ I tested it myself and it seems for some reason the equals signs are not needed but they're needed if issuing the same command from the gcode console, pretty odd.

manu7irl commented 4 years ago

I am on your last commit but still cannot get this to work... the 3 first babysteps commands where sent from the button. the last one is from the command line.

shaiss commented 4 years ago

Any updates on getting this PR merged to master? this is good stuff! thx @pluuuk @Stephan3 !

manu7irl commented 4 years ago

You can clone @pluuuk folder instaed it works wonders...

On Wed, May 20, 2020, 10:41 PM SHAI PEREDNIK notifications@github.com wrote:

Any updates on getting this PR merged to master? this is good stuff! thx @pluuuk https://github.com/pluuuk @Stephan3 https://github.com/Stephan3 !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Stephan3/dwc2-for-klipper/pull/75#issuecomment-631685107, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RDGAOGBYPYYKWHACZTRSQXAFANCNFSM4M5NLL4A .

shaiss commented 4 years ago

You can clone @pluuuk folder instaed it works wonders... … On Wed, May 20, 2020, 10:41 PM SHAI PEREDNIK @.***> wrote: Any updates on getting this PR merged to master? this is good stuff! thx @pluuuk https://github.com/pluuuk @Stephan3 https://github.com/Stephan3 ! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#75 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RDGAOGBYPYYKWHACZTRSQXAFANCNFSM4M5NLL4A .

I just did and can confirm it works. would be nice to see this fixes be merged to master.

manu7irl commented 4 years ago

Stephan is really busy... But maybe when he will have time...

On Wed, May 20, 2020, 10:52 PM SHAI PEREDNIK notifications@github.com wrote:

You can clone @pluuuk https://github.com/pluuuk folder instaed it works wonders... … <#m-449483917713812913> On Wed, May 20, 2020, 10:41 PM SHAI PEREDNIK @.***> wrote: Any updates on getting this PR merged to master? this is good stuff! thx @pluuuk https://github.com/pluuuk https://github.com/pluuuk @Stephan3 https://github.com/Stephan3 https://github.com/Stephan3 ! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#75 (comment) https://github.com/Stephan3/dwc2-for-klipper/pull/75#issuecomment-631685107>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RDGAOGBYPYYKWHACZTRSQXAFANCNFSM4M5NLL4A .

I just did and can confirm it works. would be nice to see this fixes be merged to master.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Stephan3/dwc2-for-klipper/pull/75#issuecomment-631690748, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWT4RGIL3L7VZE65XYCX4LRSQYHHANCNFSM4M5NLL4A .

witem commented 4 years ago

I tested it, and it works great for me. Thanks!

dimalo commented 4 years ago

Yeah same for me! thanks to @Stephan3 and @pluuuk. I'm running two printers simultaneously from my OdroidU3 from two docker containers. Compared to running the printers with klipper & octoprint each printer just need 25% or less CPU than before! In case anybody is interested, I have built and pushed my klipper-dwc2 docker image to dockerhub. I'll work more on the readme for the image. For now I host the source on a gitlab instance found here, which does the builds for me.

Stephan3 commented 4 years ago

i made a different approach fixing things. Thanks for your effort here @pluuuk

witem commented 4 years ago

@Stephan3 but your fixes don't include all changes from PR

Stephan3 commented 4 years ago

Yes @witem you are right. There are things not related to Klipper incompatible issues

witem commented 4 years ago

@Stephan3 you add other features from this PR to master? As example "not need to modify gcode.py" feature

Stephan3 commented 4 years ago

@witem I missed that one for sure. tbh i am a hobbyist. I do not have the time to work all through this. For now i took a look to kevins code and find a working solution fast. @pluuuk digged deeper to this, obviously

Stephan3 commented 4 years ago

@witem now i see how @pluuuk did that. noice idea :D i will pick that up

witem commented 4 years ago

@Stephan3 thanks! I understand that you hobbyist. If you need I can help you with support this repository.

Stephan3 / dwc2-for-klipper

Klipper gcode.py refactor fixes #75