-
-
Notifications
You must be signed in to change notification settings - Fork 176
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
WiFi provisioning and serial fixes #590
Conversation
this all works now btw lmao |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GUI constantly crashes (and reloads) whenever I click a button in the serial console (maybe due to the fact that I don't have a tracker connected... missing null check?)
server/src/main/java/dev/slimevr/serial/ProvisioningHandler.java
Outdated
Show resolved
Hide resolved
server/src/main/java/dev/slimevr/serial/ProvisioningHandler.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Erimel <marioluigivideo@gmail.com>
Co-authored-by: Erimel <marioluigivideo@gmail.com>
29975eb
to
2f70031
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dismissing since it also crashes in 0.6.3, so the crashing is unrelated to the PR
dismissing since it also crashes in 0.6.3, so the crashing is unrelated to the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested successfully by Zrock and dq
server/src/main/java/dev/slimevr/serial/ProvisioningHandler.java
Outdated
Show resolved
Hide resolved
LogManager.severe("Using serial ports is not supported on this platform", e); | ||
LogManager | ||
.severe("[SerialHandler] Using serial ports is not supported on this platform", e); | ||
throw new RuntimeException("Serial unsupported"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we getting rid of the original exception? I don't think it's even safe to do this anyways, as it crashes the server when it's unsupported. If this is being caught somewhere else, can I be pointed to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's executed in the TimerTask, I think it will just crash and stop the timer? I might be wrong, I didn't check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the following tasks scheduled in the timer wont run because background thread is dead
Co-authored-by: Butterscotch! <bscotchvanilla@gmail.com>
This PR does:
Some stuff is a bit broken: