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

Extend stack_overflow_protection for lcd_move_e and _lcd_level_bed #3126

Merged

Conversation

Blue-Marlin
Copy link
Contributor

Extend stack_overflow_protection for lcd_move_e() and _lcd_level_bed() with
a refined method of #3050.
Simple, small, local, elegant.

Fix for one of AnatoliyKube's problems in #3054 (comment) (if i solved his language riddle correct)

@@ -862,7 +862,8 @@ static void lcd_move_e(
if (encoderPosition != 0) {
current_position[E_AXIS] += float((int)encoderPosition) * move_menu_scale;
encoderPosition = 0;
line_to_current(E_AXIS);
if (movesplanned() <= 3)
line_to_current(E_AXIS);
lcdDrawUpdate = 1;
}
Copy link
Member

Choose a reason for hiding this comment

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

As this is, it will drop moves, and those moves will stay dropped when the LCD times out and returns to the info screen. However, it turns out there is an easy solution here to avoid dropping any moves:

if (encoderPosition != 0 && movesplanned() <= 3) {
  current_position[E_AXIS] += float((int)encoderPosition) * move_menu_scale;
  line_to_current(E_AXIS);
  encoderPosition = 0;
  lcdDrawUpdate = 1;
}

This holds onto the encoder position and doesn't reset it, so the next time this is invoked, the move will still be pending. With this change you can throw away the line_to_current call with LCD_CLICKED below.

@@ -2249,13 +2253,15 @@ char* ftostr52(const float& x) {
if (min_software_endstops) NOLESS(current_position[Z_AXIS], Z_MIN_POS);
if (max_software_endstops) NOMORE(current_position[Z_AXIS], Z_MAX_POS);
encoderPosition = 0;
line_to_current(Z_AXIS);
if (movesplanned() <= 3)
line_to_current(Z_AXIS);
lcdDrawUpdate = 2;
}
Copy link
Member

Choose a reason for hiding this comment

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

Same idea here…

if (encoderPosition != 0 && movesplanned() <= 3) {
  refresh_cmd_timeout();
  current_position[Z_AXIS] += float((int)encoderPosition) * MBL_Z_STEP;
  if (min_software_endstops) NOLESS(current_position[Z_AXIS], Z_MIN_POS);
  if (max_software_endstops) NOMORE(current_position[Z_AXIS], Z_MAX_POS);
  line_to_current(Z_AXIS);
  encoderPosition = 0;
  lcdDrawUpdate = 2;
}

@thinkyhead
Copy link
Member

The same technique I suggest can also be applied to the other move routines. You can amend this PR and add those also.

@Blue-Marlin Blue-Marlin force-pushed the stackoverflow-with-e branch from 08db379 to 893bc70 Compare March 13, 2016 12:40
Extend stack_overflow_protection for lcd_move_e() and _lcd_level_bed() with
a refined method of 3050.
@Blue-Marlin Blue-Marlin force-pushed the stackoverflow-with-e branch from 893bc70 to c73f1b2 Compare March 13, 2016 13:04
thinkyhead added a commit that referenced this pull request Mar 14, 2016
Extend stack_overflow_protection for lcd_move_e and _lcd_level_bed
@thinkyhead thinkyhead merged commit 21be07a into MarlinFirmware:RCBugFix Mar 14, 2016
@Blue-Marlin Blue-Marlin deleted the stackoverflow-with-e branch March 17, 2016 21:14
@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