search

moggieuk / ERCF-Software-V3

New software driver for ERCF control under Klipper
GNU General Public License v3.0
298 stars 63 forks source link

Danielstc91 standalone macro patch #15

Closed Danielstc91 closed 1 year ago

Danielstc91 commented 1 year ago

Reorganized Params to follow tip form sequence in SS Commented how each param effects tip shape Added SS_RAMMING param to use SS built in ramming(moves during ramming process to reduce blobs) Added default values for RapidoHT Added comment to tool change macro to clarify when to use standalone vs slicer settings Renamed COOLING_TUBE_RETRACTION to COOLING_TUBE_POSITION - This is how SS calls it and is logical

This is to hopefully clear up the tip shaping macro the way SS does it as well as being able to transfer settings from macro to SS for those who want to use per filament SS settings.

moggieuk commented 1 year ago

I love the idea of cleaning up the standalone macro... I don't know where/when it was actually introduced but it certainly isn't super clear.

Couple of concerns/questions:

  1. You have reverted the setting of params to defaults on each of the macro params. This undoes a very recent PR that moved them to variables on the macro. The advantage of the latter is that they can be manipulated anywhere in gcode even during a print (for example which different filament types)
  2. I suspect less folks are doing "standalone" top forming than using the slicer. Therefore I don't want to make standalone the default. Perhaps use a global (shared) variable to drive the STANDALONE=0 or STANDALONE=0 behavior?
  3. There has been some discussion and work on fast & simplified tip creation logic. I don't have an opinion on which is better because I haven't had the chance to test, but it would be good to sync as a group to choose the direction we go. We could also have both: SS compatible way and a "quick method" and make that a user choice too..
  4. Nit, but there are a couple of changes in the PR that would break install (the {token} expansion). I can/would omit but just pointing it out.
Danielstc91 commented 1 year ago

I noticed this just after I seen the update you pushed!

  1. I will update to variables to be more inline with the PR.
  2. I agree default standalone should not be 1. I had set it with the thought process that standalone would be a good starting point and rapid testing then swapping over to SS once u plug in numbers. I'll change defaults to 0
  3. I think having both would be a good idea as well. a SS/slicer version or a quick form macro and let users decide which one.
  4. I'll look into this as well and update
Danielstc91 commented 1 year ago

Created new PR that involves new updates to macro variables.