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

Idle during long arcs #3751

Merged
merged 2 commits into from
May 14, 2016
Merged

Idle during long arcs #3751

merged 2 commits into from
May 14, 2016

Conversation

AnHardt
Copy link
Member

@AnHardt AnHardt commented May 13, 2016

Idle during long arcs
to prevent from watchdog resets during high segmented fast arcs.

Includes: Make arc support (G2/G3) configurable #3747

Fix for: Crash (Hard Reset) when issuing G2/G3 command #3729

Saves about 2669 bytes when deactivated. (About 1% for a AT2560, about __4%__ for a AT644!)
@jbrazio
Copy link
Contributor

jbrazio commented May 13, 2016

Will close #3747, if needed we can cherry-pick from this PR.

@@ -6484,7 +6488,7 @@ void process_next_command() {
break;

// G2, G3
#if DISABLED(SCARA)
#if ENABLED(ARC_SUPPORT) & DISABLED(SCARA)
Copy link
Member

Choose a reason for hiding this comment

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

Oops. Two ampersands would be better here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

@thinkyhead
Copy link
Member

thinkyhead commented May 13, 2016

It's hard to perceive the changes, because indentation and changes are combined in a single commit. Would it be possible to add an intermediate commit, with the code changes but not the indentation, and then a commit with the indentation afterward? If that sounds tedious and annoying, I don't mind doing it myself. It just makes it much easier to see the smaller changes that were made, especially for future reference…. esp. as I will need to add similar changes to the Bézier curve support.

@AnHardt
Copy link
Member Author

AnHardt commented May 13, 2016

https://github.com/MarlinFirmware/Marlin/pull/3747/files?w=1
89d9908
Or if you want bot commits togeter but without the changes in the white space
https://github.com/MarlinFirmware/Marlin/pull/3751/files?w=1
?w=1 is your friend. :-)

r_Y = -offset[X_AXIS] * sin_Ti - offset[Y_AXIS] * cos_Ti;
count = 0;
}
millis_t previous_ms = millis();
Copy link
Contributor

Choose a reason for hiding this comment

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

You can shave this millis() call and init the var as 0.

Copy link
Member Author

@AnHardt AnHardt May 13, 2016

Choose a reason for hiding this comment

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

Why should i want that.
That would call idle() one time at the beginning of the loop, where we don't need it.

Copy link
Contributor

Choose a reason for hiding this comment

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

It depends on the perspective..
It was a suggestion.

Idle during long arcs
to prevent from watchdog resets during high segmented fast arcs.
@thinkyhead thinkyhead merged commit 6903eb1 into MarlinFirmware:RCBugFix May 14, 2016
@thinkyhead
Copy link
Member

I'm curious. Why do we disallow arc support with SCARA? And it looks like actually we should add a sanity-check for ARC_SUPPORT plus SCARA now, if they are indeed incompatible.

@AnHardt AnHardt deleted the fix-arcs branch May 15, 2016 11:41
@thinkyhead thinkyhead mentioned this pull request Jul 8, 2016
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 18, 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.

3 participants