Timelapse render option

[WisdomCode]

This Request wants to take care of the suprisingly old #1313 It offers a combo box for setting what is to be done with the timelapse after a print is finished. To not break previous setups, the default is to render always after a print. However, it is now also possible to render only successful prints after the print is finished, or to never render a print after finishing printing.

However, I am still not done with this request, because there is a bug I cannot really get my head around. Maybe someone a bit more acquainted can help me out here?

Currently, the render is successfully skipped, however the according print files are still kept as currently active, as if the recording did not stop. This must be due to the timelapse being saved as "current" (the variable in src/octoprint/timelapse.py), but I cannot find where this is happening and how to stop it. A restart of the server fixes it, as the "current" variable gets reinitialized then. I am also not really sure how to test it without printing, which is just a bit painful for debugging this efficiently. I would appreciate any help. Thank you for this great project @foosel !

[foosel]

@WisdomCode Just a quick heads-up, I've seen this and am happy to help, but it might take until next week.

[foosel]

So now that 1.10.0 is out and it also looks like it's a fairly solid release, I finally found the time to take a closer look at this :)

First of all, please make sure to run pre-commit install in your checkout, as that not having been done caused the build failure - your code changes did not pass the linter and formatter.

Also, this is an improvement of an existing feature, not a huge change, so I've rebased your changes to target the maintenance branch (and thus a 1.11.0 release instead of the 2.0.0 release) instead.

Now, about your problem, just to make sure, we are talking about this staying going:

right?

If so, the issue is not current staying set, the issue is current._file_prefix staying set, which is used to determine the processing state of the unrendered timelapse.

I'd suggest to introduce a new method on Timelapse:

    def _reset_metadata(self):
        self._image_number = None
        self._in_timelapse = False
        self._gcode_file = None
        self._file_prefix = None

        self._capture_errors = 0
        self._capture_success = 0

Then replace all calls to reset_image_number with that (self._reset_metadata) and make sure to fetch the required information for the render process before this happens, to pass to the movie render method:

        def reset_and_create():
            file_prefix = self._file_prefix
            gcode_file = self._gcode_file

            self._reset_metadata()

            render_unrendered_timelapse(
                file_prefix,
                gcode=gcode_file,
                postfix=None if success else "-fail",
                fps=self._fps,
            )

That should take care of things.

I'd also suggest to keep the logic whether to render a timelapse or not in one place, preferably Timelapse.on_print_done - right now it's there and in reset_and_create. Speaking of reset_and_create, that also had a bug in referring to the unknown self.render_failed_print 😬

Also... what do you think, should we maybe also have a "only render if failed" option? Personally I could see myself using this (I'm not interested in timelapse recordings of all my successful prints, but it can be quite helpful for failed prints).

[WisdomCode]

I will work on this next week, I got sick unfortunately... But beforehand, thanks for your extensive analysis of the problem! You suspected correctly that I was referring to the action column showing the loading/ongoing indicator, and your conclusion sounds plausible, I will check it out! Didn't find the linting explicitly as I was testing the code simply by pasting it into my live system and restarting the service :D. Thanks for pointing it out.

While I am not sure I would want such a feature (as one probably sometimes wants to retry immediatly after a failed print with a changed setting or some manual intervention), but its just very easy to add right now so I would put it in so no furher issue arises from that, just having all possible options available from the start. As they are hidden in the drop-down, there is just no disadvantage from a designer standpoint in adding another option for the user I think. :+1:

[foosel]

Get well soon!

[WisdomCode]

Edit: Tested all scenarios now, failing and successful prints on both options that are depending on the prints outcome, and a general test of always and never. So I set this as ready for review.

[GitIssueBot]

This pull request has been mentioned on OctoPrint Community Forum. There might be relevant details there:

https://community.octoprint.org/t/rendering-timelapse-on-pi-zero-2w-alternatives/60502/34

[GitIssueBot]

This pull request has been mentioned on OctoPrint Community Forum. There might be relevant details there:

https://community.octoprint.org/t/timelapse-stop-while-rendering/63013/2