Google Drive daily backup mobile data option #3083

Navid200 · 2023-09-12T03:25:08Z

This allows user to limit daily backups to WiFi and exclude mobile data.

jamorham · 2023-09-12T18:18:45Z

Will this default to not allowing backup via mobile data?

Navid200 · 2023-09-12T19:44:27Z

@jamorham Thanks for looking at this and thanks for catching that.
It is now unchecked by default so we only use WiFi by default.
Only if user checks this, will it also upload using mobile data.

Thanks

jamorham · 2023-09-13T08:06:56Z

Would having the preference disabled by default change the default behavior though, so if someone installs a new version then it will stop backing up over mobile data where as it prior to this change by default it would have done?

I don't think changing the UI checked flag is the way to set the default value to true because as far as I know the value is only written to preference store on change.

Navid200 · 2023-09-13T13:00:50Z

@jamorham
When I look at cloud_storage_api_use_mobile, I see nothing other than what I have done here.
Would you please tell me why the same concern does not exist for that?

jamorham · 2023-09-13T21:56:52Z

To answer your question: Because it defaults to true and is handled via a preferences xml rather than a databinding activity.

There are a couple of different ways you can handle this issue depending of course on whether you think the existing behavior should be maintained which in this case I think it probably should.

Either invert the logic of the preference so that you can have it defaulting to false but that is a bit backward or when you read it in Backup you can default it to true if not set and the in the UI component you can initialize it if not set. You can use Pref.isPreferenceSet() to check if its set and if not set it to true.

This needs to happen before the screen starts rendering as I don't think the prefsview is observable. To keep it clean you want encapsulate that behavior and so I think create a static public method in Backup.java to do this and then call it from the view model initialization block where currently it is already doing clear()

Otherwise we could extend the prefsview to perhaps understand defaults but that is potentially a bigger change but the main reason you're having this problem is it doesn't support unset booleans being true.

One other way to achieve the same is to actually define your setting inside the regular preferences as then it will go through an initialization stage but as far as I'm aware (without checking) all of these backup settings are handled within the backup selector activity and there is no place in the preferences xml for related settings. But I'm just giving you an overview of the constraints and various possible solutions.

Navid200 · 2023-09-13T23:12:30Z

Thanks for the guidance.
I chose the first option. I hope I didn't misunderstand.

Navid200 · 2023-09-13T23:13:08Z

jamorham · 2023-09-15T06:55:31Z

app/src/main/res/layout/activity_backup_picker.xml

@@ -5,7 +5,7 @@

        <variable
            name="vm"
-            type="com.eveningoutpost.dexdrip.cloud.backup.BackupActivity.ViewModel" />
+            type="com.eveningoutpost.dexdrip.cloud.backup.BackupActivity.ViewModel" />Changed default to true


Is this supposed to be here?

No, sorry about that

jamorham · 2023-09-15T17:48:14Z

app/src/main/res/layout/activity_backup_picker.xml

+            <CheckBox
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:checked="true"


I think this checked = true could interfere with the one which follows the preference

What do you mean by "the one which follows the preference"?

I thought that's how I can have it enabled by default.

Do you mean "False" in this?
Pref.getBooleanDefaultFalse(PREF_AUTO_BACKUP_WIFI)

Navid200 · 2023-09-15T18:22:53Z

Can I do this then?

jamorham · 2023-09-16T09:10:33Z

Well I think this is getting very confusing now. The whole point of you inverting the logic was so that you could default the preference to being true. Am I to understand it that you want the checkbox to show checked when the preference is false? If so you can often invert the logic on display but this is a two way binding so I'm not sure about that. Surely from my previous comment you just remove it being checked and let it follow the preference which should default to false and the checkbox should be unchecked by default representing false?

Navid200 · 2023-09-16T11:34:55Z

No,
The preference is true by default because I thought that was required.
To make it true, it is Use WiFi only instead of use mobile.

Navid200 · 2023-09-16T11:47:03Z

If someone doesn't mind uploading using mobile, they need to disable the new setting. If they leave it at default, it will only upload when there is WiFi. If there is no WiFi, it will not upload the daily backup by default.

This is the opposite of REST_API, where by default, it uses mobile and one has to disable it if they want WiFi only.

Navid200 · 2023-09-16T12:06:11Z

Let's say I have a service that charges a lot for mobile data and I want to minimize mobile data use.
After this PR, I will have to leave the new setting WiFi only enabled, which is enabled by default.

Navid200 · 2023-09-16T12:07:58Z

I created a new method that sets the preference's default to true.

jamorham · 2023-09-22T16:17:42Z

To be clear, I would prefer not to change the existing default behavior. If I understand this correctly (and there have been changes and possibly double negatives) then merging this will cause anyone who isn't using wifi to cease having backups occur. I would prefer that the default remains as it is and when users are setting up the backup they can make it wifi only if they want.

Navid200 · 2023-09-22T19:00:55Z

I misunderstood. Thanks. I now understand you don't want me to change the default to not use mobile data.

My database backup size, on Google Drive, is 35MB.
That's several orders of magnitude greater than the data size used by xDrip uploading readings to Nightscout.

35MB x 30 > 1GB

There are individuals who subscribe to cell phone services with limited data amount.
I thought if someone installs xDrip and enables daily backup and find out at the end of the month that they have to pay a lot, they may be disappointed in xDrip.
That's why I would like to change the default.

Looking at the other side of the coin, if you approve this PR, I can see someone can install xDrip and enable daily backups but miss the clear note "WiFi only". In 2 months, their phone breaks. They buy a new phone and go to Google drive and see no backup because they never had access to WiFi over the past 2 months.
That's a disaster I suppose your preference will avoid.

I see both of those as bad scenarios we should avoid.
Will you consider the following workaround?

I will add a notification that comes up when someone enables daily backups. It will say something like "xDrip by default will use WiFi only for daily backups. If you want to also use mobile data for daily backups, please disable the setting WiFi only".
This will be a notification that will need to be dismissed.

Will you consider that?

Navid200 · 2023-09-23T22:57:29Z

If the only way to move forward with this is to have mobile data enabled by default, can I add a notification when daily backup is enabled to inform the user that by default mobile data will be used?

Navid200 · 2023-09-24T14:37:47Z

Mobile data is used by default.
I can open another PR later for adding a notification.

jamorham · 2023-09-30T09:17:47Z

app/src/main/res/layout/activity_backup_picker.xml

+                android:gravity="center"
+                android:layout_marginBottom="20dp"
+                android:text="@string/daily_backup_even_mobile"
+                app:checked="@={prefs[`backup-automatic-mobile`]}"


I still don't think this will work properly because even though you've changed the logic to default to true, the prefsview I don't think will handle that and will default to false and without a significant rewrite of that component which carries its own risks I don't think its easily solvable. I could take a look at that though as it would be better if we were not constrained by that. For other activities we rely on the XML settings to set the defaults.

For things as they are, the only alternative options are I think you either need to set the setting to true if it isn't currently set so that new users get this defaulted true setting or invert your logic so that it is something like "Back up over WiFi only" which then defaults to false and then you don't need to worry about presetting it to true.

I will take a look at PrefsView and get back to you on whether its easily solvable that way.

Navid200 · 2023-09-30T19:36:31Z

Mobile data is used by default.
The new setting (WiFi only) is unchecked by default.

jamorham · 2023-10-06T16:52:41Z

See af3a008 this should allow you to specify like prefs[`backup-automatic-mobile:true`] to default a prefs view item to true if it has never been set.

jamorham · 2023-10-07T18:51:52Z

app/src/main/res/layout/activity_backup_picker.xml

@@ -111,11 +111,12 @@
                android:layout_width="wrap_content"
                android:layout_height="wrap_content"
                android:dependency="backup-automatic-enabled"
+                android:checked="true"


this shouldn't be specified as it should be handled by the prefsview

Navid200 · 2023-10-17T12:55:07Z

Should I squash the commits?

Google Drive daily backup mobile data option

1c0952f

Unchecked by default

39f9bff

Flipped the logic (true default)

7be19c5

jamorham reviewed Sep 15, 2023

View reviewed changes

Cleanup

428b411

jamorham reviewed Sep 15, 2023

View reviewed changes

getBooleanDefaultTrue

95644dd

Navid200 and others added 2 commits September 24, 2023 09:06

Merge branch 'NightscoutFoundation:master' into Navid_2023_09_11

deb7b0e

Use mobile data by default

e821019

jamorham reviewed Sep 30, 2023

View reviewed changes

Disabled by default

ead6c35

Navid200 and others added 2 commits October 6, 2023 15:15

Merge branch 'NightscoutFoundation:master' into Navid_2023_09_11

87f8475

Default True prefview

680aa96

jamorham reviewed Oct 7, 2023

View reviewed changes

Removed checked from the mobile data setting

f1c3f6a

jamorham approved these changes Oct 24, 2023

View reviewed changes

jamorham merged commit 286323e into NightscoutFoundation:master Oct 24, 2023
1 check passed

Navid200 deleted the Navid_2023_09_11 branch October 26, 2023 12:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Google Drive daily backup mobile data option #3083

Google Drive daily backup mobile data option #3083

Navid200 commented Sep 12, 2023

jamorham commented Sep 12, 2023

Navid200 commented Sep 12, 2023

jamorham commented Sep 13, 2023

Navid200 commented Sep 13, 2023

jamorham commented Sep 13, 2023

Navid200 commented Sep 13, 2023

Navid200 commented Sep 13, 2023

jamorham Sep 15, 2023

Navid200 Sep 15, 2023

jamorham Sep 15, 2023

Navid200 Sep 15, 2023

Navid200 Sep 15, 2023

Navid200 Sep 15, 2023

Navid200 commented Sep 15, 2023

jamorham commented Sep 16, 2023

Navid200 commented Sep 16, 2023

Navid200 commented Sep 16, 2023 •

edited

Loading

Navid200 commented Sep 16, 2023

Navid200 commented Sep 16, 2023 •

edited

Loading

jamorham commented Sep 22, 2023

Navid200 commented Sep 22, 2023

Navid200 commented Sep 23, 2023

Navid200 commented Sep 24, 2023

jamorham Sep 30, 2023

Navid200 commented Sep 30, 2023

jamorham commented Oct 6, 2023

jamorham Oct 7, 2023

Navid200 commented Oct 17, 2023

Google Drive daily backup mobile data option #3083

Google Drive daily backup mobile data option #3083

Conversation

Navid200 commented Sep 12, 2023

jamorham commented Sep 12, 2023

Navid200 commented Sep 12, 2023

jamorham commented Sep 13, 2023

Navid200 commented Sep 13, 2023

jamorham commented Sep 13, 2023

Navid200 commented Sep 13, 2023

Navid200 commented Sep 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Navid200 commented Sep 15, 2023

jamorham commented Sep 16, 2023

Navid200 commented Sep 16, 2023

Navid200 commented Sep 16, 2023 • edited Loading

Navid200 commented Sep 16, 2023

Navid200 commented Sep 16, 2023 • edited Loading

jamorham commented Sep 22, 2023

Navid200 commented Sep 22, 2023

Navid200 commented Sep 23, 2023

Navid200 commented Sep 24, 2023

Choose a reason for hiding this comment

Navid200 commented Sep 30, 2023

jamorham commented Oct 6, 2023

Choose a reason for hiding this comment

Navid200 commented Oct 17, 2023

Navid200 commented Sep 16, 2023 •

edited

Loading

Navid200 commented Sep 16, 2023 •

edited

Loading