LBRYCommunity/lbry-android
1
0
Fork 0
Code Issues 155 Pull requests 6 Projects Releases 84 Packages Wiki Activity

Switch from background service to some system-wide Android feature #725

New issue
Open
opened 2019-10-20 22:06:06 +02:00 by kekkyojin · 5 comments
kekkyojin commented 2019-10-20 22:06:06 +02:00 (Migrated from github.com)
Copy link

The Issue

Currently LBRY on Android creates a foreground service reportedly to provide a better user experience. That allows the app to load data even when the app is not on the foreground. This is a complete bad practice.

Foreground services must only be used when the user can notice if it is running. Typical examples are music or video players. Background services can be used for features which user cannot notice directly.

The feature which LBRY tries to provide can (and should) be implemented by a background service. Android is already offering a few options for this:

  • Sync Adapters, which are called by the system itself at least every 15 minutes, even if the app is not running
  • Simple background service, which can be developed in a way where the system will ask it to do its jobs in a periodic way. If using WorkManager API, the system will even delay the call if there is no network available

Expected behaviour

Don't use a foreground service

Actual behaviour

App is using a foreground service, even user is not going to notice it is running. Notification shows it is running but that's not the way foregrounbd services should be used on Android.

Anything Else

I propose a roadmap where first current implementation is moved into a background service which uses WorkManager to get the system ask it to do the background sync. As currently LBRY is already using accounts, the next step would be to implement a Sync Adapter, which would just leverage the previous phase but moving the preference settings into the Android native Account Manager.

EDIT: This would only affect the app while not on the foreground. The LbrynetService would still be syncing as it is already doing while LBRY is being shown on the screen. App would switch to the WorkManager when user is not seeing it on the screen.

I could be working on this in the future, if it is seen as something worth to be done. I would fork this repo and do the corresponding PRs.

Internal Use

Acceptance Criteria

Definition of Done

  • Tested against acceptance criteria
  • Tested against the assumptions of the user story
  • The project builds without errors
  • Unit tests are written and passing
  • Tests on devices/browsers listed in the issue have passed
  • QA performed & issues resolved
  • Refactoring completed
  • Any configuration or build changes documented
  • Documentation updated
  • Peer Code Review performed
<!-- Thanks for reporting an issue to LBRY and helping us improve! To make it possible for us to help you, please fill out below information carefully. Before reporting any issues, please make sure that you're using the latest version. We are also available on live chat at https://chat.lbry.com --> ## The Issue Currently LBRY on Android creates a foreground service reportedly to provide a better user experience. That allows the app to load data even when the app is not on the foreground. This is a complete bad practice. Foreground services must only be used when the user can notice if it is running. Typical examples are music or video players. Background services can be used for features which user cannot notice directly. The feature which LBRY tries to provide can (and should) be implemented by a background service. Android is already offering a few options for this: * Sync Adapters, which are called by the system itself at least every 15 minutes, even if the app is not running * Simple background service, which can be developed in a way where the system will ask it to do its jobs in a periodic way. If using WorkManager API, the system will even delay the call if there is no network available ### Expected behaviour Don't use a foreground service ### Actual behaviour App is using a foreground service, even user is not going to notice it is running. Notification shows it is running but that's not the way foregrounbd services should be used on Android. ## Anything Else <!-- Include anything else that does not fit into the above sections --> I propose a roadmap where first current implementation is moved into a background service which uses WorkManager to get the system ask it to do the background sync. As currently LBRY is already using accounts, the next step would be to implement a Sync Adapter, which would just leverage the previous phase but moving the preference settings into the Android native Account Manager. EDIT: This would only affect the app while not on the foreground. The LbrynetService would still be syncing as it is already doing while LBRY is being shown on the screen. App would switch to the WorkManager when user is not seeing it on the screen. I could be working on this in the future, if it is seen as something worth to be done. I would fork this repo and do the corresponding PRs. ## Internal Use ### Acceptance Criteria 1. 2. 3. ### Definition of Done - [ ] Tested against acceptance criteria - [ ] Tested against the assumptions of the user story - [ ] The project builds without errors - [ ] Unit tests are written and passing - [ ] Tests on devices/browsers listed in the issue have passed - [ ] QA performed & issues resolved - [ ] Refactoring completed - [ ] Any configuration or build changes documented - [ ] Documentation updated - [ ] Peer Code Review performed
👍 2
tzarebczan commented 2019-10-21 19:16:08 +02:00 (Migrated from github.com)
Copy link

@kekkyojin Thanks so much for opening this issue and the feedback. We'll need to see if this is possible for our use case while running the LBRY-sdk in the background.

Can we show you some appreciation for the detailed issue?

@kekkyojin Thanks so much for opening this issue and the feedback. We'll need to see if this is possible for our use case while running the LBRY-sdk in the background. Can we show you some [appreciation](https://lbry.com/faq/appreciation) for the detailed issue?
foooorsyth commented 2020-08-17 08:40:20 +02:00 (Migrated from github.com)
Copy link

LbrynetService is just starting lbry.extras.daemon.Daemon from the SDK. This action in a vacuum is not compatible with SyncAdapter/WorkManager -- it's ongoing by nature so there's no way to do it without a foreground Service.

The question is whether or not this is even necessary. What is the app even doing that requires the Daemon when the app is in the background (true background -- not playing a video in PIP mode)? The Android app doesn't host videos, so why does the Daemon need to continue to run (serious question -- I don't know the app codebase or the protocol well)? Why not just stop the Service if you're not going into PIP mode? Without any real benchmarks, I'd say the main concern here is battery drain. I would imagine that maintaining a Python event loop through Kivy at all times is a great way to burn up your battery. It would not be abnormal for Apple's (human) App Store verification to block something like that.

@akinwale could you comment on what the Daemon is needed for when the app enters the background without a video playing in PIP? Can the LbrynetService simply be stopped in this case? Thanks for any info!

`LbrynetService` is just starting `lbry.extras.daemon.Daemon` from the SDK. This action in a vacuum is not compatible with SyncAdapter/WorkManager -- it's ongoing by nature so there's no way to do it without a foreground Service. The question is whether or not this is even necessary. What is the app even doing that requires the Daemon when the app is in the background (true background -- not playing a video in PIP mode)? The Android app doesn't host videos, so why does the Daemon need to continue to run (serious question -- I don't know the app codebase or the protocol well)? Why not just stop the Service if you're not going into PIP mode? Without any real benchmarks, I'd say the main concern here is battery drain. I would imagine that maintaining a Python event loop through Kivy at all times is a great way to burn up your battery. It would not be abnormal for Apple's (human) App Store verification to block something like that. @akinwale could you comment on what the Daemon is needed for when the app enters the background without a video playing in PIP? Can the LbrynetService simply be stopped in this case? Thanks for any info!
kekkyojin commented 2020-08-18 02:41:19 +02:00 (Migrated from github.com)
Copy link

@aimkey That daemon could be rewritten into a SyncAdapter. The new SyncAdapter would download data every 15 minutes. When LBRY Android is in the foreground, the app could be requesting the SyncAdapter to download new data with a blocktime period so it is on sync.

@aimkey That daemon could be rewritten into a SyncAdapter. The new SyncAdapter would download data every 15 minutes. When LBRY Android is in the foreground, the app could be requesting the SyncAdapter to download new data with a blocktime period so it is on sync.
akinwale commented 2020-08-19 16:22:37 +02:00 (Migrated from github.com)
Copy link

The SDK is a Python process which performs a number of tasks while in the background:

  • Staying synchronised with the blockchain
  • Handling wallet transactions (send and receive LBC, send tips and supports)
  • Download files
  • Publish files
  • Streaming media files
  • Resolve claims on the blockchain (in certain scenarios)

Due to the amount of time that it takes for the sdk to get to a ready state in order to start handling requests, we keep it running in the background in order to improve the user experience when they close and reopen the app.

We stuck with Python (and Kivy) because it would be a time-consuming and expensive endeavour to reimplement the required bits of https://github.com/lbryio/lbry-sdk in native code.

It is possible to delegate the tasks listed above to lbry.tv but that leads to centralisation. It's something we're considering for a better experience, but we want to provide users with an option to choose.

The SDK is a Python process which performs a number of tasks while in the background: * Staying synchronised with the blockchain * Handling wallet transactions (send and receive LBC, send tips and supports) * Download files * Publish files * Streaming media files * Resolve claims on the blockchain (in certain scenarios) Due to the amount of time that it takes for the sdk to get to a ready state in order to start handling requests, we keep it running in the background in order to improve the user experience when they close and reopen the app. We stuck with Python (and Kivy) because it would be a time-consuming and expensive endeavour to reimplement the required bits of https://github.com/lbryio/lbry-sdk in native code. It is possible to delegate the tasks listed above to lbry.tv but that leads to centralisation. It's something we're considering for a better experience, but we want to provide users with an option to choose.
kauffj commented 2020-08-21 22:44:33 +02:00 (Migrated from github.com)
Copy link

you've made it into our chat 😁

sync_adapter_sdk

sounds like a bit of time before we'd be able to make the adjustments SDK side

you've made it into our chat :grin: ![sync_adapter_sdk](https://user-images.githubusercontent.com/530774/90933431-848f3500-e3cd-11ea-953f-7b88c6401b05.png) sounds like a bit of time before we'd be able to make the adjustments SDK side
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
dep-update
test-deploy
notification-debugging
commenting
crash-fixes
user-bugs
unlock-tips
reward-drivers
visual-tweaks
dark-theme-fix
fullscreen-on-rotate
native-rewrite
0.17.1
0.17.0
0.16.14
0.16.13
0.16.12
0.16.11
0.16.10
0.16.9
0.16.8
0.16.7
0.16.6
0.16.5
0.16.4
0.16.3
0.16.2
0.16.1
0.16.0
0.15.16
0.15.15
0.15.14
0.15.13
0.15.12
0.15.11
0.15.10
0.15.9
0.15.8
0.15.7
0.15.6
0.15.5
0.15.4
0.15.3
0.15.2
0.15.1
0.14.3
0.15.0
0.14.2
0.14.1
0.14.0
0.13.4
0.13.3
0.13.2
0.13.1
0.13.0
0.12.3
0.12.2
0.12.1
0.12.0
0.11.1
0.11.0
0.10.2
0.10.1
0.10.0
0.9.3
0.9.2
0.9.1
0.9.0
0.8.4
0.8.3
0.8.2
0.8.1
0.8.0
0.7.5
0.7.4
0.7.3
0.7.2
0.7.1
0.7.0
0.6.2
0.6.1
0.6.0
0.5.0
0.4.0
0.3.1
0.3.0
0.2.3
0.2.2
0.2.1
0.2.0
0.1.10
0.1.9
0.1.8
0.1.7
0.1.6
0.1.5
0.1.4
0.1.3
0.1.2
0.1.1
Labels
Clear labels   
android: closed alpha

   
android: open beta

   
app-parity

   
area: devops

   
area: discovery

   
area: docs

   
area: livestream

   
area: proposal

Work is part of a proposal

  
consider soon

Discuss this issue at the next planning meeting, then remove this label

  
creator

   
Epic

   
good first issue

   
hacktoberfest

Welcome to Hacktoberfest

  
help wanted

   
icebox

Long-term storage

  
Invalid

   
level: 1

No knowledge of the existing code required

  
level: 2

Some knowledge of the existing code is recommended

  
level: 3

Significant knowledge of the existing code is recommended

  
level: 4

Intimate knowledge of the existing code is recommended

  
needs: exploration

Solution unclear, needs research

  
needs: grooming

Issue needs to be groomed before work can start

  
needs: priority

Priority level needs to be defined

  
needs: repro

Issue needs step-by-step instructions on how to reproduce in latest code

  
needs: tech design

Needs technical design signoff before implementation can begin

  
on hold

Temporarily paused

  
priority: blocker

Issue is blocking release, do ASAP

  
priority: high

Work needs to be moved into sprint ASAP

  
priority: low

Work should be done but can stay in the backlog for now

  
priority: medium

Work needs to be done within 2-3 sprints

  
product review

   
resilience

   
Tom's Wishlist

   
type: bug

Existing functionality is wrong or broken

  
type: discussion

A conversation. No specific changes requested

  
type: improvement

Existing (or partially existing) functionality needs to be changed

  
type: new feature

New functionality that does not exist yet

  
type: refactor

Minimal user-visible changes, but significant internal work

  
type: task

Either work that's not related to the code, or a small chore that does not fit into other categories

  
type: testing

Solution needs additional user testing

  
unplanned

Work that was not planned into the spirnt, took priority over planned work

No labels
android: closed alpha
android: open beta
app-parity
area: devops
area: discovery
area: docs
area: livestream
area: proposal
consider soon
creator
Epic
good first issue
hacktoberfest
help wanted
icebox
Invalid
level: 1
level: 2
level: 3
level: 4
needs: exploration
needs: grooming
needs: priority
needs: repro
needs: tech design
on hold
priority: blocker
priority: high
priority: low
priority: medium
product review
resilience
Tom's Wishlist
type: bug
type: discussion
type: improvement
type: new feature
type: refactor
type: task
type: testing
unplanned
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-android#725
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?