Multimaterial oozing fix

[fughilli]

This pull request demonstrates a minimum-viable implementation of the ramp-while-wipe procedure I described in this feature request. It is not ready for merge (see the commit log for details), but I thought I'd share it in case anyone wants to give it a try.

[Ghostkeeper]

I would not implement this in the routine for skirt/brim. That will only have any effect on the first layer (and only if the skirt/brim is active). What is that doing?

I'm also not very happy with the speed_factor parameter being added to everything. Is there not a way to bind the speed required in the pathconfigs? Or perhaps in the same way as the merge infill lines does?

It would be good to look at our code style documents.

The inserts stage is not allowed to modify anything in the plan that would change the time estimates. That stage itself depends on the time estimates to work correctly.

[fughilli]

It is doing exactly that--there will be nozzle switches in the skirt/brim sequence as in the prime tower sequence. Because the prime tower first layer is only printed with extruder 0, the nozzle switch handling has to be performed in the brim/skirt code. The temperature ramp while wipe has to occur at every nozzle switch for this fix to work. When I didn't have the brim switch fix, the part was pulled off the bed by an oozed strand. The simplest way I could proof this concept was to duplicate the logic in the brim/skirt code and in the prime tower code. However, the eventual implementation should generalize nozzle switching in a way that it can be done in a single place.

I am also not happy with the way this is implemented, but I wanted to poke the bug to see if we can work together to make progress on it :) Re: speed_factor, what are you referring to by pathconfig? I see GCodePathConfig, but that contains speeds which have no public accessor. Could you provide specific references to the code so I can read those implementations?

Re: style docs: It seems that there's a fair amount of code in CuraEngine which is not compliant (function names mixing camel and snake case, repeated declaration of access restrictions out of public-protected-private ordering, incomplete/incorrect const usage, etc.). Should I be taking it upon myself to correct existing style while working on the code, or should I limit the linecount of my diff?

Re: overall program structure: Is there a document which describes what the separation of logic should be between the various modules? I see extensive use of friend and public mutable member variables which makes tracing control flow and ownership quite difficult.

Re: flag-enablement: I see that there are various objects with "Config" in their names, and they are populated from various sources. If I want to add an experimental option, such as those available under the "experimental" section in the Cura UI, what are the pieces I need?

As an aside, I'm an adherent of Google C++ style, but unfortunately that style guide is at odds with several of your internal guidelines. I was trying to copy the local style as best I could, and will take the docs you referenced into account moving forward.

[Ghostkeeper]

Unfortunately you're looking here at an application that has been developed for 7 years, from time to time under considerable time pressure. Initially it was set up by a single person (Daid) for over a year and then maintained by a single person (BagelOrb) for over 2 years, before finally being the responsibility of multiple people (though it's mostly been me the past few years). It's seen multiple hand-overs. This is why the code style and documentation is all over the place.

We hold the boy scout rule for code style: If you touch it, you fix it. Normally I consider that line-by-line to minimise the linecount of the diff, as you say (but if it's simple I'll update the variable names or something in other places too).

There is a central location for switching extruders. It's called setExtruder in the planning phase.

The path configs are generated once, reused many times. This is critical for performance.

There is documentation about the data structures here and about the different stages here.

[fughilli]

You are indeed the keeper of ghosts then! I can hang with that rule, but you'll have to be patient with the code review process as I'll probably want to set up a sequence of commits to keep the changes self-contained.

The docs you linked are awesome! Love the in-depth explanations and diagrams. I think I'm getting a better idea of what's going on in the code.

Re: LayerPlan::setExtruder: is this the correct place to plan the wipe-while-ramp? The code currently plans the priming in FffGcodeWriter::setExtruder_addPrime which invokes LayerPlan::setExtruder in addition to FffGcodeWriter::addPrimeTower --> storage.primeTower.addToGcode. Is this not modifying the total path in a layer post-planning stage? Should this logic be consolidated into LayerPlan::setExtruder? I'm basically wondering where the wipe logic should live.

Since the prime tower is added at this late stage, is it being included in the timing calculations that feeds the inserts stage?

I will also re-up this question, as I don't think you answered it:

If I want to add an experimental option, such as those available under the "experimental" section in the Cura UI, what are the pieces I need? (I looked in this doc but it does not discuss where the config comes from, i.e., which files are read to produce that config, and how the frontend, Cura, populates that config from the UI)

[Ghostkeeper]

I wasn't entirely honest, sorry; one of my colleagues has also been picking up on development the last half year. I'm glad that he is because it was a bit of a bus factor.

The front-end sends the settings over to CuraEngine via a socket (it arrives in the ArcusCommunication class of CuraEngine, but that is irrelevant for you.)

To add a setting to the front-end, you'd basically need to add it to this humongous file. Around line 6000 is where the experimental settings start, so you'd need to add it there. A few things to know when making your own setting:

• default_value is a required property. It's necessary to run CuraEngine from command line.
• value overrides default_value. The value is a Python expression (as string). All the values for other settings are pre-filled as local variables.
• The settable_per_mesh and ...per_extruder and such are all true by default, but we still like to write the most specific one down by way of documentation.

[jellespijker]

Closing this PR due to inactivity, if you feel it's still relevant feel free to reopen it