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

Manual Filament Change: A New Hope #3605

Closed

Conversation

thinkyhead
Copy link
Member

I've added onto #3601 to illustrate a more proper way to put it together, and to mess around in the menu system to see how it behaves when used "recursively." This is posted as a PR for easy viewing and commentary on the code.

Note that this feature is somewhat redundant with the (currently experimental) FILAMENTCHANGEENABLE feature that adds an M600 command and a Filament Change menu item to run the M600 routine. So in this demo implementation I made MANUAL_FILAMENT_CHANGE a subset of that feature.

@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch 3 times, most recently from d11a981 to 274712f Compare April 24, 2016 23:29
@schirrel
Copy link

Great ;D
if there's anything more that i can help on this i'd be glad x) (even know that u guys have more time coding than i have in life)

@thinkyhead
Copy link
Member Author

@codersquirrel I think I might have spent more time coding than most other things. Maybe that's sad! But it's been a useful tool to explore and test ideas — and of course to be able to improve the software that I use every day.

So I'll point out some of the more interesting alterations I made here:

  • Instead of three bool variables (which will never be set at the same time) we use an enum to keep track of the current state of the procedure. This seems to work well.
  • Instead of inserting-slow forever, it will stop after the full insert length.
  • I changed the verb from "Remove" or "Retract" to "Eject" since that is more idiomatic in English.
  • Added a "Stop Insert / Eject" menu item that displays while insert or eject is active.
  • Added a clear-and-redraw when you use "Stop …", so the menu titles can be updated. (Maybe that is also needed when you start an eject / insert?)
  • Since FILAMENTCHANGEENABLE is already existing, I decided to use the existing FILAMENTCHANGE_FINALRETRACT for the insert and retract length. (Your feature could be separate from M600 but since this is related it would be good to integrate them.)

@schirrel
Copy link

schirrel commented Apr 24, 2016

@thinkyhead yeah from your point of view i think they really show be integrate. And i will be really honest i thougth and M600 could only be used at printing time, or it isn't? That's why i have done this.
I know for sure that you have spend more time code than i have in life, so let's say not that much LOL
About your points:

  • the enum idea is awesome i'm feling kinda dumb for not thinking about it xD, i just not that used with c/c++ stuff that i always forget they have a bunch of things that the others language have.
  • i talked about the reason of the slow forever in the Manual Filament Change feature #3601 (just for don't write it again), and there's also the explication of why theres no stop to eject and insert, not really "there's no stop why" but my logic is there so...
  • The good this this with the redraw, while my test i saw some glitchs(?) with lcd

@thinkyhead
Copy link
Member Author

thinkyhead commented Apr 25, 2016

M600 could only be used at printing time…

Actually that's right – I forgot to account for that. The Tune menu only appears while printing, so the Auto Filament Change should be visible at that time. Also during printing, while the Prepare menu is hidden the Main menu is still visible. So you should either hide Manual Filament Change during active printing or –better– put it into the Prepare menu instead.

Also I think it would be good to think more about how to marry M600 and "MFC" together, so they get along. For example, they probably should share a common "total filament path" value for totally ejecting/inserting filament, along with an "extra purge" length.

@schirrel
Copy link

Yes, the Manual Filament Change should be hide while printing. I was just worried about you understand why i have the slow and fast. But maybe i'm also no understanding you cude, somethimes my brain kinda freeze.
So i'm thiking about it two. And you think it would be beter form me to fork you project, with these changes and proper implemented?

@thinkyhead
Copy link
Member Author

thinkyhead commented Apr 25, 2016

I understand why there is slow and fast, and it's a good idea. I just think there needs to be some kind of limit on the slow insert so it can't go on for too long. Either by time or by length. In the same vein, I think it's helpful to have "Stop" items to cancel the insert/eject.

I also like the use of "Yes/No" boolean menu items to toggle it. But I hope later we can have better display options that can be more descriptive.

  • Notice I also added defer_return_to_status = true during the eject/insert so the menu won't time-out and return to the status screen. I think this will also help!

…you think it would be better for me to fork your project…?

You can actually just get my commits into your repository from the command line:

git remote add thinky [email protected]:thinkyhead/Marlin.git
git fetch thinky
git checkout rc_filament_change_rcbugfix
git merge thinky/rc_filament_change_too

You can then continue your work from there, starting after the commits I added.

I would have submitted a PR to your branch, but for some reason Github doesn't allow that. (Or I'm too daft to figure it out.)

@schirrel
Copy link

schirrel commented Apr 25, 2016

Yeah, analyzing better it really shouldn't go forever. And i think that time can be better (2 minutes or 3 are enough i think).
Also once that u came about menu option, yesterday i was wondering, once i haven't see it, maybe it's there, but i haven't seen it. There's a "choice" option in marlin? As a combobox or things like it. To choose an option among others?
I will do it now =D

@schirrel
Copy link

Sadly appears:

schirrel@inspiron:~/Github/RC5/Marlin$ git fetch thinkyhead
fatal: 'thinkyhead' does not appear to be a git repository
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
schirrel@inspiron:~/Github/RC5/Marlin$ git checkout rc_filament_change_rcbugfix
Already on 'rc_filament_change_rcbugfix'
Your branch is up-to-date with 'origin/rc_filament_change_rcbugfix'.
schirrel@inspiron:~/Github/RC5/Marlin$ git merge thinkyhead/rc_filament_change_too
merge: thinkyhead/rc_filament_change_too - not something we can merge
schirrel@inspiron:~/Github/RC5/Marlin$ 

@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch from da0ec54 to 8c3ea58 Compare April 25, 2016 00:27
@thinkyhead
Copy link
Member Author

thinkyhead commented Apr 25, 2016

"git fetch thinky" … sorry 'bout that. (I like to keep my remote names short.)

git remote add thinky [email protected]:thinkyhead/Marlin.git
git fetch thinky
git checkout rc_filament_change_rcbugfix
git merge thinky/rc_filament_change_too

git push -f

@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch from 8c3ea58 to 69ab0a0 Compare April 25, 2016 00:35
@schirrel
Copy link

schirrel commented Apr 25, 2016

schirrel@inspiron:~/Github/RC5/Marlin$ git merge thinky/rc_filament_change_too
merge: thinky/rc_filament_change_too - not something we can merge

Seems to not work ;/

@thinkyhead
Copy link
Member Author

thinkyhead commented Apr 25, 2016

If unable to merge, then you may need to git fetch thinky again. I'm pretty sure I didn't alter any of your former commits.

You may also need to do git fetch upstream ; git rebase upstream/RCBugFix on your branch.

@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch 2 times, most recently from e1a94ad to e0b4b8f Compare April 25, 2016 01:35
@schirrel
Copy link

Oh god it is getting tricky i think.

Permission denied (publickey).
fatal: Could not read from remote repository.

when i use fetch thinky

@thinkyhead
Copy link
Member Author

Try again. It's probably a spurious error. You should have no trouble doing git fetch on any of your remotes (unless you're having trouble with all of them).

@thinkyhead
Copy link
Member Author

This PR and #3662 will need to be reconciled before either one could be committed.

@schirrel
Copy link

schirrel commented May 3, 2016

@thinkyhead i had a problem with my computer ins this mean time, therefore i haven't done anything.
But i will try this week yet and as i soon i as test your solution i'll give u a feedback. But:
1º U knwo the right trach that things on marlin shoud follow
2 if u tested mine to see how i worked, and tested yours, and it follows the same logic i see no problem and merging yours.

but any way i'll look it up soon

@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch 2 times, most recently from 7817e51 to 060a6b4 Compare June 13, 2016 02:02
@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch from 060a6b4 to dd690ac Compare June 24, 2016 04:34
@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch 4 times, most recently from 9ecd8e0 to b26dbbd Compare July 2, 2016 23:47
@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 2, 2016

@codersquirrel This probably needs additional patching-up to coexist nicely with the updated FILAMENT_CHANGE_FEATURE. For the moment I have them co-existing even though they use somewhat different techniques to display messages and do user interaction. We should look closer at this and see where we can take advantage of some of the methods employed by FILAMENT_CHANGE_FEATURE, and get them to look and behave in similar ways.

@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch from b26dbbd to b4d1ed4 Compare July 3, 2016 00:28
@schirrel
Copy link

schirrel commented Jul 3, 2016

Hi Scott, yeah i really Think that this feature need to be worked around. Unfortunately my intern at a 3D printer company, where i was coding and making the mods, had finished a month ago and so i haven't any machine to work around and improve it. :c

I guess this time i wouldn't be able to help more. I'm trying to save money to get one soon, near to the end of the year, but till then my hands are tied.

@thinkyhead thinkyhead force-pushed the rc_filament_change_too branch from b4d1ed4 to 5fc24f5 Compare July 3, 2016 20:40
@petrzjunior
Copy link
Contributor

@thinkyhead Finally got to rewriting and harmonizing this with M600. But I have a question: Is there any possibility of homing all axes via method? Currently only enqueing G28 is possible. What if inline void gcode_G28() called planner.homeAll(), for instance?

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 22, 2016

Enqueueing G28 is the only currently supported method to do homing from other points in the code. We haven't yet split the homing procedure from G28 but that is certainly possible. It wouldn't be a Planner method, but a Machine method (when that class exists). Is there some reason you need to "force" homing from M600?

@thinkyhead
Copy link
Member Author

Finally got to rewriting and harmonizing this

So, I should be able to git fetch your fork and merge your branch into this, yes?

@petrzjunior
Copy link
Contributor

Is there some reason you need to "force" homing from M600?

I was talking about the Manual Load/Unload - you have to be homed, if you want to automaticly move to some position befor Unload.

So, I should be able to git fetch your fork and merge your branch into this, yes?

I still don't have it publicly, just on my computer. But in few days I will push it and than yes, you could put it here. But it's completely rewriting this and moving it to its own Singleton to keep code clean, as proposed.

@thinkyhead
Copy link
Member Author

Manual Load/Unload - you have to be homed

Typically, when you can't do something right now (because you're not homed, for example) we just hide the option in the LCD menu, rather than allow it to be selected and making a buzzing sound to indicate you need to home, or something like that. It is possible to home and wait for homing. The manual-bed-leveling feature does that. But for most things, we've generally felt it best to encourage the user home the machine before their use.

@thinkyhead thinkyhead closed this Aug 26, 2016
@thinkyhead thinkyhead deleted the rc_filament_change_too branch August 26, 2016 07:32
@schirrel
Copy link

schirrel commented Feb 28, 2017

So... i've been off almost a year, and i'm no longer active in the community :( but, how's this going?

@thinkyhead
Copy link
Member Author

thinkyhead commented Mar 1, 2017

M600 is working well, especially since recent troubleshooting and cleanup.

@schirrel
Copy link

schirrel commented Mar 1, 2017 via email

@thinkyhead
Copy link
Member Author

thinkyhead commented Mar 1, 2017

But m600 can only work in printing time isnt?

M600 can be initiated at any time (homing is required and the nozzle must be heated).

@maksdampf
Copy link

maksdampf commented Apr 24, 2017

I think #3605 is not about M600, and CoderSquirrel obviously didn't ask about #3662 (mid print automatic filament change).

#3605 probably should be a menu item in "filament" and include G28 homing, M109 S260 heating and M600, very much the way the ultimaker manual material change works.
This is a standard maintenance procedure which makes sense to be included in "prepare" or "filament" 'cause it saves tons of clicks navigating to seperate maintenance g-code files on the sdcard.

I haven't tried the most recent marlin (cause DFWAB) to see if this is included and i could not find anything definitive in the patch notes or discussions on the recent Marlin RC8 about it.

@schirrel
Copy link

schirrel commented Apr 24, 2017

#3605 probably should be a menu item in "filament" and include G28 homing, M109 S260 heating and M600, very much the way the ultimaker manual material change works.

my feature was made because I saw one ultimaker doing this and at my p.o.w. this is super handy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants