tomvandeneede / p2pp

Palette2/3 Post Processing tool for Prusa Slicer
143 stars 40 forks source link

TEMPERATURECONTROL has temperature raising backwards #147

Open Daghis opened 1 year ago

Daghis commented 1 year ago

I may be wrong, but looking at mcf.py:569, I see "delayed temp rise until after purge".

Having tried this, it seems backwards to me. Don't you want to raise the temperature immediately to accommodate the higher-temperature filament?

For the two branches of this conditional, I was expecting one to have the opposite behavior. (I'm going to do a test and see if that works better for me here and will report back.)

tomvandeneede commented 1 year ago

This was intended to purge at the lowest temperature and then move to the higher after purge with the new filament It sounds contradictory but was implemented at the time to accommodate for printing supports in PV A that burns at too high temperatures.

So the feature works as intended but can look into alternative implementation is if you prefer

On 5 Nov 2022, at 00:42, Marc Chametzky @.***> wrote:

I may be wrong, but looking at mcf.py:569 https://github.com/tomvandeneede/p2pp/blob/4671c96f142fa633ce03571f4a3998bca0f6839d/p2pp/mcf.py#L569, I see "delayed temp rise until after purge".

Having tried this, it seems backwards to me. Don't you want to raise the temperature immediately to accommodate the higher-temperature filament?

For the two branches of this conditional, I was expecting one to have the opposite behavior. (I'm going to do a test and see if that works better for me here and will report back.)

— Reply to this email directly, view it on GitHub https://github.com/tomvandeneede/p2pp/issues/147, or unsubscribe https://github.com/notifications/unsubscribe-auth/AK6CHJJ5WNHGXDG2FASRQQDWGWNOLANCNFSM6AAAAAARXUO7EA. You are receiving this because you are subscribed to this thread.

Daghis commented 1 year ago

Oh, that makes sense. In my situation, I'm printing both PLA and PETG. In this case, I want the purge block always to be the higher temperature.

Now, I'll admit at this point that when I had made this issue, I had FULLPURGEREDUCTION enabled, so TEMPERATURECONTROL didn't have the right effect. I figured that out once I dug into the source a bit more. Since switching back to PrusaSlicer's purge block and the code change I made seems to have the effect that I want, but it's possible that my code change isn't actually relevant given my earlier mistake.

Here's the section after my edit:

                    if command_num in [104, 109]:
                        if v.process_temp:
                            if current_block_class not in [CLS_TOOL_PURGE, CLS_TOOL_START,
                                                           CLS_TOOL_UNLOAD]:
                                g[gcode.COMMENT] += " Unprocessed temp "
                                v.new_temp = gcode.get_parameter(g, gcode.S, v.current_temp)
                                v.current_temp = v.new_temp
                            else:
                                v.new_temp = gcode.get_parameter(g, gcode.S, v.current_temp)
                                if v.new_temp < v.current_temp:
                                    v.temp1_stored_command = gcode.create_commandstring(g)
                                    gcode.move_to_comment(g,
                                                          "--P2PP-- delayed temp drop until after purge {}-->{}".format(
                                                              v.current_temp,
                                                              v.new_temp))
                    elif command_num == 107:
                        v.saved_fanspeed = 0

I eliminated the positive conditional branch and inverted the conditional to leave the formerly-negative branch in place. Obviously, that breaks the behavior your mentioned, so it's not a good edit in general, but does this change appear correct for the PLA/PETG mix I'm doing? (It's somewhat similar in use to PVA in that I'm using PETG as supports for PLA.)

tomvandeneede commented 1 year ago

Could consider adding a HIGH/LOW specifier indicating if it should adjust purge for higher or lower temp. Let me thing about that… should be fairly easy to implement

On 5 Nov 2022, at 11:28, Marc Chametzky @.***> wrote:

Oh, that makes sense. In my situation, I'm printing both PLA and PETG. In this case, I want the purge block always to be the higher temperature.

Now, I'll admit at this point that when I had made this issue, I had FULLPURGEREDUCTION enabled, so TEMPERATURECONTROL didn't have the right effect. I figured that out once I dug into the source a bit more. Since switching back to PrusaSlicer's purge block and the code change I made seems to have the effect that I want, but it's possible that my code change isn't actually relevant given my earlier mistake.

Here's the section after my edit:

                if command_num in [104, 109]:
                    if v.process_temp:
                        if current_block_class not in [CLS_TOOL_PURGE, CLS_TOOL_START,
                                                       CLS_TOOL_UNLOAD]:
                            g[gcode.COMMENT] += " Unprocessed temp "
                            v.new_temp = gcode.get_parameter(g, gcode.S, v.current_temp)
                            v.current_temp = v.new_temp
                        else:
                            v.new_temp = gcode.get_parameter(g, gcode.S, v.current_temp)
                            if v.new_temp < v.current_temp:
                                v.temp1_stored_command = gcode.create_commandstring(g)
                                gcode.move_to_comment(g,
                                                      "--P2PP-- delayed temp drop until after purge {}-->{}".format(
                                                          v.current_temp,
                                                          v.new_temp))
                elif command_num == 107:
                    v.saved_fanspeed = 0

I eliminated the positive conditional branch and inverted the conditional to leave the formerly-negative branch in place. Obviously, that breaks the behavior your mentioned, so it's not a good edit in general, but does this change appear correct for the PLA/PETG mix I'm doing? (It's somewhat similar in use to PVA in that I'm using PETG as supports for PLA.)

— Reply to this email directly, view it on GitHub https://github.com/tomvandeneede/p2pp/issues/147#issuecomment-1304490597, or unsubscribe https://github.com/notifications/unsubscribe-auth/AK6CHJOBB42URZ3RZZQDUITWGYZEXANCNFSM6AAAAAARXUO7EA. You are receiving this because you commented.

Daghis commented 1 year ago

As an additional thought, for this use case, I don't think it's necessary to dwell when raising the temperature before purging. In my case, even going 205° → 240° seems to complete in time for the PETG to start purging after the PLA. This probably isn't a for-everyone thing, though, so if you're adding an option to allow the purge block to be printed hotter, another option for whether to dwell would be greatly appreciated!

(The dwelling causes little blobs to accumulate during that time while the PLA oozes out.)