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.

Sign up for GitHub

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

Jump to bottom

Fix M428 width DELTA & SCARA #3955

Merged
merged 1 commit into from
Jun 3, 2016
Merged

Fix M428 width DELTA & SCARA #3955

merged 1 commit into from
Jun 3, 2016

Conversation

MagoKimbra
Copy link
Contributor

No description provided.

@thinkyhead thinkyhead merged commit 4e13cd3 into MarlinFirmware:RCBugFix Jun 3, 2016
@thinkyhead
Copy link
Member

thinkyhead commented Jun 4, 2016 •
edited
Loading

@MagoKimbra It seems as though we should replace sync_plan_position with sync_plan_position_delta everywhere that it's used. But there are some places in DELTA context where sync_plan_position is being called instead (and without calculate_delta). It seems like a good idea to re-examine those.

@jbrazio @esenapaj

@MagoKimbra
Copy link
Contributor Author

I check it....
OK!

ghost
ghost reviewed Jun 4, 2016
View reviewed changes
Marlin/Marlin_main.cpp
@@ -6005,7 +6005,11 @@ inline void gcode_M428() {
}

if (!err) {
sync_plan_position();
#if ENABLED(DELTA) && ENABLED(SCARA)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thinkyhead
I'm sorry, I tried to understand your instruction and content of this merged PR,
but I stumbled on the first step.
Why isn't this ENABLED(DELTA) || ENABLED(SCARA) but ENABLED(DELTA) && ENABLED(SCARA)?
This combination is forbidden in SanityCheck.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you're right, my mistake

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patched!

thinkyhead added a commit to thinkyhead/Marlin that referenced this pull request Jun 4, 2016
@thinkyhead
Fix a typo in MarlinFirmware#3955
d7e4e8e
thinkyhead added a commit that referenced this pull request Jun 4, 2016
@thinkyhead
Merge pull request #3958 from thinkyhead/rc_fix_3955
05868de 
Fix a typo in #3955
@ghost
Copy link

ghost commented Jun 5, 2016 •
edited by ghost
Loading

@MagoKimbra
I also made a follow-up for this PR by way of trial. Please refer to this if you need.
https://github.com/esenapaj/Marlin/pull/137

This asinine my PR is extremely danger.

It breaks the function of endstops on delta machine.

Please don't refer to this.

I'm sorry.

@ghost ghost mentioned this pull request Jun 6, 2016
Closed
@thinkyhead thinkyhead mentioned this pull request Jul 8, 2016
Closed
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 18, 2016
CONSULitAS pushed a commit to CONSULitAS/Marlin-K8200 that referenced this pull request Aug 18, 2016
@thinkyhead
Fix a typo in MarlinFirmware#3955
0c99dfe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug: Confirmed ! PR: Bug Fix
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@MagoKimbra @thinkyhead @jbrazio