Stephan3 / dwc2-for-klipper

A translator between DWC2 and Klipper
GNU General Public License v3.0
160 stars 38 forks source link

DWC2 uses G-codes missing in Klipper #31

Closed antst closed 4 years ago

antst commented 5 years ago

DWC2 uses some RRF-specific G-codes (especially M120/121 for push/pop, which wraps all macros in case of RRF), which are missing in Klipper. Of course, there is always possibility to define bunch of empty/non-empty macros in Klipper. To stop DWC2 from complains, but I am unsure it is best course of action. Would it be better if DWC python code would somehow take care of it?

Stephan3 commented 5 years ago

as i never used rrf for a long period, i am not sure what you mean.can you give me an example?

antst commented 5 years ago

Try extrude some filament in dwc2-klipper, and you will see M130/M121 errors

Stephan3 commented 5 years ago

Yes i see but what is the ecxpected behavior?

antst commented 5 years ago

You pop-ups with errors about wrong g-codes? I’d say there should be none. dwc wraps up any embedded into it macro with m120/m121 which are rrf-specific. and I expect that at least those g-codes has to be filtered out by your python code.

Stephan3 commented 5 years ago

Again what is a valid usecase in ref for m120 and m121? What does it do? Try to explain in easy words. There might be possibilities in Klipper

Coffee0297 commented 5 years ago

M120: After this command endstops will be kept enabled when not homing M121: After this command endstops will be kept disabled when not homing that is the default for marlin.

i don't see a use in klipper for this as we don't print from sd. And the error comes from klipper and not dwc.

Stephan3 commented 5 years ago

We print from virtual sd. Anyways I just can make it ignoring m120/m121

antst commented 5 years ago

Again what is a valid usecase in ref for m120 and m121? What does it do? Try to explain in easy words. There might be possibilities in Klipper

It is not use-case. DWC2 JS code, as it comes from Duet, wraps all stuff in M120/M121 (Push the state of the RepRap machine onto a stack/Recover the last state pushed onto the stack), those are different from Marlin.

Therefore, if you heat up extruder, go to DWC2 webUI , dashboard and just extrude some amount of filament, or move hotted in any direction, and you will get popup about wrong G-codes.

image

antst commented 5 years ago

And it is not the same SAVE_GCODE_STATE/RESTORE_GCODE_STATE in Klipper, so there is no point to define them as aliases to those. M120/M121 store/restore those things in RRF:

Current feedrate, and Whether moves (and separately extrusion) are relative or absolute Current feedrate Extruder positions

Of course, it is always possible to define empty macro M120 and M121, in order to stop those annoying complains. But taking into account that this is generic thing, which affects everyone, I assume that it is better to be fixed on your side in python code which emulates RRF for DWC2. Maybe I am wrong.

FHeilmann commented 4 years ago

Coudl you elaborate which particular detail makes you say the two sets of gcodes are not equivalent?

Both can be used to save and restore things like feedrate, absolute/relative state as well as position of axes and extruder.

The point of using them here is to make sure that after the filament was extruded the mode of the extruder didn't change and the position of the extruder didn't change. This can easily be achieved with

SAVE_GCODE_STATE and RESTORE_GCODE_STATE

antst commented 4 years ago

M120/M121 does not store/restore positions, otherwise you would not wrap in store/restore macro which moves carriage, as result would be that carriage will get back to original position :)

Above I have list of what exactly M120/M121 store/restore. It stores only extruder position, not axes.

When it comes to extruder, you right, they both address the same use-case. But when it comes yes motion, they aim at different use-case.

FHeilmann commented 4 years ago

Well you can use SAVE/RESTORE_GCODE_STATE without giving a cr** about the position it stores. The important bit is that SAVE_GCODE_STATE does all the things M120/M121 does in this case, plus some extra stuff, which can be ignored.

So I consider simply aliasing the two here a valid solution to the problem at hand, wouldn't you agree?

antst commented 4 years ago

Nope. It can not be ignored ) What will he result if

SAVE_GCODE_STATE G92 G0 X10 RESTORE_GCODE_STATE

?

FHeilmann commented 4 years ago

what does that gcode snippet have to do with the button you're pushing in dwc2?! I'm not saying its a generic solution, I'm saying its a solution IN THIS CASE, where dwc tries to execute

M120
M83
G0 E5
M121

Here the klipper gcodes are very much applicable unless I'm missing something

antst commented 4 years ago

You miss. As RESTORE_GCODE_STATE will restore X,Y,Z coordinates. While M120/M121 will not. And DWC2 wraps motion of X,Y,Z in 120/121.

Aliasing SAVE/RESTORE was my first idea ) But then turned out that even empty macros are more suitable. But this is has to be solved on level of python code. Otherwise every potential user will have to create those macros.

Stephan3 commented 4 years ago

@antst Current master includes a save/restore solution. I tested few things and it does. Please close this once you are okay with that.

antst commented 4 years ago

I will test, but I don't see how this will be OK :) If you move X by 20mm wrapped with klipper's store/restore, you achieve not what you expect.

antst commented 4 years ago

Ah, this is something I missed completely! That there is MOVE=0 option for restore. Then it does exactly equivalent of RRF restore! Thanks

antst commented 4 years ago

But then I am lost. I checked, and M120/M121 are in the code since september. And I have DWC updated from the master last time about a week ago. And i still have M120/M121 warnings. Apparently, it looks like I am hitting some other issue.

Stephan3 commented 4 years ago

This here works now with latest klipper git. For now i printed 4 objects and everything works okay. So i am going to close this.