Re-write of Time Lapse

[GregValiant]

An update to the Time Lapse script.

• Added "insertion frequency" so the user can decide how often to take an image.
• Added support for relative extrusion.
• Retract is now a boolean and the settings come from Cura. There won't be a retraction if there is already a retraction.
• The 6th commit adds the "Pause before image" as proposed in 19556.

Description

Type of change

• [ X] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Test Configuration:

• Operating System: Windows 10 Pro (Intel processor)
• Cura 4.13.1 and Cura 5.7.0

Checklist:

• [ X] My code follows the style guidelines of this project as described in UltiMaker Meta and Cura QML best practices
• [ X] I have read the Contribution guide
• [ X] I have commented my code, particularly in hard-to-understand areas
• [ X] I have uploaded any files required to test this change

[casperlamboo]

hi @GregValiant, I'm not on the cura team anymore. best to request a review of one of the other cura devs. Thanks

[GregValiant]

@casperlamboo thanks for getting back.

I hope all goes well for you in the next phase.

Greg

[jellespijker]

@wawanbreton could you take look at this next power hours, since @saumyaj3 is currently on holiday

[GregValiant]

@wawanbreton The last commit adds support for firmware retraction.

[GregValiant]

@wawanbreton Has there been any progress on this?

[wawanbreton]

Hi @GregValiant, sorry for not answering, I'm just back from holiday. I will create tickets to integrate this PR into our backlog for review, and also the one for the other plugin.
However, please remember that the Cura team currently has very limited resources (team split up with other projects, and holiday) so this may take some time 😬

[GregValiant]

I understand completely and was not trying to jostle your elbow. I knew 5.8.0 was due to be released and I was just curious if either would get in under the deadline.

[wawanbreton]

No offense taken... It's actually a good thing that you regularly raise some flags, so that we don't forget we have PRs to look at 😃

[GregValiant]

I have added the setting from #19556 but I have yet to update this PR as I am waiting to hear back from the other poster.

I'm getting "merge conflicts" so I'm sitting on the revised version.

[GregValiant]

The 6th commit appears to be correct and includes the change proposed in 19556.

[wawanbreton]

Looks very good, there is just something curious about the extra spaces after units 🤔

[wawanbreton]

We agreed with the team that those extra spaces should be removed, should it be at least for consistency. I spotted no visual issue related to this.

[Slashee-the-Cow]

Question: do potential bugs caused by bad behaving other post-processing scripts count as problems, especially when there's an easy fix?
Line 173-175 (and 181-182, code reuse smells):

if " E" in line:
    last_e = line.split("E")[1]
    if float(last_e) < float(prev_e):

That assumes that E is the last parameter. In code produced by CuraEngine and Script.putValue() it will be, but if there's a script out there running before this which is crafting its own lines it might not be so polite. If "E" isn't the last parameter then the second item of the split() will be a string which will throw a ValueError when you try and cast it to a float.

The script does catch exceptions, but in doing so it'll stop processing that entire layer, including if there's supposed to be a shot at the end. What's wrong with this? We already know there's an E value, we checked for that.

last_e = float(self.getValue("E"))

[GregValiant]

@wawanbreton,
The extra spaces after units are needed when the post processor has a lot of settings and the vertical scroll bar is present. The spaces push the text to the left so the scrollbar doesn't occlude the text. I got in the habit of adding them "just in case". Feel free to box them up and send them back to me.
(NOTE: Please be advised that I am philosophically opposed to paying for shipping "spaces" from the Netherlands to the US so please don't make them "cash on receipt" or I will refuse to accept them. We don't want to create an international incident.)

@Slashee-the-Cow I saw that but something else caught my eye and I forgot about it.

[wawanbreton]

The spaces push the text to the left so the scrollbar doesn't occlude the text.

Ok, good to know 🙂 if at some point we have to change the UI of the post-processing script, then we shall fix that !

[Slashee-the-Cow]

@wawanbreton So if I want to put my code where my mouth is and try and push request in a refactored version that I, in my completely biased opinion, think is better:

1. I'm assuming I'm at least a couple of weeks too late?
2. How would I do it anyway? Never actually used GitHub for anything but my own single person projects.

[wawanbreton]

1. I'm assuming I'm at least a couple of weeks too late?

For 5.9, yes. But you can still start working on this now and it can be integrated in next release.

2. How would I do it anyway? Never actually used GitHub for anything but my own single person projects.

Just make a fork of this Cura repository, then do whatevere you want on it, and when you are satisfied with your work, create a PR back to this repository