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

M600 wait for heatup, prevent stepper timeout, etc. #5794

Merged
merged 6 commits into from
Feb 15, 2017

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Feb 10, 2017

Rebase, squash, and cleanup of #5737

Reference: #5675

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 10, 2017

@Tannoo Please pay particular attention to what happens if the nozzles time out and you click to continue. It looks to me like the "Wait for nozzle to heat" message is not going to get displayed.

  bool temps_ready = false;
    while (!temps_ready) {
     idle();
     temps_ready = true;
     HOTEND_LOOP() {
       if (abs(thermalManager.degHotend(e) - temps[e]) > 3) {    
//lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_WAIT_FOR_NOZZLES_TO_HEAT);
          temps_ready = false;
          break;
        }
       }
      }

@ghost
Copy link

ghost commented Feb 10, 2017

I know, I didn't do that.

@ghost
Copy link

ghost commented Feb 10, 2017

I am going to download this and test as it is.

@thinkyhead thinkyhead force-pushed the rc_m600_improve branch 3 times, most recently from f0e9cf6 to 8eaf202 Compare February 10, 2017 21:14
@thinkyhead
Copy link
Member Author

thinkyhead commented Feb 10, 2017

Ok, I've updated the function so it will always show "Wait for heatup." If the nozzle is already at temperature then the message will only show for 1 frame (or possibly not at all) before showing the next screen.

The main thing I wanted to avoid was repeatedly setting the message screen inside of loops, instead setting just once before each loop. I believe the LCD screen handler will keep calling lcd_implementation_hotend_status as long as the screen is showing, but if the screens (particularly character displays) don't update during heating, then we can just add lcdDrawUpdate = LCDVIEW_CALL_REDRAW_NEXT after each call to lcd_implementation_hotend_status and that should keep them re-displaying.

@ghost
Copy link

ghost commented Feb 11, 2017

I commited my changes to get it working again to @Roxy-3D's RCBugFix.

@ghost
Copy link

ghost commented Feb 11, 2017

These are the changes:

marlin_main.cpp:

    // Wait for filament insert by user and press button
    wait_for_user = true;    // LCD click or M108 will clear this
    next_buzz = 0;
    runout_beep = 0;
    HOTEND_LOOP() temps[e] = thermalManager.target_temperature[e]; // Save nozzle temps

    while (wait_for_user) {
      millis_t current_ms = millis();
      if (!nozzle_timed_out)
        lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_INSERT);
      if (nozzle_timed_out)
        lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_CLICK_TO_HEAT_NOZZLE);
      #if HAS_BUZZER 
        filament_change_beep();
      #endif //HAS_BUZZER
    if (nozzle_timed_out)      // Turn nozzles back on if we turned them off.
      HOTEND_LOOP() thermalManager.setTargetHotend(temps[e], e);

    bool temps_ready = false;
    while (!temps_ready) {
      idle();
      temps_ready = true;
      HOTEND_LOOP() {
        if (abs(thermalManager.degHotend(e) - temps[e]) > 3) {
          lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_WAIT_FOR_NOZZLES_TO_HEAT);
          temps_ready = false;
        }
      }
    }

    wait_for_user = true;    // LCD click or M108 will clear this
    next_buzz = 0;
    runout_beep = 0;
    while (wait_for_user) {
      if (nozzle_timed_out == true)
        lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_INSERT);
      else break;
      #if HAS_BUZZER
        filament_change_beep();
      #endif
      idle(true);
    }

ultralcd.cpp:

    void lcd_filament_change_wait_for_nozzles_to_heat() {
      START_SCREEN();
      STATIC_ITEM(MSG_FILAMENT_CHANGE_HEADER, true, true);
      STATIC_ITEM(MSG_FILAMENT_CHANGE_HEATING_1);
      #ifdef MSG_FILAMENT_CHANGE_HEATING_2
        STATIC_ITEM(MSG_FILAMENT_CHANGE_HEATING_2);
      #endif
      STATIC_ITEM(" ");
      STATIC_ITEM(MSG_FILAMENT_CHANGE_NOZZLE, false, true);
      lcd_implementation_hotend_status();
      lcdDrawUpdate = LCDVIEW_CALL_REDRAW_NEXT;
      END_SCREEN();
    }

    void lcd_filament_change_heat_nozzle() {
      START_SCREEN();
      STATIC_ITEM(MSG_FILAMENT_CHANGE_HEADER, true, true);
      STATIC_ITEM(MSG_FILAMENT_CHANGE_HEAT_1);
      #ifdef MSG_FILAMENT_CHANGE_INSERT_2
        STATIC_ITEM(MSG_FILAMENT_CHANGE_HEAT_2);
      #endif
      STATIC_ITEM(" ");
      STATIC_ITEM(MSG_FILAMENT_CHANGE_NOZZLE, false, true);
      lcd_implementation_hotend_status();
      lcdDrawUpdate = LCDVIEW_CALL_REDRAW_NEXT;
      END_SCREEN();
    }

    void lcd_filament_change_insert_message() {
      START_SCREEN();
      STATIC_ITEM(MSG_FILAMENT_CHANGE_HEADER, true, true);
      STATIC_ITEM(MSG_FILAMENT_CHANGE_INSERT_1);
      #ifdef MSG_FILAMENT_CHANGE_INSERT_2
        STATIC_ITEM(MSG_FILAMENT_CHANGE_INSERT_2);
      #endif
      #ifdef MSG_FILAMENT_CHANGE_INSERT_3
        STATIC_ITEM(MSG_FILAMENT_CHANGE_INSERT_3);
      #endif
      STATIC_ITEM(MSG_FILAMENT_CHANGE_NOZZLE, false, true);
      lcd_implementation_hotend_status();
      lcdDrawUpdate = LCDVIEW_CALL_REDRAW_NEXT;
      END_SCREEN();
    }
    void lcd_filament_change_extrude_message() {
      START_SCREEN();
      STATIC_ITEM(MSG_FILAMENT_CHANGE_HEADER, true, true);
      STATIC_ITEM(MSG_FILAMENT_CHANGE_EXTRUDE_1);
      #ifdef MSG_FILAMENT_CHANGE_EXTRUDE_2
        STATIC_ITEM(MSG_FILAMENT_CHANGE_EXTRUDE_2);
      #endif
      #ifdef MSG_FILAMENT_CHANGE_EXTRUDE_3
        STATIC_ITEM(MSG_FILAMENT_CHANGE_EXTRUDE_3);
      #endif
      STATIC_ITEM(" ");
      STATIC_ITEM(MSG_FILAMENT_CHANGE_NOZZLE, false, true);
      lcd_implementation_hotend_status();
      lcdDrawUpdate = LCDVIEW_CALL_REDRAW_NEXT;
      END_SCREEN();
    }

@ghost
Copy link

ghost commented Feb 11, 2017

Tested working. Applied these to @Roxy-3D's RCBugFix.

@ghost
Copy link

ghost commented Feb 11, 2017

Ok...the ghost click is eluding me. I've narrowed down that the only time it is NOT there is the first M600 call after flashing the firmware. Any other calls for M600 will result in the ghost (extra) click. This ghost click is responding with the click beep, but nothing happens. After that ghost click, all else works as normal.
Power cycling or resetting the controller still results in the ghost click. If I flash the firmware, the ghost click is not there on the first instance of M600.

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 11, 2017

This may not be related... But the Z-BabyStep edit screen no longer times out and goes back to status. It is supposed to do that. I'm only seeing the extra click on the very last click to either resume the print or extrude more filament.

@ghost
Copy link

ghost commented Feb 11, 2017

I'm only seeing the extra click on the very last click to either resume the print or extrude more filament.

Yes.. the ghost click. And it is isolated to only that spot.

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 11, 2017

One thing to check would be how other edit type fields and confirmation messages exit their routines. I wonder if we are missing some kind of clean up at the end of the M600 ? Because it is very strange it happens at the end. Or here is another possibility... Maybe the screen really has been exited, but a new screen has never been redrawn over the old (and not needed) M600 Exit screen. (Maybe we are missing a REDRAW THE SCREEN type flag???)

Actually... Looking closer at the code:

    #if defined(FILAMENT_CHANGE_EXTRUDE_LENGTH) && FILAMENT_CHANGE_EXTRUDE_LENGTH > 0
      do {
        // Extrude filament to get into hotend
        lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_EXTRUDE);
        destination[E_AXIS] += FILAMENT_CHANGE_EXTRUDE_LENGTH;
        RUNPLAN(FILAMENT_CHANGE_EXTRUDE_FEEDRATE);
        stepper.synchronize();
        // Ask user if more filament should be extruded
        KEEPALIVE_STATE(PAUSED_FOR_USER);
        lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_OPTION);
        while (filament_change_menu_response == FILAMENT_CHANGE_RESPONSE_WAIT_FOR) idle(true);
        KEEPALIVE_STATE(IN_HANDLER);
      } while (filament_change_menu_response != FILAMENT_CHANGE_RESPONSE_RESUME_PRINT);
    #endif

    lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_RESUME);

I'm wondering if that last line needs to be take out. Once the do loop ends... It might be nice to put a message up on the LCD, but we don't want to wait for the user to click again!

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 11, 2017

It is confirmed. Removing the last line eliminates the extra click at the end.

    lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_RESUME);

However, to be clean about it, we should also take out the case: from the switch in lcd_filament_change_show_message()

        case FILAMENT_CHANGE_MESSAGE_RESUME:
          lcd_goto_screen(lcd_filament_change_resume_message);
          break;

and the function:

    void lcd_filament_change_resume_message() {
      START_SCREEN();
      STATIC_ITEM(MSG_FILAMENT_CHANGE_HEADER, true, true);
      STATIC_ITEM(MSG_FILAMENT_CHANGE_RESUME_1);
      #ifdef MSG_FILAMENT_CHANGE_RESUME_2
        STATIC_ITEM(MSG_FILAMENT_CHANGE_RESUME_2);
      #endif
      #ifdef MSG_FILAMENT_CHANGE_RESUME_3
        STATIC_ITEM(MSG_FILAMENT_CHANGE_RESUME_3);
      #endif
      END_SCREEN();
    }

I can't do the Pull Request easily because this stuff migrated off my fork. @Tannoo, are you set up to do the deletions? Oh! It maybe some of those MSG_FILAMENT_CHANGE_RESUME_1/2/3 messages don't need to exist anymore either????

@thinkyhead
Copy link
Member Author

thinkyhead commented Feb 11, 2017

I can't do the Pull Request easily because this stuff migrated off my fork.

You can both always do pull requests to my branch.
Github doesn't limit you to only doing PRs against MarlinFirmware.

https://github.com/thinkyhead/Marlin/compare/rc_m600_improve...MyName:my_copy_of_branch

I do this all the time on contributors' branches.

When I merge the changes to my branch from your PRs, they are magically added to this PR.

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 11, 2017

You can both always do pull requests to my branch.
Github doesn't limit you to only doing PRs against MarlinFirmware.

The root problem I'm fighting is GitHub won't let me fork something more than once. It complains it doesn't know where to put it. How can I fork (or clone) your branch in my area?

@thinkyhead
Copy link
Member Author

thinkyhead commented Feb 11, 2017

git remote add thinkyhead https://github.com/thinkyhead/Marlin.git
git fetch thinkyhead
git checkout thinkyhead/rc_m600_improve -b patch_m600_improve

...

git push --set-upstream origin patch_m600_improve
open https://github.com/thinkyhead/Marlin/compare/patch_m600_improve...Roxy-3D:patch_m600_improve?expand=1

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 11, 2017

No web page equivalents?

@thinkyhead
Copy link
Member Author

thinkyhead commented Feb 11, 2017

No web page equivalents?

Not that I'm aware of. Working outside the command-line would only slow me down. The web interface only has the "basics."

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 11, 2017

If you give me administrator privileges to thinkyhead/rc_m600_improve I'll get the changes there...

@thinkyhead
Copy link
Member Author

You don't need special privileges to check out my branch or to make a PR against it.

@ghost
Copy link

ghost commented Feb 11, 2017

Sounds like the same battle I had with Roxy's branch a while ago.

@ghost
Copy link

ghost commented Feb 11, 2017

It is confirmed. Removing the last line eliminates the extra click at the end.

This will also mean that it won't display the "Resuming...". But, this is just a screen display that doesn't require user interaction.

The ghost click I found was before this screen even gets displayed.

@ghost
Copy link

ghost commented Feb 11, 2017

Now, I have that ghost every time at the end of filament change.
I have tried several different ways to eliminate it....but nope.

@ghost
Copy link

ghost commented Feb 11, 2017

I had no resume screen, but still had to click twice.

@thinkyhead
Copy link
Member Author

If all you want is for the "resume" screen to time-out back to the info screen…

        case FILAMENT_CHANGE_MESSAGE_RESUME:
          lcd_goto_screen(lcd_filament_change_resume_message);
+         defer_return_to_status = false;
          break;

@thinkyhead
Copy link
Member Author

Sounds like the same battle I had with Roxy's branch a while ago.

I urge you all to master git.

@ghost
Copy link

ghost commented Feb 12, 2017

I do not run Linux anymore. Too many other softwares I use are not on Linux and do not run well under Wine.

@ghost
Copy link

ghost commented Feb 12, 2017

Some of the commands you list are not there for the Git Desktop. Or, I'm not sure how to get to it.

@thinkyhead
Copy link
Member Author

Google for how to install a command-line git on your system, and you will find simple step-by-step instructions.

@Roxy-3D
Copy link
Member

Roxy-3D commented Feb 12, 2017

I had no resume screen, but still had to click twice.

I think I'm still seeing the need for a double click. So... Maybe the resume screen should stay??? And perhaps I'll try ThinkyHead's suggestion tomorrow. I just got a filament run out sensor wired up so I'm testing that right now. And maybe tomorrow I'll be in a position to do some real testing on this annoying double click demon.

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Feb 12, 2017

A command line git is automatically installed with Git Desktop. You just have to call it by by right clicking the repository name an selecting Open in Git Shell.
For me a tremendous help is 'git gui' and most of all 'gitk'. If you know how to use them you can do almost all you could do with the pure shell 'git ???`' in a more or less graphical manor. Espesially adding (temporary) additional repository (forks) and removing them later is really easy.
(Even if i admit AnHard is the git wizard here at KitLab.)

@ghost
Copy link

ghost commented Feb 12, 2017

Holy Crap! There's a whole other world right there! Thanks @Blue-Marlin.

More schooling is in order. lol

@thinkyhead
Copy link
Member Author

A command line git is automatically installed with Git Desktop.

I figured that had to be the case. On macOS it simply has "Open in Terminal," and probably it's the same on Linux.

@ghost
Copy link

ghost commented Feb 12, 2017

If all you want is for the "resume" screen to time-out back to the info screen…

The info screen and the resume screens work just fine.

All we wanted was the "dead", or "ghost" click to be gone. "double-click" is the wrong term because there is no time delay involved.
At the resume or extrude screen:

lcd_filament_change_show_message(FILAMENT_CHANGE_MESSAGE_OPTION);

Click once....nothing happens. Doesn't matter how long you wait.
Click a second time, and it finally resumes or extrudes (depending on the selection).

@ghost
Copy link

ghost commented Feb 12, 2017

So... Maybe the resume screen should stay???

Yes, I believe so.

Should I bow out while you heavy-weights work on this? I can and will still test. If I bow out, I'll still be here.

@ghost ghost mentioned this pull request Feb 13, 2017
@thinkyhead thinkyhead force-pushed the rc_m600_improve branch 2 times, most recently from 0ba2875 to 5681816 Compare February 15, 2017 05:59
@thinkyhead
Copy link
Member Author

thinkyhead commented Feb 15, 2017

If you make this change does the "ghost click" go away?

    void lcd_filament_change_option_menu() {
      START_MENU();
-     #if LCD_HEIGHT > 2
-       STATIC_ITEM(MSG_FILAMENT_CHANGE_OPTION_HEADER, true, false);
-      #endif
      MENU_ITEM(function, MSG_FILAMENT_CHANGE_OPTION_RESUME, lcd_filament_change_resume_print);
      MENU_ITEM(function, MSG_FILAMENT_CHANGE_OPTION_EXTRUDE, lcd_filament_change_extrude_more);
      END_MENU();
    }

And/or, if you scroll up and down in the menu before clicking the menu item does it still need an extra click?

@thinkyhead
Copy link
Member Author

Merging this now because it's at least better than it was. But we may still need to clear up why that first click is being swallowed.

@thinkyhead thinkyhead merged commit babe1d2 into MarlinFirmware:RCBugFix Feb 15, 2017
@ghost
Copy link

ghost commented Feb 15, 2017

I'm testing this here in a few hours.
Thank you, @thinkyhead.

@ghost
Copy link

ghost commented Feb 15, 2017

if you scroll up and down in the menu before clicking the menu item does it still need an extra click?

Yes it did.

@ghost
Copy link

ghost commented Feb 15, 2017

Also, once the extra click was done..like when extruding more, any additional clicks worked as intended. Like extruding more again or resuming print.

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.

None yet

3 participants