CR6Community / Marlin

This Marlin fork has the goal of cleaning-up the source code changes for the CR-6 so it can be merged upstream. We also want to extend the functionality to make it fully functional
GNU General Public License v3.0
474 stars 82 forks source link

[FR] When using either the display controls or Octoprint controls to move a stepper motor, all steppers are locked, instead of just the stepper that has been moved #54

Closed Thinkersbluff closed 3 years ago

Thinkersbluff commented 3 years ago

I was planning to file a bug report, saying that Stepper Control seems to be implemented as "all or none", rather than each motor enable/disable being controlled individually as it is on my Ender-3, but I suspect the CR-6 behaviour is actually coded this way by Creality for some reason. I have therefore classed this as a Feature Request instead, for consideration.

On my Ender-3 running either Marlin 2.0.5.3 or 2.0.7.2

On my CR-6, running the latest extui version (downloaded 10pm on 25 Nov 2020):

I only own the two printers, so I have no idea whether the Ender-3 behaves the way all other 3D printers behave, or whether they are both unique, I only know they are not the same.

**In my mind, it would be wonderful if users could have all printers running Marlin behave in one standard way.

If possible, please at least rename the Disable Stepper ON/OFF control to read "Disable Steppers", since it actually does release them all, by design, and the ON/OFF suggests a toggling function but pressing the button repeatedly does NOT toggle the state of the motors.**

NOTE: If in fact that button SHOULD toggle the state of the steppers, please note that it presently does NOT.

Disable Stepper Bug Log.txt

Sebazzz commented 3 years ago

Thank you for the report. To summarize: the issue is that when moving a stepper motor, all steppers are locked, instead of just the stepper that has been moved?

Thinkersbluff commented 3 years ago

Yes.

I like to Home X, and then pull the bed forward, if I have cancelled a print due to some kind of error. Locking all 3 steppers when I move X to the left means I also have to select Back and Disable Steppers as part of my workflow...

Sebazzz commented 3 years ago

I forgot to ask, is this when using the touchscreen or when using Octoprint?

Thinkersbluff commented 3 years ago

I forgot to ask, is this when using the touchscreen or when using Octoprint?

I did not test moving motors from Octoprint, to see whether it locked all 3 motors. I will do that soon and update this report.

Is it ok/correct of me to be renaming this issue for greater precision, or should I stop doing that?

Sebazzz commented 3 years ago

Is it ok/correct of me to be renaming this issue for greater precision, or should I stop doing that?

It helps keep things organized, thank you 😀

Sebazzz commented 3 years ago

Issue CR-6-Touchscreen#18 will be "resolved" by removing the toggle, as you have already suggested.

Sebazzz commented 3 years ago

In the touch screen we do this:

https://github.com/CR6Community/Marlin/blob/c16e2183af125d68958e6d6bcdf4a8c5de830a8f/Marlin/src/lcd/extui/lib/dgus_creality/DGUSScreenHandler.cpp#L818-L820

And a G0 indirectly does this:

https://github.com/CR6Community/Marlin/blob/c16e2183af125d68958e6d6bcdf4a8c5de830a8f/Marlin/src/gcode/motion/G0_G1.cpp#L101

which then does

https://github.com/CR6Community/Marlin/blob/c16e2183af125d68958e6d6bcdf4a8c5de830a8f/Marlin/src/module/motion.cpp#L926

So, I suppose we could just fill destination and call `prepare_line_to_destination.

The only problem I see is that we also feed the current position back to the screen, so things might not work well and we'd need to maintain our own position buffer. Perhaps @Doridian also has some insights into this.

edit: I still don't see how this can be the cause because it all eventually ends up in planner.buffer_line. Perhaps the floating point of the touch screen which is passed in the event handlers isn't exactly the floating point number we already have in the position.

Doridian commented 3 years ago

@Sebazzz The thing, though, is that we don't re-sync all positions. We only sync positions in the change handler. So the display would need to fire a change event on all 3 axes for it to update all 3 positions. current_position is a Marlin internal variable, which isn't even defined by the screen. Maybe we should add a serial log on this event to see what events fire.

Sebazzz commented 3 years ago

I think so. It is too late to properly analyze Marlin's planner algorithms 😅 but from what I can see the steppers only get enabled when there are actually steps to perform, so we send something in the planner what probably warrants a tiny microstep.

Thinkersbluff commented 3 years ago

I forgot to ask, is this when using the touchscreen or when using Octoprint?

I have now confirmed that all 3 axes on the CR-6 also lock if I nudge just one of the three axes with the move buttons in Octoprint.

Thinkersbluff commented 3 years ago

I see the discussion above & though it important to add these things I learned today:

  1. All 3 motors on the CR-6 and on the Ender-3 unlock themselves 2 minutes after I have moved the last one. (apparently per this Configuration_adv.h setting: "#define DEFAULT_STEPPER_DEACTIVE_TIME 120")
  2. I discovered that tapping on the position display on the CR-6 display actually pops up a keypad - did not know that before - moving them that way also behaves as described above.
  3. Neither of the other position displays change, on the CR-6 display, when I move just one axis.
  4. My Ender-3 was running an earlier version of Marlin - 2.0.5.3. I updated it ti 2.0.7.2, today, and confirmed it still only locks the axes I command to move.

I also learned that when an axis is commanded to move, the driver command does not check for an endstop switch, it just keeps going and grinding until the number of steps is finished... (i.e. I learned not to push the bed out of my way & then command it to go to Y=0 when Marlin thinks it is currently at Y=235) Sounds like another feature request I should put in to Thinkyhead...

Thinkersbluff commented 3 years ago

I forgot that Configuration_adv.h also allowed me to set the automatic stepper release commands so that my Z axis would not drop. I make a habit of setting that on my Ender-3 and will start doing that on the CR-6 also, in case it impacts my Z-Offset calibration activity.

i.e. I will now be editing my CR-6 SE extui Configuration_adv.h file to change "true" to "false", here:

/**

AND I am editing the SD_FINISHED_RELEASECOMMAND to "M84XYE" from "M84", here:

define SD_FINISHED_STEPPERRELEASE true // Disable steppers when SD Print is finished

define SD_FINISHED_RELEASECOMMAND "M84XYE" // Use "M84XYE" to keep Z enabled so your bed stays in place

It may be a kindness to make those settings a default, for the Community Version bin file users. Should I open a new FR to suggest that?

Sebazzz commented 3 years ago

It may be a kindness to make those settings a default, for the Community Version bin file users. Should I open a new FR to suggest that?

@Thinkersbluff Yes, please do. If there isn't anything actionable here, please close this issue.