base auto stop bug fix #152
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@micheal-parks - if you want to see my attempt at this. still doesn't work with multiple clients on the same machine, but im tracking the touch 'session' locally and cancelling if the base is moving but touch has ended. |
|
This ended up not being the specific bug Eliot was seeing, but it is similar. |
njooma
requested changes
Nov 30, 2023
lib/widgets/joystick.dart
Outdated
| ) | ||
| ], | ||
| ); | ||
| } | ||
|
|
||
| void callSetPower(StickDragDetails details) { | ||
| touchActive = (details.x != 0 && details.y != 0); |
There was a problem hiding this comment.
I think this should be || for the use case of when I'm fully horizontal (e.g. only fully right). Then X would be non-zero, but Y would be zero
There was a problem hiding this comment.
oh yeah you're totally right, great catch, thank you!
njooma
approved these changes
Nov 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added a perhaps too lengthy comment describing some of the thoughts on this issue in the code, I will explain here as well.
Essentially things tend to work well in normal network conditions, but as soon as the client is in spotty conditions, not enough to end the session but enough to garble the order of some calls or have some not make it through, the joystick widget tends to not auto stop not upon touch release and the base will continue to move in whatever the last command was.
My first thought was to shorten the sessions heartbeat interval, but that is configured per robot and not per client.
So I ended up tracking the touch locally in the widgets state and sending a stop command if the base is still moving but touch has been released, I check 100 milliseconds after each call.
If network conditions allow this will stop the base sooner than the ending session would, though if network conditions do not allow the stop call to make it through the ending session will eventually stop base.
Another option would be to continually check if the bass is moving and if the touch is active every 100ms during the widgets life cycle. I didn't opt for that to limit extraneous calls.