fieldOfView / Cura-CustomJobPrefix

A Cura plugin that lets the user set a custom print job prefix
GNU Affero General Public License v3.0
24 stars 5 forks source link

Add additional parameters and support presliced mode #22

Closed ayufan closed 3 years ago

ayufan commented 3 years ago

This adds:

Thanks for the hint where to add it :)

This also makes us support presliced mode where profile/material parameters are replaced with ?.

ayufan commented 3 years ago

@fieldOfView WDYT about this?

ayufan commented 3 years ago

@fieldOfView I added one more addition: the support for presliced mode, as it appears that in this the CustomPrefix is completely ignored, including the prefix.

fieldOfView commented 3 years ago

it appears that in this the CustomPrefix is completely ignored

This is by design. Any parameters you get from the current stack (including what printer is selected) can be totally different from what the file was sliced with. So any changes to the prefix/postfix would be potentially misleading.

ayufan commented 3 years ago

@fieldOfView

Definitely, this is why all parameters except printer name, as this is where you effectively send it to are replaced with ?.

This is effectively aligned with a baseline implementation: to include a printer abbreviated name, but all other params that are unknown to be replaced.

However, this still allows you do chose a directory based on slicer (presliced vs cura), and organise them by date/time of slicing.

ayufan commented 3 years ago

@fieldOfView I added an option to allow to enable prefix for pre-sliced models, instead of enabling it unconditionally always.

I hope that this makes the best of two worlds:

ayufan commented 3 years ago

@fieldOfView Any thoughts on this?

fieldOfView commented 3 years ago

My thoughts on this are that you need to be a little more patient. I don't mean to dissuade you from contributing, but I unfortunately cannot spend all my time on these plugins, so sometimes you will just have to wait a little.

I think it is unfortunate that you put the two things (additional parameters and supporting gcode files) in a single PR. I cannot (easily) merge one without the other.

I understand the functionality of allowing filename changes of gcode files may work for your workflow (where I think you are using gcode files that do not have a prefix or postfix yet), but for people that create gcode files with Cura and load these back in your added functionality will lead to confusion at best (with multiple "stacked" pre- and postfixes containing contradicting information).

I am inclined to reject the functionality for now, which means I have to reject the whole PR.

ayufan commented 3 years ago

@fieldOfView

I think it is unfortunate that you put the two things (additional parameters and supporting gcode files) in a single PR. I cannot (easily) merge one without the other.

It slightly reduces amount of changes.

I understand the functionality of allowing filename changes of gcode files may work for your workflow

I completely understand that. This is why the proposed approach is to allow this configured, with this being disabled by default thus being backward compatible.

fieldOfView commented 3 years ago

This is why the proposed approach is to allow this configured, with this being disabled by default thus being backward compatible.

My consideration is that a checkbox option does not explain to user when they should or should not use the option. IE: it is hard to explain in the user interface the pros and cons of all the options one could images, so I would rather keep the number of options limited. A simple option that has possibly confusing outcomes will result in confused users and support requests that I would prefer not to get.

The PR has not been merged, nor has it been rejected yet. It is under consideration. I am thinking about the implications of merging it, and about ways to make it as little confusing as possible. This takes some time.

ayufan commented 3 years ago

Sure.

Actually I did find it surprising that prefix is not appended for gcode, as gcode is still a project in terms of a Cura. Especially that prefix/postfix is being presented.

Screenshot 2020-09-28 at 22 54 09

It does mean that regardless of that change we do have thi bug today that we show the prefix and postfix for gcode-enabled files regardless of the setting or how it is implemented on upstream master.

fieldOfView commented 3 years ago

as gcode is still a project in terms of a Cura.

It isn't. A project file can be edited, a gcode file cannot. Not in Cura anyway. The more I think about it, the more I think the printjob name for gcode files should be entirely readonly, like the gcode itself is.

fieldOfView commented 3 years ago

I'm closing this PR. This commit hides the prefix and postfix labels for presliced files: c245b2f1eaf6f518bc1252524e1a5dd23102a7be Prefixes nor postfixes are added to the filename when loading gcode or ufp files.

Unfortunately this means your additional parameters are also not merged from this PR. That's the effect of trying to fix multiple things in one PR.

ayufan commented 3 years ago

@fieldOfView

What can I do for you to merge?

Can you suggest a way to move forward?

fieldOfView commented 3 years ago

ability to define prefix/postfix for gcode

I have decided that that is not going to happen

additional parameters

Start a new PR

ayufan commented 3 years ago

@fieldOfView

I have decided that that is not going to happen

Can you reconsider this decision?

The more I think about it, the more I think the printjob name for gcode files should be entirely readonly, like the gcode itself is. My consideration is that a checkbox option does not explain to user when they should or should not use the option.

I don't think that this is gonna be confusing with your change fixing visibility aspect. Previously you could be confused by this: as prefix was shown, but it was not applied. Maybe we can further refine the wording, tooltip, or anything that would make people not ask about that.

Currently it is:

Enable for pre-sliced models
Use path and prefix for pre-sliced models (gcode). The missing parameters will be substituted with '?'

Maybe something like this would be better?

Append prefix/postfix to `.gcode` files

Cura can load pre-sliced `.gcode` files for the purpose of visualisation and printing. Pre-sliced file does not contain a print parameters, but in some cases it might be desired to append path, prefix and postfix to the saved or upload file. Parameters unknown in pre-sliced model are replaced with `?`.

Maybe even marking this as:

Append prefix/postfix to `.gcode` files (experimental)

I hope that this reads easily, and makes people to not be confused. Especially with your fix that properly presents when prefix/postfix is appended, which was not case before.

fieldOfView commented 3 years ago

Give me a usecase.

in some cases it might be desired to append path, prefix and postfix to the saved or upload file

What cases?

If you load a file into Cura, and then save it again, the file should be unchanged. And in my opinion, so should the filename.

When is it useful to change the printer name or type part of the filename, even though the gcode has not changed, so the suitability for that printer has not changed?

ayufan commented 3 years ago

@fieldOfView

Give me a usecase.

Upload all files to Octoprint under a dedicated path and filename for the archival purposes. Each of such files has a dedicate entry, and a associate time lapse that can be easily connected to the upload.

This is possible given there's the {date_iso} and {time_iso} being exposed as part of parameters.