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

Configurable auto-reconnect and supporting changes. #252

Open
wants to merge 3 commits into
base: 2.7.1-dev
Choose a base branch
from

Conversation

thebeline
Copy link
Contributor

@thebeline thebeline commented Mar 16, 2021

Implementation of configurable Automatic Connect behavior. Limited to turning Auto Connect on or off.

This was requested in #241 and I got researching when I was trying to understand #250. Frankly, this seems reasonable, as the default behaviour (which I left as the defacto default behaviour), has the side-effect of breaking Disconnect in the OctoPrint Web Interface (as immediatly after clicking Disconnect, OctoScreen would initiate an automatic reconnect).

Since that is historical behaviour, I left it, but providing the option to make OctoScreen respect user-choice and the duties of the OctoPrint UI seemed resonable.

As part of this I did rename some methods and buttons for clarity (Reconnect instead of Retry), and there was a misspelling I corrected (instane, unrelated, but I was there).

This build is currently running on all of my instances with expected behaviour.

Resolves #241

@thebeline thebeline mentioned this pull request Mar 16, 2021
@thebeline
Copy link
Contributor Author

It just occurred to me that this existing behaviour, and this PR, all go towards mitigating #10, #56, #58 and #114

@JeffB42
Copy link
Collaborator

JeffB42 commented Mar 23, 2021

@thebeline Just a heads up, I'm thinking of rewriting the internal state machine in 2.8, so this PR may not be needed. Let's keep this PR open and I'll take a look at it again during the development of 2.8.

@thebeline
Copy link
Contributor Author

@thebeline Just a heads up, I'm thinking of rewriting the internal state machine in 2.8, so this PR may not be needed. Let's keep this PR open and I'll take a look at it again during the development of 2.8.

Ooff, that seems like an awful lot for a minor version, but also definitely cool.

Not to press, but in a way i kind of considered this a bug fix more than a feature addition (despite the switch being optional), as the current behaviour invalidates the documented and expected behaviour of the OctoPrint web interface.

@JeffB42
Copy link
Collaborator

JeffB42 commented Mar 24, 2021

Ooff, that seems like an awful lot for a minor version, but also definitely cool.

Yea, 2.6 was a major release. And 2.7 as well, and what I have planned for 2.8, 2.9, and 3.0 are each going to be massive, so they probably should of have their own major release number, but I'm sticking with incrementing the minor version, b/c the UI and UX structure are going to be somewhat consistent.

Not to press, but in a way i kind of considered this a bug fix more than a feature addition (despite the switch being optional), as the current behaviour invalidates the documented and expected behaviour of the OctoPrint web interface.

Just a heads up - my plan is to reuse the OctoPrint states and then add anything extra, like a "connecting to OctoPrint" state. I think between the work done in 2.6, 2.7, and then this, nearly all of the crashing bugs and reboots will be taken care of. (crosses fingers)

One other thing to mention, is that I plan to add a "pre-print" (or maybe call it a "warn-up") state. When you start a print and the tools are cold, OctoPrint is basically unresponsive until the hotend and bed meet their target temps. For example, start a print and then try to cancel it... you can't cancel until they've reached the target temperatures... this doesn't have anything to do w/OctoScreen - it's an OctoPrint issue, but it would be nice to manage this myself, and then start the print, and this is something I'm planning on adding.

On a completely different topic, several users have asked for a feature where the feed from a connected webcam is displayed. Do you own a web cam? If so, are you interested in working on this feature? I haven't looked into it too much yet, I've only just scratched the surface. I've researched it a little and did come across https://stackoverflow.com/questions/35474813/how-to-get-webcam-feed-to-gtk-window and https://packages.debian.org/jessie/libcheese-gtk-dev. I have a lot of thoughts on this feature. If you are interested in taking this on, I'll post a FR with the notes in them (after 2.7 is published)

@thebeline
Copy link
Contributor Author

Yea, 2.6 was a major release. And 2.7 as well, and what I have planned for 2.8, 2.9, and 3.0 are each going to be massive....

That sounds awesome.

Just a heads up - my plan is to reuse the OctoPrint states and then add anything extra, like a "connecting to OctoPrint" state. I think between the work done in 2.6, 2.7, and then this, nearly all of the crashing bugs and reboots will be taken care of. (crosses fingers)

I don't fully what you mean, but I suppose I will later.

One other thing to mention, is that I plan to add a "pre-print" (or maybe call it a "warn-up") state. When you start a print and the tools are cold, OctoPrint is basically unresponsive until the hotend and bed meet their target temps. For example, start a print and then try to cancel it... you can't cancel until they've reached the target temperatures... this doesn't have anything to do w/OctoScreen - it's an OctoPrint issue, but it would be nice to manage this myself, and then start the print, and this is something I'm planning on adding.

So... A few things here:

Looking in to the OctoPrint GCode stream system, it basically renders out the gcode, and chunks it into a queue. As far as I can tell, the only thing OP can do it stop feeding the queue, but otherwise, the queue must flush to the printer (hence "pause" and "cancel" not being immediate when printing, and the printer will continue for a few line segments before it halts).

Additionally, the waiting for the bed and or tools to come up to temp is due to either OctoPrint, or the slicer it's self (the gcode) setting the bed or tool temp with something like M190. This is a blocking code, and the printer will not respond to any other codes until the target has been reached, unless the firmware was compiled with the EMERGENCY_PARSER enabled.

Now, there is already a configuration option in OctoPrint that can handle breaking with M108 in just this kind of situation (Serial Connection > Behavior > Canceling), but it will not work if your firmware does not support it.

So, all of that said, I would kind of hazard away from going too far down that path.

On a completely different topic, several users have asked for a feature where the feed from a connected webcam is displayed.

I have been thinking on it, I'll have a look. I have zero experience with gdk/gtk, and the whole UI system is making my head kind of spin, but I'll sort it out, sure.

Back on topic: Comments/questions/concerns on this PR? I still consider it a bug fix, as I said, it breaks the OctoPrint UI, essentially.

@thebeline
Copy link
Contributor Author

Still kinda sad about this and resolution not being in 2.7, but hey, 2.7.1 already has a headstart! Woot! ;-)

@thebeline
Copy link
Contributor Author

#241 (comment)

@JeffB42
Copy link
Collaborator

JeffB42 commented Mar 29, 2021

This was a big feature change, and I had to weigh delaying the 2.7 release vs. getting this in. Version 2.8 is going to overhaul the state machine, so this PR, or portions of it, might be redundant, so my plan is to work on 2.8, implement the new state machine, and then revisit this PR.

@thebeline
Copy link
Contributor Author

Can you activate the Discussions tab so I can throw some thoughts around outside of the Issues Board?

@thebeline thebeline changed the base branch from 2.7.0-dev to 2.7.1-dev March 29, 2021 14:02
@thebeline
Copy link
Contributor Author

@JeffB42 - I am rounding back to some of my work here, wanted to check before I merged master into my branches if the PRs and branches are even relevant any more. With regards to this branch, the description of the behavior is in comment: #10 (comment)

Let me know if this is still relevant, and if I should update this branch to make this PR relevant.

Thank you.

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

Successfully merging this pull request may close these issues.

2 participants