Attempt to solve: Option to disable auto sync with metered connection…

[Cedarus777]

Purpose / Description

I add a method which can be helpful to the feature: add an option to whether do sync in mobile data.

Fixes

Fixes #10706

Approach

I add a new method isWifiConnected() in async.Connection. But I haven't add an option to it, hope others can do that. The method can be called in anki.DeckPicker to decide whether to sync during auto-sync, only in WIFI environment can anki process auto sync.

How Has This Been Tested?

The method has not been called yet so it does not have to be tested.

Learning (optional, can help others)

Describe the research stage

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

• You have not changed whitespace unnecessarily (it makes diffs hard to read)
• You have a descriptive commit message with a short title (first line, max 50 chars).
• Your code follows the style of the project (e.g. never omit braces in if statements)
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[welcome]

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

[Arthur-Milchior]

Hi @Cedarus777, nice to meet you.
Thanks a lot for contributing.

I'm very sorry if I'm asking plenty of changes simultaneously, I perfectly understand you are new here and you're attacking a complex issue, which led to plenty of request.

Usually, when an implementation depends on the API level, we add the method in com.ichi2.compat.Compat. Then you add in CompatV21 the implementation for V21 and up, and in compatV29 you add the implementation starting at V29.
Hopefully, it should even avoid adding an extra "deprecated" suppression, since it's clear in CompatV21 that it is for V21 API.

Regarding your commit message. You must imagine that someone, looking at the history of the file, will read the first 80 characters only and, from that, have to decide whether it's the commit they want to look at. So "Attempt to solve" is taking useless space here. Plus, if we accept to merge it, it means we are relatively convinced that you are indeed solving it. In this case

adding isWifiConnected in Compat

would be a good first line
On top of that, I'd also love your commit message to explain how you did test it. How did you ensure that it is indeed working as expected. Ideally, I'd love unit test or emulator test, but I fear I don't know this domain well enough to actually do them.

Regarding your PR message. You are not fixing this bug. You are helping fixing it clearly, but there remains a lot of work to ensure that we have a setting to determine whether auto sync is done on metered and take it into account. So can you edit your PR message to state just "bug: #10706"

Last issue is that your method only consider wifi. While the bug considered "metered connection". Admittedly, a good heuristic is that wifi is not and otherwise it is, but that's not what was asked. If you can't do better than wifi, I'd like that you add some comment on top of the function, to explain why you can't do better. What you tried/looked at/read. Ensuring that if someone wants to improve it later, they don't repeat your work and they don't have to understand by themselves why it can't be improved

[github-actions]

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically