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

Lock device during refresh re-launch another refresh #464

Open
Jean-BaptisteC opened this issue May 19, 2024 · 14 comments
Open

Lock device during refresh re-launch another refresh #464

Jean-BaptisteC opened this issue May 19, 2024 · 14 comments
Labels
help wanted Extra attention is needed

Comments

@Jean-BaptisteC
Copy link
Contributor

During the first time, when the app refreshes data, if the device is locked and users unlock the device. The coroutines are re-launched.
We need to find a way to not re-launch coroutines when the device is unlocked

@Jean-BaptisteC Jean-BaptisteC added the help wanted Extra attention is needed label May 19, 2024
@Jean-BaptisteC Jean-BaptisteC changed the title Lock device during refresh can re-launch another Lock device during refresh can re-launch another refresh May 19, 2024
@Jean-BaptisteC Jean-BaptisteC pinned this issue May 20, 2024
@Iamlooker
Copy link
Contributor

Hi, if this issue is unassigned, I would like to work on this.

I looked into the code for about 30 mins and I think this might be one of the root causes of the issue. But I am not sure, I think we can remove the updateReport from these two functions: 1, 2

The mutability of the the IS_RUNNING function can be causing the issue or it could be the LifecycleService I can do a thorough research on it once I get a green flag 👀

I also went through some other parts of the codebase and found a huge dependency on MutableList and other mutable variables (which are not using the full potential of the coroutines), as the project is leaning towards coroutines this could lead to race situations. Also the cancellation is not handled appropriately

I think the performance can be improved significantly, but thats for another issue to handle. But just for context one thing which can be improved is, the notification shows 60/130 fetched, but none are reflected to database immediately.

@Jean-BaptisteC
Copy link
Contributor Author

Jean-BaptisteC commented Jun 2, 2024

Okay, do you think setupSwipeRefreshLayout method is called when we unlock the device?
Don't ask to work on issue, open PR if you want :)
It can be cool if you improve performance, some users complain refresh takes too much time

@Iamlooker
Copy link
Contributor

Yes, I think it is been called when the fragment is resumed, but I need to test it personally

@Iamlooker
Copy link
Contributor

In my recent tests I am unable to reproduce this exact issue in last few days. But I can help with improving performance in the app. I will work on this and create a PR asap

@Iamlooker
Copy link
Contributor

I have requested API key via mail for testing. I have done some drastic changes and will push them once I am done testing :)

@Iamlooker
Copy link
Contributor

Most changes are done, some testing is still needed

@Jean-BaptisteC
Copy link
Contributor Author

FIY, api_key is integrated in build debug workflow

@Iamlooker
Copy link
Contributor

I willtest and get be soon

@codeurimpulsif
Copy link
Contributor

@Iamlooker Hey, do you still need an API key?

@Iamlooker
Copy link
Contributor

@Iamlooker Hey, do you still need an API key?

Sorry I don't think I will be able to test for few hours. I will revert back once I am back to workstation 😃

@Iamlooker
Copy link
Contributor

This is the error I am getting on my debug build

21:30:47.054 ExodusAPIRepository      D  Attempting download of app details on com.google.android.gm.
21:30:47.287 ExodusAPIRepository      W  Failed to get app details, response code: 401. Returning emptyList.

I have not made any changes to ExodusAPIRepository yet

@Jean-BaptisteC
Copy link
Contributor Author

Jean-BaptisteC commented Jun 8, 2024

Yes, I have same behaviour, I don't know where is the problem, maybe the API (max limit requests API?)
I suggest you to retry later

@Jean-BaptisteC
Copy link
Contributor Author

Jean-BaptisteC commented Jun 10, 2024

I continue to have refresh starting when I unlock device with your changes :(
FIY I use Pixel 6 with Android 14

@Jean-BaptisteC
Copy link
Contributor Author

I have find a way to reproduce:

  • Start app for the first time
  • Accept pop-up and notifications
  • Doing nothing and wait device sleep and refresh is done
  • Unlock device -> Refreshed was start for the second time

@Jean-BaptisteC Jean-BaptisteC changed the title Lock device during refresh can re-launch another refresh Lock device during refresh re-launch another refresh Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants