cgeo 🚀 - Rework storage framework acc. to Android 11 requirements

If I'm not mistaken this happens since long time ago. After a fresh install c:geo still points to internal storage, even with permissions granted, and only after completely removing c:geo from RAM (thus forcing a reload with permissions already set) this works. IIRC there is already an issue for this regarding not finding the backup db or so.

moving-bits on 15 Jun 2020

only after completely removing c:geo from RAM (thus forcing a reload with permissions already set) this works

Not all the time. See my scenario described here - tried to load a backup several times today, to no avail, although I removed c:geo from memory on every single try.

Although I have to admit (as I realize right now) that I have just removed it from "Recent Apps", but not stopped it via Settings -> Apps. May be this should be done additionally ...?

MagpieFourtyTwo on 15 Jun 2020

If I'm not mistaken this happens since long time ago.

Maybe after first install. But it stays like that even after restart.
I can clearly see a difference between the mentioned versions.

Also normally c:geo always asked to restore existing backups after installation, so it found them.
This is no longer the case now, which is also a followup of this problem.

Lineflyer on 15 Jun 2020

If I'm not mistaken this happens since long time ago.

Maybe after first install. But it stays like that even after restart.

Ok, that's new then. Needs to be debugged into.

Also normally c:geo always asked to restore existing backups after installation, so it found them.

This has not worked reliably for me in the past, at least not in the emulator.

moving-bits on 16 Jun 2020

If I understood the two articles (see below) right, then this seems like a bigger thing: Android switches file access rights from being able to access nearly all public directories (even if created and maintained by a different app) to an app-specific directory ("storage space") not accessible by other apps. Also access to common folders like "Downloads" is restricted. I'm not yet sure about the consequences this has for c:geo and how we need to adapt our app (and how we can do so in a cross-version compatible way).

See the following links for reference:

Need to dig in much deeper to look for our options and their respective consequences. Any help appreciated...

moving-bits on 16 Jun 2020

Without having read it in much detail, I think Google enforces, that apps don't clutter user storages with random data. I also hate it, that some apps just create random new dirs on my drive.
Since the last refactoring of this part of c:geo we however use a IMHO very sophisticated structure for all the data relevant to users to import/export/store.
If this really turns out to be no option anymore we would maybe require to rely on export functions for all the actions like backup, field-notes, etc.

@pstorch As you helped a lot with redefining the data storage logic in the past....if you are interested or happen to have time you are welcome to help in any possible way.
Anyhow I think we need to understand first, what these changes mean for c:geo...e.g. when upgrading the user storage is kept.

Lineflyer on 16 Jun 2020

e.g. when upgrading the user storage is kept.

Yep - but as this will not be the case for new installations (as far as I understood), downgrading (i. e. remove and re-install) could become a problem, as this will implicitly change to scoped storage, thus the downgraded version may probably not be able to access previously saved backups of db and settings.

As for the app specific storage: After a first quick glance I would understand it like a rights model: You still can create folders (which BTW will not hinder cluttering, although at least may redirect it to special sub folders), but your app id will be the only one which has the right to access this folder. Except applications that are allowed to access all of them, like (hopefully) e. g. CX/ES File Explorer & Co (in order to handle e. g. backup history).

MagpieFourtyTwo on 17 Jun 2020

An app should not ask for the external file permission anymore. Newer Android versions will move away from this. Apps should either use the internal App specific storage or use the Storage Access Framework to get access to specific directories selected by the user outside the App scope.

pstorch on 17 Jun 2020

Apps should either use the internal App specific storage or use the Storage Access Framework to get access to specific directories selected by the user outside the App scope.

So backups will still work, if the user grants access to a particular folder to c:geo, right?
This sounds to me like it is right now, already (in Android 10), and they "just" change "Grant access to external storage" to "Grant access to a special folder on external storage" ...?

MagpieFourtyTwo on 18 Jun 2020

There is no permission in the manifest needed anymore, the app need to request a directory like this:

protected void requestBackupDirectory() {
        final Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT_TREE);
        intent.addFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
        startActivityForResult(intent, REQUEST_CODE_BACKUP_DIR);
}

@Override
public void onActivityResult(final int requestCode, final int resultCode, final Intent resultData) {
        super.onActivityResult(requestCode, resultCode, resultData);
        if (resultCode == Activity.RESULT_OK && resultData != null && requestCode == REQUEST_CODE_BACKUP_DIR) {
            final Uri uri = resultData.getData();
            if (uri != null) {
                getContentResolver().takePersistableUriPermission(uri,Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
                // do something with the Uri or convert it to a pseudo file object:
                DocumentFile file = DocumentFile.fromTreeUri(context, uri);
            }
        }
}

You don't get a real file object and it is sometimes a little bit awkward to work with it, but it does the trick.

pstorch on 18 Jun 2020

Do you know if takePersistableUriPermission() will grant access to the requested folder only or will it allow access to subfolders, too? IIRC there where problems in the past with getting access rights to sub stuff, esp. if the requested folder lived in the root of external storage. I'm thinking of folders which may be created by c:geo itself (like /external/cgeo, containing .../backup and .../files, whereas .../export, .../import etc. may have to go there, too) or folders that even already exist, created by a previous c:geo installation (like a backup) or even other apps (like /external/locus/mapsVector plus .../_themes)?

MagpieFourtyTwo on 19 Jun 2020

I think you also get access to subdirectories. At least I used it in another App to read subdirectories. I have not yet created any.

pstorch on 19 Jun 2020

As we have a workaround applied for up to Android 10 (see #8473) I renamed the issue and set labels accordingly.
We need a solution starting with Android 11.

Lineflyer on 14 Jul 2020

Info:
Workaround tested by fresh install of 2020.07.15-NB on my Android 10 , Samsung Galaxy Tab S4.
Everything fine: User storage c:geo dir: /storage/emulated/0/cgeo is being used.

Lineflyer on 15 Jul 2020

Just for information / reference: Changed developer guidelines from Play Store, valid from July 2020: (sorry, text in German, coming from Play Store)

Berechtigung "Zugriff auf alle Dateien"

Dateien und Verzeichnisattribute auf dem Gerät eines Nutzers gelten gemäß den Richtlinien für personenbezogene und vertrauliche Informationen und den folgenden Anforderungen als personenbezogene und vertrauliche Nutzerdaten:

Apps dürfen nur in dem Umfang Zugriff auf den Gerätespeicher anfordern, wie er für die Funktion der App entscheidend ist, und dürfen nicht im Namen eines Drittanbieters Zugriff auf den Gerätespeicher anfordern, der nicht in Zusammenhang mit wichtigen Funktionen für den Nutzer steht.
Android-Geräte mit R (Android 11, API-Level 30) oder höher benötigen die Berechtigung MANAGE_EXTERNAL_STORAGE, um den Zugriff auf den freigegebenen Speicher zu verwalten. Alle Apps, die auf R ausgerichtet sind und einen umfassenden Zugriff auf freigegebenen Speicher ("Zugriff auf alle Dateien") anfordern, müssen vor der Veröffentlichung eine entsprechende Zugriffsüberprüfung bestehen. Apps, die diese Berechtigung verwenden dürfen, müssen Nutzer eindeutig dazu auffordern, unter den Einstellungen für "Spezieller App-Zugriff" die Option "Zugriff auf alle Dateien" für ihre App zu aktivieren. Weitere Informationen zu den R-Anforderungen finden Sie in diesem Hilfeartikel.

moving-bits on 24 Aug 2020

Haven't done any coding yet, just wanted to share my readings and thoughts on this so far, to dive into discussion on how to continue:

Further reference: Info on use-cases

According to the migration guide, existing data should be moved to one of the following locations:

private data: use the dir returned by getExternalFilesDir()
shared files: move to an app-specific subdir of Downloads

To my understanding we do not use the first case, and the second case is for what's currently stored in /sdcard/cgeo/ and its subdirs, but we may discuss, whether backup files of database and settings as well as logfiles should be regarded as "private files" in this context.
Don't know about the cached data (images etc.) for caches. They are probably already stored on internal dirs.

We also need to check what happens to those files (both shared and private) on uninstalling the app. Probably the "private" files will get deleted automatically (which we would not want for the files mentioned above), the "shared" won't (as the OS does have no clue how the dir is named).
(just guesses from me here, though)

Looks like the dir beneath Downloads can be named freely, so we could stay here with "cgeo" (so maybe it's primarily about using /Downloads/cgeo/ instead of /sdcard/cgeo/ + migration?).

Files need to be moved using File API, and with the requestLegacyExternalStorage still set to true in AndroidManifest.xml (which we currently have). Accordings to the docs on Android 11 devices this flag will only work until the app gets uninstalled and reinstalled. Haven't tested this yet, so don't know what that will mean for users switching between release / beta / nightly builds back and forth.

Our file choosers (eg: map dir, map theme dir, gpx import/export etc.) probably won't work any longer (have not tested) after a fresh uninstall of a non-dev version on Android 11+, targeting API 30+. Those may have to be replaced by the intent/onActivityResult-scheme described by @pstorch above.

Are there more things we have to take care of?

How could a transition be implemented:

keep the compatibility flag (requestLegacyExternalStorage)
on startup (and if running on Android 10+) look for legacy base dir (/sdcard/cgeo/) and if it exists: start a migration routine, blocking all other functionality in c:geo until it's finished
- show a popup message to inform about migration and the new dir location
- move all files from legacy location to a similar dir in new shared location, keeping the subdir-structure (try to move first, copy-and-delete on error)
on requesting files internally: always look up at the new location first, then the legacy location

Any thoughts on this?
Anyone having experience with migrating an existing app to scoped storage?

moving-bits on 26 Sep 2020

I installed a fresh AVD with API level 30, set requestLegacyExternalStorage to false and targetSKD to 30. From playing around in c:geo installed this way I found:

directory selection dialogs in Settings (e.g. for offline map storage) do not work any more
regardless of directory settings I do there, all data seems to be stored in (sub)folders of /data/data/cgeo.geocaching - which assumingly is the "private" folder of c:geo since I can't access this folder from the (emulated) device, only via Android Studio (see screenshot). I tried map download, gpx export, logfile export and db backup
exception of the above are the files stored for offline caches (e.g. images), they are still in /sdcard/Android/data/cgeo.caching (but also not accessible from device)
second exception from above are the image copies made by cgeo when selecting images for offline logs, those are in /sdcard/Pictures/cgeo, and those are accessible also from device

@moving-bits I have nothing to add to your comprehensive analysis of the situation. Since I think this is the most pressing issue for c:geo currently (I want to switch to Android 11 as soon as possible :-)), is there any way I can help by e.g. taking over part of implementation? Unfortunately I have no previous experience with this sort of migration.

eddiemuc on 27 Sep 2020

@moving-bits I am about to reach a merge able step for my current issue ("fast track to add images"). Since I feel android 11 is our most pressing issue, can I assist you in implementation. For example, does it help if I take over changing the "directory selection" logic for various settings (e.g. map storage dir and such)?

eddiemuc on 3 Oct 2020

I've set up a new branch 'storage_framework' to be able to collaborate on this topic. Whether we merge this branch later on into master or put the findings into separate PR is to be decided later.

Additionally I've created a project "Android 10+ storage framework" to group all issues related to this task.

@eddiemuc
Thanks for your offer. Any help is welcome! Maybe you can start by looking at what I've come up with so far, see #9136. Currently I'm stuck with getting the saved granted uri (tree/primary:Download/cgeo) to work as a base folder replacement in LocalStorage, method getExternalPublicCgeoDirectory - but maybe we need to rewrite our logic here?

Regarding the directory / file selectors:
A bit surprisingly our current dir/file selectors continue to work within the new base dir structure, but only for the files we've created on our own, not for files moved into the new base dir structure externally. (Eg: If you download a new mapfile by using the internal map downloader, it get's stored into the new dir structure, and gets listed within c:geo as possible map source. If you add another map file to the same dir, using Android Studio, c:geo will not see this file.) - To be investigated. We probably need to

replace the dir/file selectors by using the SAF selectors,
replace the listing of available maps by either a menu entry "select offline map file" (which opens the SAF selector), or store the names of the map files we've been granted access to in our local db and list those.

moving-bits on 4 Oct 2020

@moving-bits I looked at your code and at documentation, and I fear we have to rework the way we deal with data (and may no longer be able to use "File" class directly) when dealing with data in public directories...

I wrote a hack specifically for logfiles (we have to start somewhere :-)) which, after logfile creation, copies the logfile to the public directory, building on the permission granted by the intent you coded. It uses the contentresolver methods and DocumentFile class, maybe you can take a look at it?

eddiemuc on 10 Oct 2020

PR is #9178

eddiemuc on 10 Oct 2020

Note to self: https://developer.android.com/training/data-storage/shared/documents-files

eddiemuc on 11 Oct 2020

Note to self: changing all read/write-access from File to Input/Outputstrem logic will not work for some use cases: example: Offline Maps (uderlying "MapDatabase" needs a File to work with, see MapsforgeMapView.setMapSource)

eddiemuc on 11 Oct 2020

I changed the MapView to work with DocumentFile in another project. Maybe this helps you:
https://github.com/OpenTracksApp/OSMDashboard/blob/main/src/main/java/de/storchp/opentracks/osmplugin/MapsActivity.java#L316

pstorch on 11 Oct 2020

At https://developer.android.com/about/versions/11/privacy/storage#scoped-storage it sounds like everything will work on Android 11 as before even if someone removes & reinstalls c:geo, as long as we do not target API30.

Only if we target API30 (what we have to do at some point in the future) we need to migrate to the Storage Access Framework (described in https://developer.android.com/training/data-storage/use-cases#migrate-legacy-storage)

But it sounds like it is safe for our users to update to Android 11 as we have the requestLegacyStorage Flag set in the manifest...

fm-sys on 11 Oct 2020

@pstorch thanks a lot, I think you just gave me the solution I seeked :-)
One question to you, since I am a bit struggling on the whole SAF: I have the feeling that in order to remain future-proof with c:geo we would have to change more or less every usage of File class in the system towards usage of DocumentFile/DocumentContract or direct Input/OutputStream usage. I saw some things out there on how to still use Files (e.g. replacing beginnings of URIs by "/storage/emulated/0" and the like), but those seem pretty "hacky" to me.
Would you agree on that in general? E.g. should we try to get rid of "File" usage in general?

eddiemuc on 11 Oct 2020

But it sounds like it is safe for our users to update to Android 11 as we have the requestLegacyStorage Flag set in the manifest...

My understanding was that Android 11 forces API30 and the flag will not work for newly installed applications any more (only for apps already present when device was upgraded to Android 11). BUt I might be wrong here...?

eddiemuc on 11 Oct 2020

From https://developer.android.com/about/versions/11/privacy/storage#scoped-storage

Apps that run on Android 11 but target Android 10 (API level 29) can still request the requestLegacyExternalStorage attribute. [...] After you update your app to target Android 11, the system ignores the requestLegacyExternalStorage flag.

fm-sys on 11 Oct 2020

See https://developer.android.com/training/data-storage/use-cases#migrate-legacy-storage

Set the preserveLegacyExternalStorage flag to true to preserve the legacy storage model so that your app can migrate a user's data when they upgrade to the new version of your app that targets Android 11.
Note: If you set preserveLegacyExternalStorage to true, the legacy storage model remains in effect only until the user uninstalls your app. If the user installs or reinstalls your app on a device that runs Android 11, then your app cannot opt out the scoped storage model, regardless of the value of preserveLegacyExternalStorage.

Looks like we have two different flags here. One is only for migration, and the other one is to give developers a bit more time... So if we continue using requestLegacyExternalStorage we should be safe ;-)

fm-sys on 11 Oct 2020

@eddiemuc only for the app internal/private files you can still use File, but for everything else you should replace by Uri, DocumentFile or InputStream. And I think it is recommended to remove those hacks 😉

pstorch on 11 Oct 2020

@eddiemuc only for the app internal/private files you can still use File, but for everything else you should replace by Uri, DocumentFile or InputStream. And I think it is recommended to remove those hacks 😉

@pstorch : thanks again for your help!
With regards to the hacks I mentioned: I saw those in the internet (stack overflow etc), not in cgeo code. And I will not introduce them into c:geo :-)

eddiemuc on 11 Oct 2020

👍1

Looks like we have two different flags here. One is only for migration, and the other one is to give developers a bit more time... So if we continue using requestLegacyExternalStorage we should be safe ;-)

Might be, but at some point we have to migrate. Lets use the time we have to have a good migration plan and (well-tested) code to support it when we can't ignore this any more.
Plus, my feeling after installion c:geo (with SDK29) under an (emulated) Android 11 was that not all is working.

eddiemuc on 12 Oct 2020

As far as I can see it from code and own experience we have the following public folder we need to access:

FOlders we need to WRITE to which are based on a common base dir (today: /cgeo in root dir):
- backup (special case: has subfolders)
- gpx
- logfiles
- field-notes
Folders we need to WRITE and READ which are based on another common (but fixed) base dir:
- Pictures/cgeo (for offline log images)
Folders we need to READ/WRITE whose place is configurable by Settings:
- maps (do we need to write here too due to map download?)
- _themes

Question to all: did I forget anything? Did I misinterpret something?

I thought about the following rough plan to tackle the storage thing for Android 11:

Create an enum LocalFolder which encapsulated for each of the above folders the basics we need to know (name of folder, do we need to read and/or write, base dir for folder, maybe things like mime type, file suffix, unique-file-build-rule etc)
Create an interface PublicLocalStorage (in contrast to LocalStorage, which will afterwards only be used for private storage only)
- This IF shall offer methods based on Streams (Input/Outputstreams), something like e.g.
  - boolean writeToStorage(final LocalFolder folder, final String name, final Outputstreams out);
  - InputStream readFromStorage(final LocalFolder folder, final String name);
  - List<String> getContent(final LocalFolder folder);
  - more methods are they seem necessary (e.g. something to handle subfolder creation for backup)
Make two implementations of this interface, one for Android
- The one for Files internally
- The one for >=SDK29 works with DocumentFiles/SAF internally
- Neither File nor DocumentFile (or other classes related to SAF) are exposed
Note: since SAF (in contrast to File usage) requires a reference to an Activity in most cases, the new IF needs to have such a reference and thus shall be instanciated per Activity using public storage.
Let all parts of c:geo using public storage USE the new classes. This will be the most difficult part, and I fear there will be some regression bugs inbetween until this runs smooth. Some cases I already identified
- Logfile creation most likely has to write in a (private) File first (due to IF of ProcessBuilder) and then copy that file's content into a public folder
- Internal usage of mapsforge for offline maps has to be changed, since today's implementation needs to be passed a Map Database File (@pstorch fortunately provided an example on how we can use a FileInputStream instead, kudos!)
- ImageUtils heavily works with Files unfortunately and needs to be converted to usage of Input/Outputstreams (the libs used by ImageUtils support this though)
Some additional helpers will be needed to make SAF work
- A helper class to be put into activities to make the necessary intent calls to user in case a necessary public folder has no read/write permission (yet). This one needs a callback to onActivityResult, maybe a similar pattern as in ImageActivityHelper can be used here
- One-time migration routines are needed for copying content of the current structure to the new
- How do we choose the new "common base dir"? In principal the user can select ANY dir he/she wants, but probably a subdir of Documents would be best
- I also think that the common media folders (and subfolders) are writeable/readable by default, maybe when using those we don't need to aquire permissions. THis would ease things a lot! (e.g. Documents, Downloads, Pictures and such)

Please give feedback. I really want to have a group opinion before diving into implementation, because this is kind of huge.

eddiemuc on 12 Oct 2020

@eddiemuc when you wrote about hacks I remembered this one in c:geo where the fstab file is parsed to find other mounted directories:

https://github.com/cgeo/cgeo/blob/master/main/src/cgeo/geocaching/storage/LocalStorage.java#L215

And I think this should be removed completely.

pstorch on 12 Oct 2020

👍1

Looks like we have two different flags here. One is only for migration, and the other one is to give developers a bit more time... So if we continue using requestLegacyExternalStorage we should be safe ;-)

If this allows, that c:geo can be used for Android 11 for the time being, we should IMHO make use of it. This would provide time to think about a proper migration and implement it step by step.
Because we already have some users on support mail asking about Android 11 as their devices have upgrades available.

Lineflyer on 12 Oct 2020

@eddiemuc when you wrote about hacks I remembered this one in c:geo where the fstab file is parsed to find other mounted directories:

https://github.com/cgeo/cgeo/blob/master/main/src/cgeo/geocaching/storage/LocalStorage.java#L215

And I think this should be removed completely.

Yes, I definitely agree

eddiemuc on 12 Oct 2020

Looks like we have two different flags here. One is only for migration, and the other one is to give developers a bit more time... So if we continue using requestLegacyExternalStorage we should be safe ;-)

If this allows, that c:geo can be used for Android 11 for the time being, we should IMHO make use of it. This would provide time to think about a proper migration and implement it step by step.
Because we already have some users on support mail asking about Android 11 as their devices have upgrades available.

requestLegacyExternalStorage is already set to true in c:geo. If this is all it takes, then c:geo should work as is in Android 11. I guess we just need volunteers for test then :-) I would recommend making a backup before though.

preserveLegacyExternalStorage is not set yet, but as the documentation claims it would only work until reinstall and is meant for migration only.

eddiemuc on 12 Oct 2020

Looks like we have two different flags here. One is only for migration, and the other one is to give developers a bit more time... So if we continue using requestLegacyExternalStorage we should be safe ;-)

If this allows, that c:geo can be used for Android 11 for the time being, we should IMHO make use of it. This would provide time to think about a proper migration and implement it step by step.
Because we already have some users on support mail asking about Android 11 as their devices have upgrades available.

AFAIK we have already set this flag. But we need some testing on Android 11 devices. Maybe some future changes need to be done temporarily, and as a temporary solution even some "hacky" implementations would be ok IMHO.

Edit: @eddiemuc You were faster... ;-)

fm-sys on 12 Oct 2020

😄1

. But we need some testing on Android 11 devices

Well, I just got info, that Samsung started public beta phase for my S20.Shall I take the risk?
In that case I can easily provide feedback while using nightlies.

Lineflyer on 12 Oct 2020

Well, I just got info, that Samsung started public beta phase for my S20

Nevermind....currently only in US and Korea.

I just aligned with @mucek4 that we buy a used Pixel smartphone. Will have it next days for testing.

Lineflyer on 12 Oct 2020

@Lineflyer before you buy a phone for this, maybe we can start with the emulators coming with Android Studio. There is android 11 support already.

I guess the hard part will.be to simulate an upgrade from installed c:geo on android <=10 to 11 though

eddiemuc on 12 Oct 2020

Ok, I installed c:geo as-is (current master state, targeting SDK29, requestLegacyExternalStorageset to true) on a clean Android 11 Emulator Device (Pixel XL API30).

Results are not so promising:

I created a logfile using Settings-GUI. Logfile was created in private storage and can't be accessed outside of c:geo app
I tried setting a map download directory, but could only browse c:geo private storage with it

In summary it seems that with current settings c:geo will NOT work under Android 11, even when we target only SDK 29. If this turns out to be true, I fear we have to do the "big refactoring" or something like it.

Any alternative test results?

eddiemuc on 13 Oct 2020

Any alternative test results yet? Otherwise I must assume that current c:geo version does not work for Android 11.

I also thought about a hack solution (which might fall in our back later). At least on emulator I can access public dirs (Pictures, Downloads, Documenta,...) Using File IF I know the individual file path prefix beforehand. We could use this loophole to keep changes at a minimum. Alas, I didn't find anything about that in documentation, so I can't tell whether this works on all devices or whether this way will get closed with later Android versions. Also, I don't know a non-hack-way to get the correct file path prefix (Android API uses Uris)

eddiemuc on 14 Oct 2020

At least on emulator I can access public dirs (Pictures, Downloads, Documenta,...) Using File IF I know the individual file path prefix beforehand.

So maybe the requestLegacyExternalStorage
flag is restricted to these public directories (just guessing), or did the file access even work without this flag?

Than we would only need to migrate the cgeo dir to one of this folders as a short term solution...

fm-sys on 14 Oct 2020

At least on emulator I can access public dirs (Pictures, Downloads, Documenta,...) Using File IF I know the individual file path prefix beforehand.

So maybe the requestLegacyExternalStorage
flag is restricted to these public directories (just guessing), or did the file access even work without this flag?

Than we would only need to migrate the cgeo dir to one of this folders as a short term solution...

Again: maybe, but since I can't find any docu about this this solution is most likely:

hacky (since it is cleary not the way Android Framework wants you to go), and
not guaranteed to work in all scenarios, and without support if it doesn't
Please proove me wrong

eddiemuc on 14 Oct 2020

Two updates from my side:

I will probably have an Android 11 testing device by this weekend and can do some practical checks
I got in touch with a c:geo user who recently upgraded to Android 11. He stated, that c:geo seems working normally for him. I took the chance to send him a little questionaire and waiting for reply just now:
Did you upgrade your device to Android 11 while c:geo was already installed, or did you freshly install c:geo while the device was already running Android 11?

Can you still perform a backup (c:geo Menu - Settings - Backup/Restore)?
Can you still export a GPX (List - Menu - Export - Export to GPX)?
Can you still open the dialog to select a map directory for offline map (c:geo Menu - Settings - Map)?
Can you still change the directories for GPX export/import defined in c:geo Menu - Settings - Offline Data?
Can you still move the Geocache data directory in c:geo Menu - Settings - Offline Data between internal memory and SD-card?

Lineflyer on 14 Oct 2020

👍1

Quick update:

Freshly installed cgeo 2020.10.10 on an Android 11 device just now.
Everything seems to work normal.

More detailed test tonight.

Sys info:

--- System information ---
Device: Pixel 3a (sargo, google)
Android version: 11
Android build: RP1A.200720.009
c:geo version: 2020.10.10
Google Play services: disabled - 20.36.15 (150400-333172415)
Low power mode: inactive
Compass capabilities: yes
Rotation vector sensor: present
Orientation sensor: present
Magnetometer & Accelerometer sensor: present
Direction sensor used: rotation vector
Hide caches: -
Hide waypoints: -
HW acceleration: enabled (default state)
System language: de_DE
System date format: dd.MM.yy
Debug mode active: no
System internal c:geo dir: /data/user/0/cgeo.geocaching (47,1 GB free) internal
User storage c:geo dir: /storage/emulated/0/cgeo (47,1 GB free) external non-removable
Geocache data: /storage/emulated/0/Android/data/cgeo.geocaching/files/GeocacheData (47,1 GB free) external non-removable
Database: /data/user/0/cgeo.geocaching/databases/data (496,0 KB) on system internal storage
GPX import path: /storage/emulated/0/cgeo/gpx
GPX export path: /storage/emulated/0/cgeo/gpx
Offline maps path: /storage/emulated/0/offlinemap
Map render theme path: 
Live map mode: true
Global filter: display all caches
Fine location permission: granted
Write external storage permission: granted
Geocaching sites enabled:
   geocaching.com: Logged in (Anmeldung OK) / BASIC
Geocaching.com date format: MM/dd/yyyy
Installed c:geo plugins:  none
BRouter connection available: false
--- End of system information ---

Lineflyer on 15 Oct 2020

Can't tell you why - but today it works in the emulator as well. Don't know what to think of this.

eddiemuc on 15 Oct 2020

Do we have the requestLegacyExternalStorage already contained in 2020.10.10 ?

Lineflyer on 15 Oct 2020

as far as I can see its there since 19.6.20 (by @moving-bits )

eddiemuc on 15 Oct 2020

I have tested now a bit deeper:

Directory structure is normal as we are used to (see sysinfo above)
Those directories are all write- and readable:
- Export GPX OK
- Import GPX OK
- Backup settings/database OK
- Restore settings/database OK
- Store logs pictures OK and available in offline case (=stored and readable)
- Use offline map stored in random dir (see sysinfo above) OK
- Map downloader OK to download and move map to offline map dir

I could not yet test moving geocachedata to a real external SD (as the phone does not have one) but I assume it should work
Permission handling:
- The permission handling on first start up works normal (as on Android 10)
- There is however one new function: Users can grant one time permission for location. c:geo seems to handle this properly, i.e. on next session start permission for location is requested again)

I also tested some basic functions (live map, search, logging) without any noticeable regressions.

But maybe its worth a try to see what happens if we increase targetSDK because we currently target Android 10 (API 29).

@moving-bits
If my tests are confirmed by someone (e.g. emulator) or user experience we should IMHO remove the warning from the main screen (PR to release). BTW: I totally missed creating the #android11 anchor for the FAQ...but seems we do not need it.

Lineflyer on 15 Oct 2020

I did not follow the complete thread of this issue, however this page

https://developer.android.com/about/versions/11/behavior-changes-all

does not imply any behavior change for storage, those only apply once we target API 30 (Android 11):

https://developer.android.com/about/versions/11/behavior-changes-11

Lineflyer on 15 Oct 2020

Ok, since it is inconclusive whether we need a change here or not, I will restrain from doing any further coding for now on this isse. My solution suggestion from more above still stands for discussion in case (or when) we need to start activities here.

eddiemuc on 16 Oct 2020

Ok, since it is inconclusive whether we need a change here or not, I will restrain from doing any further coding for now on this isse. My solution suggestion from more above still stands for discussion in case (or when) we need to start activities here.

My conclusion after reading through the links above is:

We do not need it urgently, but we need it in future as soon as we target API 30. I don't know when and if Google enforces this on Google play at some point in future (maybe end of next year as they enforce API29 in November 2020).
IMHO (if my understanding of the changes is correct) we can differentiate between reading (e.g offline maps) which should still be possible by using a new permission and writing (which should use some sharing function or storage framework instead of direct access).

Lineflyer on 16 Oct 2020

Any alternative test results yet? Otherwise I must assume that current c:geo version does not work for Android 11.

Last try with today's master has been negative - crashing already during startup because of not being able to create some dirs (coming from externalPublicCgeoDirectory being null).

moving-bits on 20 Oct 2020

If my tests are confirmed by someone (e.g. emulator) or user experience we should IMHO remove the warning from the main screen (PR to release). BTW: I totally missed creating the #android11 anchor for the FAQ...but seems we do not need it.

Cannot confirm c:geo working reliably under Android 11 yet, at least on emulated devices, see my comment in https://github.com/cgeo/cgeo/issues/9225#issuecomment-712772466

Maybe we should add the anchor and summarize the info & experience we have with c:geo running on Android 11. For the "wild and crazy" we can list some tips for preparation:

Some people might want to try using c:geo with Android 11, but they should do so in a prepared way, just in case, eg.:

backing up data and settings beforehand, including copying the data to a different device
being prepared to move some data around (eg: map, gpx etc.)
and to update some paths manually using Settings => System => View Settings
... (maybe more)

moving-bits on 20 Oct 2020

In the meantime: I would continue working on a new storage framework for Android 11 (SDK 30), but I would appreciate some feedback to the suggestions I made more up this thread before (I don't want to invest too much work going into the wrong direction...)

eddiemuc on 20 Oct 2020

As far as I can see it from code and own experience we have the following public folder we need to access:

FOlders we need to WRITE to which are based on a common base dir (today: /cgeo in root dir):

backup (special case: has subfolders)

gpx

logfiles

field-notes

Folders we need to WRITE and READ which are based on another common (but fixed) base dir:

Pictures/cgeo (for offline log images)

Folders we need to READ/WRITE whose place is configurable by Settings:

maps (do we need to write here too due to map download?)

_themes

Question to all: did I forget anything? Did I misinterpret something?

Thanks for this comprehensive summary. LGTM generally. One tiny addition: gpx is hybrid - we have a directory selector for import/export dir, but it defaults to /cgeo/gpx.

Regarding the maps dir: Yes, c:geo writes to that dir on downloading a map file.

moving-bits on 20 Oct 2020

How do we choose the new "common base dir"? In principal the user can select ANY dir he/she wants, but probably a subdir of Documents would be best

I also think that the common media folders (and subfolders) are writeable/readable by default, maybe when using those we don't need to aquire permissions. THis would ease things a lot! (e.g. Documents, Downloads, Pictures and such)

The migration guide (https://developer.android.com/training/data-storage/use-cases#migrate-legacy-storage) names the Downloads dir as base dir for public app data:

Move any shared non-media files to an app-dedicated subdirectory of the Downloads/ directory.

Although semantically I find the Documents/ tree more logical as new base for cgeo and its subdirs (gpx, backup, map etc.)

moving-bits on 20 Oct 2020

Move any shared non-media files to an app-dedicated subdirectory of the Downloads/ directory.

Although semantically I find the Documents/ tree more logical as new base for cgeo and its subdirs (gpx, backup, map etc.)

Please don't use the downloads folder if there is any other possibility. IMHO the download folder should only be used for downloads, not for normal files which should not be deleted... So +1 for /document/

fm-sys on 20 Oct 2020

I am still awaiting some sort of feedback with regards to the technical solution I sketched here: https://github.com/cgeo/cgeo/issues/8457#issuecomment-706885184
As soon as we have an agreement in what direction to proceed I am good to go

Or would you rather go the alternative way: keep using File, move public folders to subfolders of Document/Downloads/Pictures/... and trust that this stays usable with SDK30 across all devices?

eddiemuc on 27 Oct 2020

I am still awaiting some sort of feedback with regards to the technical solution I sketched here: #8457 (comment)
As soon as we have an agreement in what direction to proceed I am good to go

Sorry for not having replied to your comprehensive proposal yet. The way you describe sounds suitable to me, but alas my knowledge of the technical details are not deep enough to really help you with this.

Or would you rather go the alternative way: keep using File, move public folders to subfolders of Document/Downloads/Pictures/... and trust that this stays usable with SDK30 across all devices?

That was the way I tried in my first draft - use Downloads/cgeo as new base dir (could have been Documents/cgeo as well) and try to continue file handling the same otherwise, except gathering access rights once to this base dir and its sub dirs (which opens the new system dialog "allow c:geo access to directory ..."). But at least in my rough draft it did not work completely, as only the files created by c:geo itself have been visible to c:geo (in the dir chooser / directory listings) - but I can't say if this is an inherent limitation or some missing flag or else. (To my understanding the grant access dialog should do just that, grant our application to a certain dir, its subdirs and all files in it. But I may have missed something along the way.)

So from my side: Feel free to direct your implementation in either way.
And a big thanks for all the effort you have already put in there 👍

moving-bits on 27 Oct 2020

Thanks @moving-bits for the good feedback.

One note regarding usage of subfolders of public dirs: by experiments in emulator I found that those are simply publiccally accessible same way as before the whole phone was. No need to do something with regards to permissions when we use them. The real Problem ist to find out the correct local file path to them (android libs just give Uris). And of course, since this is not documented (or I didn't find it), I don't know if this way is Future-Proof. See also comments of @pstorch further above

eddiemuc on 28 Oct 2020

BTW: how could you access the /Downloads or /Documents folders?

pstorch on 28 Oct 2020

BTW: how could you access the /Downloads or /Documents folders?

Hi @pstorch I noticed that storing of offline log images in Pictures/cgeo still worked, even in ask 30, without the legacy-flag and despite it using the safe or getting any new permissions. I experimented from there. I am not sure where ImageUtils currently gets this directory, possibly through the dirty hack you mentioned earlier in this thread.
Note though: I only tried this in an emulator, never on a real device.

eddiemuc on 28 Oct 2020

BTW: how could you access the /Downloads or /Documents folders?

Hi @pstorch I noticed that storing of offline log images in Pictures/cgeo still worked, even in sdk 30, without the legacy-flag and despite it using simple files instead of SAF or getting any new permissions. I experimented from there. I am not sure where ImageUtils currently gets this directory, possibly through the dirty hack you mentioned earlier in this thread.
Note though: I only tried this in an emulator, never on a real device.

eddiemuc on 28 Oct 2020

So you haven't tested to access /Downloads nor /Documents?

Regarding Pictures/cgeo: the ImageUtils used LocalStoragegetLogPictureDirectory() and then Environment#getExternalStoragePublicDirectory which is deprecated: https://developer.android.com/reference/android/os/Environment#getExternalStoragePublicDirectory(java.lang.String) and should not be used, but maybe it still works for the time being.

pstorch on 28 Oct 2020

So you haven't tested to access /Downloads nor /Documents?

Yes I did. I took the working file path for pictures and replaced pictures with downloads and documents. Worked (in emulator)

eddiemuc on 28 Oct 2020

To state my very own opinion on this: I don't think we should rely/fall back to some "hackish" usage of some "loophole" and thus should NOT use any File-based access for public storage any more (even if we are able to get a grip on the above public directories using Files somehow). Instead we should use the SAF as it is intended by Google/Android. Ths also means using Input/OutputStreams instead of Files.

But this is a lot of work.

eddiemuc on 28 Oct 2020

👍1

Yes I did. I took the working file path for pictures and replaced pictures with downloads and documents. Worked (in emulator)

Ok, don't know if this is reliable.

pstorch on 28 Oct 2020

A public application base directory like /sdcard/cgeo is not "Android like" and I would suggest to not use it anymore.

For these public folders I would ask the user via SAF individually as needed:

backup
gpx
logfiles
field-notes
maps
_themes

For the log images I would use the MediaStore API, so they are probably put under Pictures folder, but in a reliable Android way. You can define a subdirectoy via the RELATIVE_PATH attribute.

pstorch on 28 Oct 2020

Whenever I find time, I am currently working on this using the branch storage_framework. From time to time I need to rebase this branch so it does not fall behind master too much. Yesterday I realized that this involves forced pushes to central repository - which is a bad thing when sharing remote branches.

Several questions:

Is there anyone currently using this branch so I have to sync with them?
Is there a way of doing this kind of rebases without the need to force-pushes (--force-with-lease of course)?
Is it ok for now to merge non-reviewed PRs to storage_framework?

eddiemuc on 2 Nov 2020

Checked in a drafty version of SAF for logfiles creation. TODO list for self:

[x] Core: PublicLocalFolder: transform from enum to class (hierarchy). Enum is not flexible enough
[x] Core: Easify usage/check for missing folder permissions by Activities (provide easy helpers in AbstractActivity, but make it easy also for Activities not inheriting from it and having to use PublicLocalStorage directly)
[x] Core: PublicFolderStorage: add methods for reading files, getting folder content etc
[ ] Core: clean up everything
[x] Core: check whether solution also works on very old Android versions (API21). Otherwise a fallback-implenemtation is needed (Filebased) -> tested, no fallback needed
[ ] Core: convert choosing of folders in Settings to SAF
[x] Logfiles: clean up and test severely
[ ] Logfiles: implement WSAF also for head dumps
[x] Map: transfrom base dir readingto SAF. (single file, combined, map attribution)
[ ] Map: transform external map dir (map and themes!)
[ ] Map: transfrom writing (downloaded) map files to SAF. TODO: clean up Settings for map downloads by this
[ ] Backup: transfrom reading/writing Backups to SAF
[ ] GPX: transfrom reading/writing GPX files to SAF
[ ] field-notes: transfrom writing field notes files to SAF
[ ] (Log) Images: transfrom ImageUtils from File-based to Stream-based
[ ] (Log) Images: transfrom reading/writing Images to SAF
[ ] Migration routine: allow users to migrate old content (easiest way: let them choose previously used dir as cgeo public root dir)
[ ] Final test of everything

eddiemuc on 3 Nov 2020

Is there anyone currently using this branch so I have to sync with them?

Is there a way of doing this kind of rebases without the need to force-pushes (--force-with-lease of course)?

Is it ok for now to merge non-reviewed PRs to storage_framework?

For now I assigned this issue to me. I will also regularly rebase storage_framework to master using git push --force-with-lease since I seem to be currently the only one working on this. Also I don't know any other way to rebase.

To anyone: please notify me before you start working/commiting to this branch too. I assume I have to stop using forced pushes then.

eddiemuc on 3 Nov 2020

Also I don't know any other way to rebase.

I use VCS > Git > rebase > upstream (c:geo/master) (or something like this, I currently have no PC with me) for rebasing with android studio...

fm-sys on 3 Nov 2020

Hi @fm-sys , problem is not so much the rebase itself (which I do locally) but when it comes to pushing it to remote. Since rebase rewrites history, change must be pushed with --force-with-lease. Which is exactly what you shouldn't do to (remote) branches also worked on by other people, because it might screw up their work.

This is why I ask anyone wanting to work on storage_framework branch to give me a short notice. I will then stop pushing rebases onto it immediately.

eddiemuc on 4 Nov 2020

You're right. AFAIK a better way would be to merge branches like we do when we are fixing changes at release and metge them back to master. But I'm really not a git expert... As long as you're the only one working at this branch, you're current way should also be fine

fm-sys on 4 Nov 2020

You're right. AFAIK a better way would be to merge branches like we do when we are fixing changes at release and metge them back to master. But I'm really not a git expert... As long as you're the only one working at this branch, you're current way should also be fine

As far as I understand git(and I am definitely a git novice; I learn as I go, and c:geo is my first project with real git experience):
Using merge instead of rebase would be the way to go as soon as other people start working on the same branch too. However, until they do I would use rebases since this creates a cleaner codebase/git-history in the end (at some point this all has to be merged to master and I don't want merge hell at that point)

eddiemuc on 4 Nov 2020

Oh man, I realized today that accessing media files should be done using yet another API than SAF or direct file access: via contentResolverusing various constants and methods of MediaStore and its subclasses.
This is so much work...

eddiemuc on 6 Nov 2020

With API 30 there is also Direct File Path access to the media store: https://developer.android.com/training/data-storage/shared/media#direct-file-paths

But for API 29 the requestLegacyExternalStorage is needed. Don't know how backward compatible that is, for even older API levels.

pstorch on 6 Nov 2020

@pstorch Thanks for the link. I am a bit unsure, but doesn't this basically mean that nothing needs to be changed in c:geo with regards to the images (because it is already using File to access a subdir of Image-dir, and the website sounds like this will be officially supported in SDK30)?
Only thing I couldn't find was how to stable and w/o depreciation getting the base Image dir as a File.

eddiemuc on 7 Nov 2020

I'm also not sure.I have not tried it.
But it might be an explanation why https://github.com/cgeo/cgeo/issues/8457#issuecomment-717825645 worked for you.

pstorch on 7 Nov 2020

A question to the round with regards of choosing Files and Directories from public storage locations.

I browsed c:geo code to find out how this is done currently (because it will need replacement/change when applying SAF), and my impression is that this is today a bit overpowered. I found a hierarchy of 6 activities solely dealing with file/directory choosing (see screenshot below), import of an external intent class (org.openintents.intents.FileManagerIntents) as well as an own implementation of a diretory chooser (cgeo.geocaching.files.SImpleDirChooser).

It is my current belief (and please correct me if I oversee something) that all this stuff can and must be replaced with SAF Intents Intent.ACTION_OPEN_DOCUMENT_TREE (for dirs) and Intent.ACTION_OPEN_DOCUMENT (for file choosing). This would result in code simplification in that part of c:geo too I guess. However, I believe some features in c:geo dir/file selection might not work any more than it did in the past:

I learned from code that on dir choosing c:geo searches standard paths of the device and, if it finds files matching the selected scheme, presents them as default path choices (for example: when choosing a new map dir, c:geo tries to look up directory Locus/mapsVector and searches it for existing map files). Such a logic is no longer possible with SAF since we will not be allowed to read these paths any more (we can only search in directory trees which were previously granted by user to c:geo)
Dir/File chooser dialog is no longer in our hands and thus cannot be designed by c:geo any more

I currently believe that the above regressions are unavoidable. HOwever, I also believe they are perfectly acceptable.

What is the group's opinion about that?

eddiemuc on 8 Nov 2020

👍1

IMHO these regressions are totally ok. But just for my understanding: Will c:geo still be able to access the /cgeo/ directory without user interaction? For example when you uninstall and reinstall c:geo there is dialog asking if you want to restore the latest backup (if there is one). Will it still be possible? If not, this would indeed be a crucial regression especially for developers/nightly testers who are often switching between versions (at least I do 😁).

fm-sys on 9 Nov 2020

While trying to integrate offline maps (usage and download) into SAF I discovered some seemingly strange logic on how the last used MapSource is stored. Apparently this is somehow split into two preferences "MapSource" (using a hashed string id) and "MapFile" (using, in case of OfflineMaps, the file to the offline map).

Questions to that:

I noticed that hashed strings are used as "numeric identifiers", but I can see no reason why strings can't be used instead as identifiers (since hashes have a small risk of colliding). Why was it done (or left) this way?

I don't understand why MapFile is needed in addition to the "id" - since in case of Offline Maps the map name is usedas idenfitier anyway

I would merge this into one property (and remove mapDirectory btw), except anyone can explain to me the necessity of current desing.

Maybe @Lineflyer or @moving-bits ?

eddiemuc on 22 Nov 2020

Regarding hash values as map identifiers: I don't know about the reasons for the original design decision, but IIRC the hash values are used as identifiers in the map menus for selecting a map.

moving-bits on 22 Nov 2020

Hi @moving-bits ,

I am in the process of removing MapFiles from c:geo, one of the many preconditions to introduce SAF. WHile doing that I stumbled across class cgeo.geocaching.helper.QueryMapHelper (see below).

It seems to be relatively new (introduced in April 2020) and if I figure correctly it "misuses" c:geo Settings to pass existing mapfile location to WhereYouGo. This will no longer work once map paths are real uris only (and probably even currently does no longer work since filepath is a file uri since last public release). As of now I fear this is an unavoidable regression (or in other words: whereyougo needs to implement SAF too). Would you agree?

And can I just delete this code file or use a dummy value to return as mappath for now?

eddiemuc on 25 Nov 2020

Yeah, this is used to query the current map path from c:geo for WhereYouGo to ease map onboarding over there. I don't know how frequently this is actually used (@Lineflyer : any indication for this on support?), but we can probably remove it. (And if we do, we should remove the counterpart in WhereYouGo as well, which I can take.)

moving-bits on 25 Nov 2020

Well, there is not such a huge user base for WhereYouGo, so I do not have any feedback on this from support.

Additionally:
By c:geo dropping mapsforge V3 support (and supporting download of maps with higher versions) the feature becomes a bit less attractive and more error prone.

Lineflyer on 25 Nov 2020

BTW: As you most probably know, WhereYouGo does support old MapsForge only anyway (which is the reason why I still have an old version of OSM installed, too) - hence c:geo's map path will (currently) not be useful for WYG, anyway,

Nevertheless, we're using WYG now and then, so if there is feedback for e. g. c:geo interaction needed, feel free to ask.

MagpieFourtyTwo on 25 Nov 2020

By c:geo dropping mapsforge V3 support

No, we haven't dropped the support for old map files. We just have finished the migration to an updated version of the mapsforge library which still contains backward compatibility for older mapfile versions

fm-sys on 25 Nov 2020

@MagpieFourtyTwo A genaral testing support for WYG would be very helpful. Currently @SchoolGuy is on the way to update, modernize and fix some parts.

bekuno on 25 Nov 2020

Good to hear, cause it's in fact a little bit ... outdated. ;)
Most regretfully we don't have that much WIGs around here ...
But we'll see what we can do,

MagpieFourtyTwo on 25 Nov 2020

By c:geo dropping mapsforge V3 support

No, we haven't dropped the support for old map files. We just have finished the migration to an updated version of the mapsforge library which still contains backward compatibility for older mapfile versions

Ah, therefore I was not able to find the old Use MapsForge v3 menu entry when searching for it a few minutes ago ... ;)

MagpieFourtyTwo on 25 Nov 2020

By c:geo dropping mapsforge V3 support

No, we haven't dropped the support for old map files. We just have finished the migration to an updated version of the mapsforge library which still contains backward compatibility for older mapfile versions

True, but for WYG afaik the libs have not been upgraded (or has it?). So WYG can most likely not use more and more the map files used by c:geo. Esp I assume the map files downloaded via c:geos integrated map download support have version > 3 and can't thus be read by WYG any more. Right?

@moving-bits : with regard to "graceful degradation": should I for now leave the QueryMapHelper in c:geo but just return a dummy value (maybe an empty string), or should I rather delete it?

eddiemuc on 26 Nov 2020

True, but for WYG afaik the libs have not been upgraded (or has it?). So WYG can most likely not use more and more the map files used by c:geo.

Yes, but we accepted this when implementing this feature. (Hoping that someone with time and knowledge for this would implement a v5 compatible mapsforge in WhereYouGo.)

Esp I assume the map files downloaded via c:geos integrated map download support have version > 3 and can't thus be read by WYG any more. Right?

Yes, we're downloading from the v5 directory.

@moving-bits : with regard to "graceful degradation": should I for now leave the QueryMapHelper in c:geo but just return a dummy value (maybe an empty string), or should I rather delete it?

Only partly "graceful" possible :-)

WhereYouGo checks the returned value, and for an empty string it displays an error toast "No mapfile set in c:geo".
If you return an invalid file name, WhereYouGo will display the file system error.
If you remove the QueryMapFile activity in c:geo completely, WhereYouGo will display ""c:geo application not found" on starting the request

All three of them are slightly misleading, but there is no way to deliver a dedicated error message which is pointing to that specific situation, so probably stay with an empty string or a non-existing filename.

moving-bits on 26 Nov 2020

@moving-bits If you want... After I am done with login checking I can add the support for V5... But that will not be tomorrow (metaphorically spoken).

SchoolGuy on 27 Nov 2020

🚀2

@moving-bits If you want... After I am done with login checking I can add the support for V5... But that will not be tomorrow (metaphorically spoken).

@SchoolGuy
Thanks for your offer, it would be really cool & helpful, if you could update WhereYouGo's mapsforge implementation to the current version.

moving-bits on 27 Nov 2020

Given recent discussions I would for now change the QueryMapHelper to return the c:geo map directory instead of the last selected map file. Some things to note:

If WYG is updated to use new mapsforge libs, I would suggest to also make it use inputstreams instead of files (like it was done in c:geo) as preparation for SAF
I would also suggest that WYG should instanciate a "combined" map with all files found in given map directory
Note that while right now map directory is a file uri, it will be a SAF tree uri (where WYG most likely need to get access permission by user) in the future
For the long run: it would be nice to factor out the whole map viewer code of c:geo such that both WYG and c:geo might be able to use it as some sort of library together. Same goes for settings and maybe there are other parts too. What do you think?

eddiemuc on 28 Nov 2020

My opinion is the following:

Settings: I am preparing a PR to use AndroidX and thus I am defining the settings via SharedPrefernces. Absolutely no code should be reusable because I am just writing what I need, the rest is Jetpack doing for me.
Maps: I have built no knowledge there so far but I guess the same applies for this. What we are doing shouldn't be so complicated that we require a library for it. If I am at the point where I think a library is needed, I will ping again.

Regarding the rest... Well I will catch up when I am not on the go but we are not developing a new rocket thus I think everything should already be there. My personal motto is reuse instead od reinvent if reasonably possible.

SchoolGuy on 28 Nov 2020

I have a question to the community how best to deal with a potential obstacle with SAF.

Since SAF does not allow access to public folders before explicitely granting it, that means when freshly starting c:geo we can no longer access the c:geo default folder "cgeo" to store logfiles, maps, gpx etc. Question is how to make this fact present to the user. I have following ideas:

Force the user to select and grant access to such a folder on startup. User can't continue working with c:geo until she did that. This is currently implemented (it is the easiest solution implementation-wise)
Just inform user but dont force her. Until a folder is selected an internal folder is used for all operations (and thus, user can't access files in there via another app or file explorer)
Just inform user but don't force her. All functions depending on external folder access will not work until such a folder is selected. Implementation-wise and testing-wise this is the option with most work.

Are there any opinions or other ideas on that?

Merry christmas to all of you and your loved ones, and stay healthy.

eddiemuc on 22 Dec 2020

In order to support your work (and maybe allow others to test the current state) I have implemented a dedicated nightly job for SAF:

This job will build a nightly from head of branch storage_framework each night at 1:30CET.
The APK can be downloaded from http://download.cgeo.org/cgeo-nightly-SAF.apk

@eddiemuc
Do you think this version is ready for some testing yet?
If not, thats fine. If you prefer feedback from testing, I would create a dedicated label and ask all testers to use it in order to make tracing the related issues easier.

Lineflyer on 22 Dec 2020

🎉1

Does c:geo really need a public directory in the root folder? From my understanding public app directories like /documents/cgeo/ are accessible without user interaction. But I'm not firm with this topic in any way, so correct me if I'm wrong...

fm-sys on 22 Dec 2020

Since SAF does not allow access to public folders before explicitely granting it, that means when freshly starting c:geo we can no longer access the c:geo default folder "cgeo" to store logfiles, maps, gpx etc. Question is how to make this fact present to the user. I have following ideas:

If we cannot get access to a (public) folder without user interaction I would vote for alternative 1: Ask the user and continue only after granting access to such a new base folder. (We probably need to recheck accessibility on each start of c:geo, though, as the folder may have been deleted, removed, renamed or whatever else making in inaccessible.)

If there is a chance to get a public base folder without user interaction, e. g. something like documents/cgeo etc., I would prefer such a directory, though.

In both cases, next step after creating / granting access to such a folder would be to migrate existing public c:geo data from today's base folder (and subfolders) to the new folder. This should probably be atomic and accomplished before the user can continue working with c:geo as well.

moving-bits on 22 Dec 2020

Do you think this version is ready for some testing yet?
If not, thats fine. If you prefer feedback from testing, I would create a dedicated label and ask all testers to use it in order to make tracing the related issues easier.

@Lineflyer actually my plan was to make a pull request from storage_framework to master this evening (or maybe tomorrow, depends on how much more time I need for cleanup)...

eddiemuc on 22 Dec 2020

In both cases, next step after creating / granting access to such a folder would be to migrate existing public c:geo data from today's base folder (and subfolders) to the new folder. This should probably be atomic and accomplished before the user can continue working with c:geo as well.

@moving-bits Actually my idea was a bit different: in the dialog "forcing" the user to choose a public base dir I suggest that the user may best take the existing "cgeo" application folder in root so all his/her data will still be available. This saves us the whole need for any migration routines.

Regarding whether there is some "fallback" public folder usable in case the user simply does not want to select a folder: I can check this. Probably documents/cgeo works, but alas Android was "intelligent" enough to provide different APIs for document access and media access, and both seem to be not completely compatible.

eddiemuc on 22 Dec 2020

@Lineflyer actually my plan was to make a pull request from storage_framework to master this evening (or maybe tomorrow, depends on how much more time I need for cleanup)...

My idea would be to have a testing phase first with the dedicated nightly on the current branch. This IMHO makes it a lot easier to test the migration cases.

Lineflyer on 22 Dec 2020

@Lineflyer no problem with that. Branch can be used for this testing as of now.
Additionally I created a (draft) pull request and put a lot of explanations/screenshots etc on it: PR #9571
Happy to get feedback

eddiemuc on 22 Dec 2020

Some thoughts after a first test:

it works
it's not really nice that you have to search for the c:geo folder, so we should at least tried to use the /documents/cgeo/ folder as default without user interaction. This is also the recommended way and avoids "root-directory-spamming"
a user defined map path is just ignored and discarded on upgrade, so at a first glance it might look like as all ofline maps were deleted
can we point the grand-folder-access-window to a default path? That would be very helpful for a quick and easy folder configuration
the explanation dialog for public dir configuration is cancelable. Is that really wanted?

Nevertheless thanks @eddiemuc for your great work!

fm-sys on 22 Dec 2020

Cgeo: Rework storage framework acc. to Android 11 requirements

Most helpful comment

All 109 comments

Related issues