Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insert gcode at layer change #4353

Merged
merged 11 commits into from Mar 8, 2019
Merged

Conversation

wporter82
Copy link
Contributor

I needed the ability to manually send commands on layer change and couldn't find a solution so I made this one up to fill that need.

@Ghostkeeper
Copy link
Collaborator

As seen on Reddit!

Developers, see issue CURA-5713.

@Kriechi
Copy link
Contributor

Kriechi commented Sep 11, 2018

@wporter82 I made a similar script to inject some instructions for timelapse creation with the Duet & RepRapFirmware. I think there is a good overlap - so maybe we can merge our approaches together! Let me know what you think!

@Kriechi
Copy link
Contributor

Kriechi commented Sep 11, 2018

@wporter82 oh - I see you also added a timelapse script (intentional in this PR?).
It would be nice to make the head-park optional - for the people with stringing issues who don't want to move just because of the webcam.

@wporter82
Copy link
Contributor Author

@Kriechi I have added the option for parking the head to be optional as well as looked at what your scripts were doing for the Duet. I added the wait for moves to finish but for the picture taking command, the default I have set is to use the Marlin code M240 but can be changed by the user to something like what you have.

gcode_to_append = self.putValue(M = 400) + ";Wait for moves to finish\n"
if park_print_head:
gcode_to_append = self.putValue(G = 90) + ";Absolute positioning\n"
gcode_to_append += self.putValue(G = 1, F = feed_rate, X = x_park, Y = y_park) + ";Park print head\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

G90 vs. G91 is always tricky - I guess the users have to be aware at this point that using the park-print-head features switches to absolute positioning - and does not switch back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add another M400 to wait for the parking move to be finished?

@Kriechi
Copy link
Contributor

Kriechi commented Sep 12, 2018

This looks good! If it gets merged I will change https://github.com/Kriechi/DuetRRF-timelapse to make use of this script once it is released with the next Cura version.


gcode_to_append = self.putValue(M = 400) + ";Wait for moves to finish\n"
if park_print_head:
gcode_to_append = self.putValue(G = 90) + ";Absolute positioning\n"
Copy link
Contributor

@LipuFei LipuFei Sep 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line and the next in this if block contain TABs as indents.

Signed-off-by: Wayne Porter <wporter82@gmail.com>
Signed-off-by: Wayne Porter <wporter82@gmail.com>
Signed-off-by: Wayne Porter <wporter82@gmail.com>
Signed-off-by: Wayne Porter <wporter82@gmail.com>
Signed-off-by: Wayne Porter <wporter82@gmail.com>
@alekseisasin
Copy link
Contributor

@wporter82 , I have tried Before and After and all of them has the same output, did not see any difference.

@alekseisasin
Copy link
Contributor

@wporter82 , do you still want to add this script?

@dector
Copy link

dector commented Nov 18, 2018

Let me insert my 50 cents here. 😄

I think that searching for the beginning of the layer in this way isn't good enough for cases when another script has already modified some layer and added its own code before ;LAYER. For example, you can add this script into post-processing queue twice and second one will not work as expected.

Case "adding gcode after layer" doesn't work as expected as well. As @alekseisasin already said, in current implementation it's the same as adding value before layer (we'll have redundant gcode instruction before the first layer and no instruction after the last one).

Some time ago I made something similar (but for adding gcode only before layer though) here. I can open another PR if @wporter82 will be absent for some time.

@wporter82
Copy link
Contributor Author

I agree with @dector in that this can cause problems if considerations are not taken. I've considered writing a complete plugin just for time lapse instead of going this route of basically find/replace. If this appears to cause more possible problems than it helps solve, I would think it should be abandoned for a better solution.

@alekseisasin
Copy link
Contributor

Right, a solution which might introduce another issue is not a good scenario.
If will be no more effort done for this ticket then I suggest closing this issue.

Copy link
Contributor

@diegopradogesto diegopradogesto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggested changes.

"version": 2,
"settings":
{
"insert_loc":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not abbreviate the setting name:

Suggested change
"insert_loc":
"insert_location":


if in_layer:
index = data.index(layer)
if self.getSettingValueByKey("insert_loc") == "before":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if self.getSettingValueByKey("insert_loc") == "before":
if self.getSettingValueByKey("insert_location") == "before":

else:
in_layer = False

if in_layer:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: This can be simplified by using if ";LAYER:" in lines[0]: and remove the 4 previous layers.

"version": 2,
"settings":
{
"trigger_cmd":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not abbreviate setting name:

Suggested change
"trigger_cmd":
"trigger_command":

park_print_head = self.getSettingValueByKey("park_print_head")
x_park = self.getSettingValueByKey("head_park_x")
y_park = self.getSettingValueByKey("head_park_y")
trigger_cmd = self.getSettingValueByKey("trigger_cmd")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
trigger_cmd = self.getSettingValueByKey("trigger_cmd")
trigger_command = self.getSettingValueByKey("trigger_command")

if park_print_head:
gcode_to_append += self.putValue(G = 1, F = feed_rate, X = x_park, Y = y_park) + ";Park print head\n"
gcode_to_append += self.putValue(M = 400) + ";Wait for moves to finish\n"
gcode_to_append += trigger_cmd + ";Snap Photo\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
gcode_to_append += trigger_cmd + ";Snap Photo\n"
gcode_to_append += trigger_command + ";Snap Photo\n"

else:
in_layer = False

if in_layer:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: This can be simplified by using if ";LAYER:" in lines[0]: and remove the 4 previous layers.

@diegopradogesto
Copy link
Contributor

@wporter82 are you handling the suggested changes? No rush, only to know if we need to keep this in mind or not.

@diegopradogesto diegopradogesto merged commit 821a4de into Ultimaker:master Mar 8, 2019
@diegopradogesto
Copy link
Contributor

I did some changes here 9396628 and merged it since it was taking a lot of lead time.

@Ghostkeeper
Copy link
Collaborator

Ghostkeeper commented Mar 20, 2019

We might want to add the option to retract during the move towards the park location as well at some point...

@Ellecross Ellecross mentioned this pull request Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants