-
Notifications
You must be signed in to change notification settings - Fork 1.3k
accelsender: add widget and settings, remove need to reload on activa… #4159
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
Conversation
bobrippling
left a comment
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.
LGTM
Sometimes accelsender gets stuck active and I only find out when the App Loader becomes unstable because it's spammed with accel messages.
So the problem is that the interval sounds like it drops to a very short time causing the spam? Or there's no debounce on the accel handler?
| // Note: this is ignoring interval variable from GB and prefering watch settings | ||
| break; | ||
| default: | ||
| if (_GB) setTimeout(_GB, 0, e); |
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.
Out of interest, why do we have this in a timeout?
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.
I copied from gbmusic, so I'm not sure, but I'd assume its to give it a new "thread" to run in:
BangleApps/apps/gbmusic/boot.js
Line 35 in 2a71b0f
| if (_GB) setTimeout(_GB, 0, e); |
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.
Researched a bit and I was wrong: at least in other javascript situations, the setTimeout is for helping avoid call stack growth and possible blocking/timing "re-entrancy" issues...
Which in my over-simplified view, is like starting it as a new thread/task. But I guess might be more of a "yield"? 🤷
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.
Ah, so gbmusic uses it to ensure it runs last of all, after any other processing of the event. I don't think we need that here :) But you're right, it's like starting a new task
The main problem is that it gets stuck "on." GadgetBridge sends the command to enable tracking, but if we're not connected when the command to disable is sent, it stays on. Long explanation: GadgetBridge can't query Sleep as Android about current tracking state, and GadgetBridge only receives SleepAsAndroid messages while a device is connected, so it can't be sure what state tracking is supposed to be in, just relay the on/off messages... So my best idea is the widget and letting users decide to disable. (I should definitely add info about that to the readme) Secondary problem is that it only ever sends accel messages every 1 second. |
Thanks for the explanation - I suppose this adds overhead for the user to monitor it, what do you think about detecting disconnects and turning it off in that case? And/or we could provide an event so GB can query the current state, wdyt? |
|
Hopefully this only happens in rare cases, so its more of a debug indicator than something you'd need to monitor. As far as I can tell this has happened to me 4 times in a year. I don't know about stopping automatically on disconnect, because I feel like it'd be more common that I lose connection for like 1-5min during the night, and I'd want it to keep tracking when connection restored. Since GB can't query SleepAsAndroid, when the bangle reconnects GB doesn't know if accelsender should be active or not. But now that I think of it, GB could say that if we just connected and accelsender is active, send the "start tracking" command to SleepAsAndroid. I assume if it was already active, it would ignore it, but if not then SleepAsAndroid would make it really obvious on the phone that it was active. I'll put this on hold and give that a test. |
|
Sure - I'm happy to merge this and let you continue with the test in another PR if you like? |
|
Ok I thought more and realized the better option is to go with your auto-stop on disconnect idea, but add setting to disable it for users like me who think they know better. I can add that without it affecting these changes, so this can merge if you want. Thanks! |
|
Great, thanks for this PR and I look forward to the next one |
Link: https://kidneyhex.github.io/BangleApps/?id=accelsender
Tagging original author: @AnotherStranger
This is just a proposal, I'm good with living with my own fork.
Sometimes
accelsendergets stuck active and I only find out when the App Loader becomes unstable because it's spammed with accel messages.This adds a widget to show when
accelsenderis active, and adds a settings page to manually disable it and/or set the interval time.Updates
widget.jssetings.jsboot.jsandroidQuestions
a) 1s - uses about 10% battery per night for me
b) 5s - uses about 3-5% per night
c) 10s - I keep losing connection when I try this
d) other
intervalfrom GadgetBridge?accelsenderor split intowidaccelsender?TODO
readmeat all.androidcode for 'accelsender' messages if this is approved.