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

Apply config changes, tweak comments, next_ping_ms => next_idle_ms #3761

Merged
merged 1 commit into from
May 20, 2016

Conversation

thinkyhead
Copy link
Member

Recent changes were made to prevent watchdog timeout in very long arc moves. This involves calling idle() every 200ms. A side-effect of this call is some stuttering during arcs. This PR addresses the issue by only calling idle() once out of 4 times, while only calling thermalManager.manage_heaters() (which also resets the watchdog timer) the other 3 times.

@jbrazio jbrazio added this to the 1.1.0 milestone May 15, 2016
@AnHardt
Copy link
Member

AnHardt commented May 15, 2016

An alternative could be to call thermalManager.manage_heaters() unconditional. If will return immediately if no update is required. Or to use if (!temp_meas_ready) for the condition to call thermalManager.manage_heaters().

if (!temp_meas_ready) return;

Finding an alternate condition for idle() is much harder. For manage_inactivity() and checking the encoder ~1/second seems to be to slow, while display updates with 1Hz seem to be ok to me.

@thinkyhead
Copy link
Member Author

Perhaps it should be made possible to lower the display refresh rate with a variable, so we can still call lcd_update to get controller knob and button events, but redraw the display only every half second (minimum).

@thinkyhead thinkyhead force-pushed the rc_plan_arc_idle branch 2 times, most recently from f033504 to aadf42e Compare May 17, 2016 21:18
@thinkyhead thinkyhead changed the title Less stuttering in plan_arc? Less stuttering in plan_arc? – Adding a variable lcd display update interval May 17, 2016
@thinkyhead
Copy link
Member Author

Trying this idea with both plan_arc and cubic_b_spline functions.

@AnHardt
Copy link
Member

AnHardt commented May 18, 2016

The time between display updates is aboute 1s. Changing the comment does not change the fakts.

-    if (currentMenu == lcd_status_screen) {
-      if (!lcd_status_update_delay) {
-        lcdDrawUpdate = LCD_DRAW_UPDATE_CALL_REDRAW;
-        lcd_status_update_delay = 10;   /* redraw the main screen every second. This is easier then trying keep track of all things that change on the screen */
-      }
-      else {
-        lcd_status_update_delay--;
-      }
-    }
+    // Simply redraw the Info Screen 10 times a second
+    if (currentMenu == lcd_status_screen && !(++lcd_status_update_delay % 10))
+      lcdDrawUpdate = LCDVIEW_REDRAW_NOW;

Why this ugly, slow modulo division?

@thinkyhead
Copy link
Member Author

thinkyhead commented May 18, 2016

"Simply redraw the Info Screen 10 times a second"

I must have been lacking sleep or very stoned I wrote that. Obviously backward.

Otherwise, valid concerns. Please propose/contribute alternatives/improvements.
I have very limited time to attend to Marlin right now.

@thinkyhead thinkyhead changed the title Less stuttering in plan_arc? – Adding a variable lcd display update interval Apply config changes, tweak comments, next_ping_ms => next_idle_ms May 19, 2016
@thinkyhead thinkyhead merged commit 61de6da into MarlinFirmware:RCBugFix May 20, 2016
@thinkyhead thinkyhead deleted the rc_plan_arc_idle branch May 20, 2016 00:10
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