Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LIN_ADVANCE does not work with DELTA #7521

Closed
tcm0116 opened this issue Aug 16, 2017 · 8 comments
Closed

LIN_ADVANCE does not work with DELTA #7521

tcm0116 opened this issue Aug 16, 2017 · 8 comments

Comments

@tcm0116
Copy link
Contributor

tcm0116 commented Aug 16, 2017

I tried LIN_ADVANCE on my Delta machine, and it did not go well. With any non-zero value for K, the extruder would shudder violently and ultimately retracted the filament as opposed to feeding anything. I've been looking through the code, and I have a few thoughts as to why this is happening.

First, the a, b, and c parameters passed to Planner::_buffer_line for a Delta machine are the distances to move the tower carriages. However, Planner::_buffer_line computes the distance of the move as if a, b, and c are in the XYZ coordinate system. As such mm_D_float and block->millimeters do not represent the actual distance that the nozzle moves.

To correct this issue, Planner::_buffer_line needs to know the starting and ending coordinates of the move in the XYZ coordinate system. One solution would be to call forward_kinematics_DELTA to determine these coordinates, but that would be rather expensive. Another option is to provide either the XYZ delta in mm to Planner::_buffer_line when the machine is a Delta type, so that it can calculate the actual length of the move in the XYZ coordinate system. This should be easily achievable since prepare_kinematic_move_to can simply provide segment_distance to Planner::buffer_line (which calls Planner::_buffer_line).

  const long target[XYZE] = {
    LROUND(a * axis_steps_per_mm[X_AXIS]),
    LROUND(b * axis_steps_per_mm[Y_AXIS]),
    LROUND(c * axis_steps_per_mm[Z_AXIS]),
    LROUND(e * axis_steps_per_mm[E_AXIS_N])
  };

  #if ENABLED(LIN_ADVANCE)
    const float mm_D_float = SQRT(sq(a - position_float[X_AXIS]) + sq(b - position_float[Y_AXIS]));
  #endif

  const long da = target[X_AXIS] - position[X_AXIS],
             db = target[Y_AXIS] - position[Y_AXIS],
             dc = target[Z_AXIS] - position[Z_AXIS];

  float delta_mm[XYZE];
  delta_mm[X_AXIS] = da * steps_to_mm[X_AXIS];
  delta_mm[Y_AXIS] = db * steps_to_mm[Y_AXIS];
  delta_mm[Z_AXIS] = dc * steps_to_mm[Z_AXIS];
  block->millimeters = SQRT(
      sq(delta_mm[X_AXIS]) + sq(delta_mm[Y_AXIS]) + sq(delta_mm[Z_AXIS])
  );

Second, block->use_advance_lead is set to true if there are steps in the X or Y axes, but these once again correspond to movement of the carriages on the towers and not movement in the XYZ coordinate system. The seemingly simple solution would be to check that block->steps are not equal for the ABC axes since moving all three carriages an equal amount results in the nozzle moving up or down in the Z axis. However, I believe the Delta calibration offsets can result in the steps not being equal for a Z only movement. A better solution is to use the same solution as the previous issue and look at the XYZ delta in mm and verify that there is movement in the XY plane.

    block->use_advance_lead =  esteps
                            && (block->steps[X_AXIS] || block->steps[Y_AXIS])
                            && extruder_advance_k
                            && (uint32_t)esteps != block->step_event_count
                            && de_float > 0.0;

I'll try to implement fixes to these two issues and see what happens. If anyone has any other insights into this problem, please chime in.

In the meantime, we might consider adding a sanity check to verify that LIN_ADVANCE is not enabled with DELTA since it currently does not work

@Sebastianv650
Copy link
Contributor

Good and true information. I just wasn't reading them in time. thinkyhead added the sanity check as he also found out that it's incompatible.
I'm working in a version 2 that should fix it (#9048), but I doing tests with the planner system first.

One important thing I learned is to put a note into the LIN_ADVANCE section of configuration_adv to mention "@Sebastianv650" or I will likely never read the related issue reports X-)

@filippor
Copy link
Contributor

for this i suggest to look at pull request #8778

@mylife4aiurr
Copy link

mylife4aiurr commented Jan 10, 2018

I dont experience any issues with lin_advance on my delta since 1.1.6

even 1.1.8 disable sanitycheck.h for Delta and Lin_Advance, no problems.
delete or comment out

#if ENABLED(LIN_ADVANCE) && !IS_CARTESIAN
#error "DEPENDENCY ERROR: Sorry! LIN_ADVANCE is only compatible with Cartesian."
#endif

I must be an anomaly

@tcm0116
Copy link
Contributor Author

tcm0116 commented Jan 11, 2018

@mylife4aiurr it will probably work in a degraded manner, but I doubt it provides optimal results due to the issues I laid out.

@filippor I completely forgot about this issue when I was working on #8878. But to your point, that solution is basically an implementation of what I outlined above.

@thinkyhead
Copy link
Member

thinkyhead commented Feb 11, 2018

@tcm0116 Does #8778 by happenstance do anything positive for LIN_ADVANCE?

@tcm0116
Copy link
Contributor Author

tcm0116 commented Feb 11, 2018

#8778 does address the first issue I outlined. I haven't looked at #9379 to see if it addresses the second issue.

@Sebastianv650
Copy link
Contributor

@tcm0116 yes #9379 is now fully DELTA compatible.

@MarlinFirmware MarlinFirmware deleted a comment from tcm0116 Feb 19, 2018
@MarlinFirmware MarlinFirmware deleted a comment from iosonopersia Feb 19, 2018
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants