TheSpaghettiDetective / OctoPrint-Obico

GNU Affero General Public License v3.0
138 stars 42 forks source link

Fix server ws backoff bug + discovery backoff refactoring #145

Closed kennethjiang closed 3 years ago

kennethjiang commented 3 years ago

Fixed the bug that caused server WS to be retried only 3 times.

Also simplified the logic for discovery loop and made it use ExpoBackoff in the consistent way with other parts of the code.

encetamasb commented 3 years ago

@kennethjiang This is basically a revert of a fix I added for discovery. Neil was not able to use autodiscovery when he was testing the kit. Somehow time on rpi skews randomly, not always, but it failed consistently that time (happened to me too). I made this change to get around it, by not tracking time directly.

Discovery is a bit special, because its shuts itself down when time is out, so it cannot "heal" in this sense. For the other loops I would actively check that last recorded time is not too far away (far means difference is bigger than max expected interval).

kennethjiang commented 3 years ago

@kennethjiang This is basically a revert of a fix I added for discovery. Neil was not able to use autodiscovery when he was testing the kit. Somehow time on rpi skews randomly, not always, but it failed consistently that time (happened to me too). I made this change to get around it, by not tracking time directly.

Discovery is a bit special, because its shuts itself down when time is out, so it cannot "heal" in this sense. For the other loops I would actively check that last recorded time is not too far away (far means difference is bigger than max expected interval).

Oh I didn't know that. Can you test this PR on your Pi now? Is the problem still happening? I tested auto discovery on my Pi 4 and it worked. Also did the time.time test and it worked.

Screen Shot 2021-11-12 at 7 57 42 AM

It's quite scary to think we can't rely on time.time for logics. We have quite a few other places that are using time.time. We probably need to rewrite them too, depending on the nature of the time.time bug.