Allow the user to select their SD card and other external storage paths in addition to emulated SD storage

[shawn99452]

Allow choosing from all returned external writable storage locations for article and image cache, instead of just the first one; show the path for each returned location so the user can decide which one is best for them. On many devices, selecting "External Storage" ended up choosing a path pointing to the emulated external storage Android provides, which on devices with small internal memory and with large wallabag databases can take up significant space. This change allows those users to choose their SD card instead for storage.

[di72nn]

Hi, @shawn99452! Thank you for your contribution! I haven't had the time and a proper set up to work with android stuff recently, but I hope to improve the situation in the near future and review this.

[shawn99452]

No problem. I've been using a debug version of the app on my e-ink device since posting this change a few months ago, and it's been working fine. I'm not sure if the UI is particularly user-friendly as far as making it obvious what each selection means, but the underlying changes seem sensible, and Android doesn't provide any way to tell which path is emulated and which path(s) are real SD cards, so I didn't want to do anything hacky like string comparisons on the returned path to make it look nicer in the UI.

[Strubbl]

Thanks for the reminder. But we need the help of @tcitworld to get this PR into a release.

[shawn99452]

Thanks for the reminder. But we need the help of @tcitworld to get this PR into a release.

Yeah, I've been using the debug build myself for months, and recently added that .apk it as a pre-release in my fork's Releases, since at least one other person was having the problem I was, and didn't have a way to build it for themselves. I'd rather use the official Play Store version, but the debug build suffices until it gets merged/released.

[Strubbl]

Yes, i am also using a debug APK with all my open PR included, because development/release stalled a bit.

[norweeg]

Thanks for the reminder. But we need the help of @tcitworld to get this PR into a release.

Yeah, I've been using the debug build myself for months, and recently added that .apk it as a pre-release in my fork's Releases, since at least one other person was having the problem I was, and didn't have a way to build it for themselves. I'd rather use the official Play Store version, but the debug build suffices until it gets merged/released.

I think that person was me 😅. I did eventually build it myself. Is there anything I can do to test/help this PR get merged faster?? Also, is there a way with these changes to know which path the Play Store version of the app would have chosen as the SD card location? Just wondering if I can tell where on my device it was putting the database before?

[Strubbl]

Merging is one thing. There are some other PRs of which are ready to merge, but need to be merged e.g. by @tcitworld or @di72nn. After that, only @tcitworld can build a release for GPlay.

[shawn99452]

Also, is there a way with these changes to know which path the Play Store version of the app would have chosen as the SD card location?

The first path it shows in the list should be the one it would have chosen if you selected "External" before I added the feature. It seems to list the emulated SD storage first, and then lists the real SD storage path(s) after. On at least some devices, the path also contains the word "emulated" for the emulated SD storage.

[di72nn]

I've started reviewing this. I want some changes to be made, but I don't want to explain all the stuff, so I'll just do them myself. I'll try to find the time to finish this in the coming days.

@shawn99452 sorry for the wait and thanks again for your contribution!

[norweeg]

I can confirm that yes, the emulated SD card appears first on the list of available database storage locations, so the current play store version of the app was misleading me into believing I was freeing internal storage when I wasn't and that, having built the app myself from the pull-requested branch, it works perfectly. If I might make a suggestion: don't even offer to move the database if the only storage targets use internal storage

[shawn99452]

If I might make a suggestion: don't even offer to move the database if the only storage targets use internal storage

You unfortunately can't do this. I wanted to do this to make it clear to people which path was the 'fake' one. Android doesn't provide any way to know whether the external storage path is actually "external" or not. You can look at the path containing "emulated" but not every device works that way, it that could produce false negatives (or in weird cases false positives). All Android has is "give me the internal path" and "give me a list of external paths". It doesn't offer any other information about them other than the path, which is the mount point and doesn't let you know how the path is mounted or anything.

[di72nn]

Sorry for the delay again.

I slightly rewrote the original commits and added some more changes:

• I don't think the READ_EXTERNAL_STORAGE permission is needed (also, affects only Android ~5 devices), so I removed it.
• I'm not sure where that "external" value come from, so I removed it.
• Changed/fixed some logic.

The "database location" path is selected in the app settings. The external location (for image cache, queue and log saving) path is chosen as follows:

• If the DB location is set to a non-default (internal) location, that path is chosen.
• Otherwise, the first external location is chosen (same as before the PR).

Things to note:

• The option is still called "database location" in the UI, which should be updated in some way.
• Switching the location does not move the image cache data.

I still think this is a niche option, so I don't mind merging this as is and deal with the aforementioned issues sometime later (if required).

@shawn99452 thank you for you contribution!

[shawn99452]

Cool; will hopefully be able to switch back to the Play Store version when it updates. To be clear, it doesn't move the image cache data when you change the option, but it does USE the new location for the image cache; probably the right way to really do this is to move the files (could take a LONG time and be a pain) or to delete the image cache and have some kind of warning that changing the setting will clear the image cache. Probably not worth the effort, since it is indeed a niche option - but on my device it's required to get the application to use my SD card, and I think a few other people had reported it too, so it's nice to have.

[di72nn]

Rebased to resolve a conflict with translations.

To be clear, it doesn't move the image cache data when you change the option, but it does USE the new location for the image cache

Yes, that is correct.

probably the right way to really do this is to move the files (could take a LONG time and be a pain) or to delete the image cache and have some kind of warning that changing the setting will clear the image cache. Probably not worth the effort, since it is indeed a niche option

Agree.