-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Close unidentified websocket when the app is in background #13337
base: main
Are you sure you want to change the base?
Conversation
ce18927
to
20b75db
Compare
The issue of battery drain (especially on AOSP) was there since long. But I just noticed today it now uses only 2% battery as opposed to 30+ %. I had last updated the app on 6th December to v6.41.3 Somehow seems that the issue was kind of fixed before it made it to the update. Or maybe the exact behavior of the cause still not fully known. Issue #12341 seems to be definitely related to this. Ultimate goal should be to minimize battery drain while still not having to miss notifications. Hopefully this fix will make it more efficient. |
After going through #12341 entirely, I now realised that it used 2% battery because I didn't leave it open that day. But mostly I do leave it open in the app drawer and only then it uses excessive battery. |
@p1gp1g Got it. Thanks. |
How does one know when this PR makes it to the final build ? |
Maybe we can just close this open issue with the resolution being uninstall signal and install Molly? |
20b75db
to
ca93e33
Compare
I've force-pushed a new version (co-authored with @valldrac). This now works differently:
As the previous patch:
The previous patch can be found here: https://github.com/p1gp1g/Signal-Android/tree/close_unidentified_ws_v1 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@p1gp1g Can you please elaborate which patch has found its way to the now distributed application? |
@flxai Can you clarify which distributed application are you talking about ? |
Signal for Android on the Play Store |
Well, the pull request is still open, so nothing has been merged into the app |
@p1gp1g And for now it seems like using unifiedPush may be the only solution to the battery drain without GMS. |
I use Molly-FOSS with websockets, and this PR is merged, and I am not experiencing any delay in notifications whatsoever. |
@matchboxbananasynergy 🤔 you may be right. I may have misconfigured UP which is causing that.
Does it still drain the battery ? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Most likely still relevant right? |
I'd say so. I was tagged in some other signal issue related to this so there is still discussion going on. It was a month ago or so. I provided the version of Molly that I'm using. Alternatively, my proposed solution above still stands. This issue has been on again off again in my experience, for the past couple years. I believe there were related issues to this opened in the past but I can't be certain. To answer your question; I believe it should stay open since I believe it's still an issue per the other issues that are discussing it. Personally, I've moved onto Molly since they fixed it right away. So this can stay open but I don't know what good it will do... |
Anyone experiencing more than 10% battery usage by signal/molly in a day, one important consideration to compare battery usage between the two: make sure to disable chat backups under settings in both signal and molly. Molly has an option to schedule it on daily/weekly basis whereas signal does not. If backups are enabled in signal it means it performs backup daily and it uses a huge amount of battery(up to 50%) daily especially if you have a lot of chats and media(I have 20+GB). It would probably consume same amount of battery for chat backups in molly. After disabling chat backups, I have around/under 10% battery use by signal, which I think is still huge. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Might be still relevant |
First time contributor checklist
Contributor checklist
Fixes #1234
syntax > This is related to Excessive battery drain #12341, but it requires some feedback before considering it fixed.Description
At present, Signal behaves in the same way in the background as it does when you're using the application : it keeps the 2 websockets connected (unidentified and identified). But only the identified one is necessary. Having the unidentified websocket connected is a useless resource consumption, and if it fails, then it restart the websockets for nothing.
This patch change the behavior of the IncomingMessageObserver to close the unidentified websocket when it goes in background, after a timeout. This will reduce the number of reconnection, and the number of websocket messages, and the resources used by Signal.
When the user needs the unidentified socket, it connects again.
Before, you can see the unidentified websocket in use:
After, you can see the unidentified websocket being closed. It is open again when I open a conversation:
Edit: I've force-pushed a change, the check in mayDisconnectUnidentified wasn't correct