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

Adding an option "Post Connect Delay" (enhances #234) #269

Open
wants to merge 5 commits into
base: devel
Choose a base branch
from

Conversation

Gifford47
Copy link

Thank you for your interest into contributing to OctoPrint-PSUControl, it's highly appreciated!

Before submitting please make sure you have ticked all points on this checklist:

  • Your PR targets the devel branch.
  • Your PR was opened from a custom branch on your repository (no PRs from your version of master or devel please)
  • Your PR only contains relevant changes: no unrelated files, no dead code, ideally only one commit - rebase your PR if necessary!
  • Your changes follow the existing coding style.
  • You have tested your changes (please state how!)

Feel free to delete all this help text, then describe your PR further. You may use the template provided below to do that. The more details the better!


What does this PR do and why is it necessary?

it´s on top of PR #234 but with the difference of using timer. its tested multiple times und works great!

How was it tested? How can it be tested by the reviewer?

Set a post connect delay in the options and connect to printer with the ui button.

Any background context you want to provide?

see #234

What are the relevant tickets if any?

#234

Screenshots (if appropriate)

Further notes

@kantlivelong
Copy link
Owner

kantlivelong commented Aug 22, 2023

Wouldn't this be more of a "pre connect delay" which would be satisfied by "postOnDelay" ?

@Gifford47
Copy link
Author

Yes, technically, actually. I also had trouble with the naming at first. Since I originally took the code from the author, the "post" refers to the action after the button is pressed.
Maybe it depends on how you look at it :)

Feel free to rename it :)

@Gifford47
Copy link
Author

@kantlivelong are planning to implement this PR?

@Gifford47
Copy link
Author

@kantlivelong is this repo actually maintained?

@kantlivelong
Copy link
Owner

@kantlivelong is this repo actually maintained?

Pretty much break/fix for now.

@Gifford47
Copy link
Author

My PR is now tested over 1 year with no further issues. Can you merge it?

@paccerdk
Copy link

Please merge.
There's currently a handful of forks, dealing with this particular problem.
Judging by that, this seems to be the main problem that people actively deal with - this is a problem that needs fixing.

I've been applying this patch manually for the last year while waiting for a merge (since this is what is in the plugin repository)

If the repository isn't actually going to be maintained (as in, merge pull request for fixes) please let us know.

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.

3 participants