Skip to content
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

Disconnect on Cordova Pause and Reconnect on Cordova Resume #3808

Merged
merged 7 commits into from
Aug 25, 2016

Conversation

graywolf336
Copy link
Contributor

@RocketChat/core

Closes: RocketChat/Rocket.Chat.Cordova#112

This basically listens for Cordova's pause event and disconnects from the server thus saving background battery usage. Then when Cordova fires resume event we reconnect to the server. This will cause a little bit of delay, but I feel that delay is worth the extra battery life saved.

@marceloschmidt
Copy link
Member

Does push notification still work after that?

@TheReal1604
Copy link
Contributor

@marceloschmidt After disabling the mobile background data usage for the rocket.chat app on Android the mobile push notifications work like before. (Because the push notifications come from a system service..)

@sampaiodiego
Copy link
Member

My only concern is about this.

I think we should call disconnect() and reconnect() on the existing callbacks to avoid a race condition causing the user not being set away.

@rodrigok
Copy link
Member

@graywolf336 did you test this? Solved the battery problem?

@graywolf336
Copy link
Contributor Author

@sampaiodiego I agree, I didn't see that last night when I looked for existing ones. I will move it to there.

@rodrigok Not yet, I created this pull request so I could make usage of the heroku deployment.

@sampaiodiego
Copy link
Member

@graywolf336 you forgot to change the package.js file and change the filename 😉

@graywolf336
Copy link
Contributor Author

graywolf336 commented Jul 19, 2016

That's what I get for trying to do it fast before I left for work. 👎

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-3808 July 19, 2016 14:15 Inactive
@engelgabriel
Copy link
Member

engelgabriel commented Jul 19, 2016

We should set a timer, for 5 minutes, before disconnecting. If the user is just going to another app to get some info, he can come back quickly. The 5 minutes can be a user's config.

@rodrigok
Copy link
Member

@engelgabriel maybe 1 minute?

@graywolf336
Copy link
Contributor Author

@engelgabriel I agree with @rodrigok, one minute is probably good enough by default to increase the battery saving this hopefully will enable.

@engelgabriel
Copy link
Member

Agreed, 1 minute should be fine. Can you do that @graywolf336?

@graywolf336
Copy link
Contributor Author

Yes I will take a look at adding that this week.

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-3808 August 24, 2016 19:30 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-3808 August 24, 2016 19:30 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-3808 August 24, 2016 19:31 Inactive
@graywolf336
Copy link
Contributor Author

@RocketChat/core Thoughts on this now?

@engelgabriel
Copy link
Member

Should we add a issue to turn the time into a setting @graywolf336 ?

@TheReal1604
Copy link
Contributor

Nice! Thanks for your amazing work. @graywolf336 @engelgabriel

@graywolf336
Copy link
Contributor Author

@engelgabriel Yeah we could, probably should be a personally setting though.

@Sing-Li
Copy link
Member

Sing-Li commented Aug 25, 2016

Thanks for keeping the world green , @graywolf336 !

@marceloschmidt marceloschmidt modified the milestone: 0.38.0 Sep 5, 2016
@kevincolten
Copy link

I like @graywolf336 's idea, this should be an admin setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve battery usage
8 participants