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

Fix, cleanup DELTA G28 / G29 support functions #4373

Merged
merged 2 commits into from
Jul 22, 2016

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Jul 22, 2016

Reference: #4361 (comment)

Recent changes to probing and movement functions missed some nuances, such as:

  • How to calculate a difference in position. (Use subtraction)
  • How to use the movement functions. (Don't set current_position ahead of calling movement functions except those that specifically move to the current_position because some of them want to compare the destination to current_position.)
  • There's no need to set destination from current_position after a call to prepare_move_to_destination* because they always call set_current_to_destination.

This PR patches these bugs and removes other superstitious redundancies in setting current_position and destination.

@thinkyhead thinkyhead force-pushed the rc_homing_leveling_wtf branch from 495e0b1 to 311c7da Compare July 22, 2016 21:18
@thinkyhead thinkyhead merged commit 5655f8b into MarlinFirmware:RCBugFix Jul 22, 2016
@thinkyhead thinkyhead deleted the rc_homing_leveling_wtf branch July 22, 2016 21:34
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 22, 2016
@ghost
Copy link

ghost commented Jul 23, 2016

Thanks for the trying to fix and hard work, but results of G28 and G29 of DELTA has been exactly same as Yesterday's report unfortunately...

@ghost
Copy link

ghost commented Jul 23, 2016

By the way, your (@thinkyhead's) DELTA machine has been shipped by DHL just now.
So, Is it better that fixing for homing and probing of DELTA are left as it's until you get a DELTA machine?

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 23, 2016

shipped by DHL just now

❗ WOW❗ I'm very excited for its arrival! But I think I should keep addressing any issues that I happen to catch. For example, #4370 brings some important changes and certainly addresses some bugs. That PR deals with the movement functions mostly, not with the logic of delta probing (yet). So I need to look at the logic next.

If you do more testing on your delta after #4370 is merged, please use DEBUG_LEVELING_FEATURE + M111 S32 and post a log. That is very helpful, even more than a video oftentimes.

@ghost
Copy link

ghost commented Jul 23, 2016

Oh, I've never known about M111 S32.
I wasn't sure why I can't get debugging log nevertheless I enabled DEBUG_LEVELING_FEATURE and uint8_t marlin_debug_flags = DEBUG_LEVELING.
I'm thankful for teaching.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 23, 2016

I've never known about M111 S32.

Yes, we need to mention that in the configuration file! (#4378)

@thinkyhead thinkyhead mentioned this pull request Jul 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants