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

target: Add persistent_data_path to AndroidTarget #622

Merged
merged 1 commit into from
May 30, 2023

Conversation

mrkajetanp
Copy link
Contributor

Some apps, such as Unity-based applications, can store some files in their 'persistent data path' which is usually under '/storage/emulated/0/Android/data'. Add a handle to easily access the path within AndroidTarget in the same way as package_data_directory currently is accessible.

devlib/target.py Outdated
@@ -1785,6 +1786,7 @@ def __init__(self,
is_container=is_container,
max_async=max_async)
self.package_data_directory = package_data_directory
self.persistent_data_path = persistent_data_path
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this should be the same for a ChomeOS target? In which case should we include a similar parameter for that target as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm would it? I don't really know anything about ChromeOS from that side..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness the way we handle ChromeOS targets (which support Android apps) is essentially creating both a Linux and Android Target together so any parameters (or properties) we add should also be usable by the Android portion of the ChromeOs target.

@douglas-raillard-arm
Copy link
Contributor

douglas-raillard-arm commented Apr 4, 2023

All I can find when looking for "persistent data path" is Unity, are we sure we want to add some app-specific concepts in AndroidTarget ? Unless there is an android API doc I missed. Maybe there is a related property that could just be queried (adb shell getprop) ?

@mrkajetanp
Copy link
Contributor Author

All I can find when looking for "persistent data path" is Unity, are we sure we want to add some app-specific concepts in AndroidTarget ?

That's the only place I found the name itself used but the path is definitely used by other apps as well here and there. It's analogous to the /data/data one but does not require root access. I'm not sure Android has any 'official' name for it which is why "persistent data path" is what I went with but if we have any better names then that works just as well I think.

@douglas-raillard-arm
Copy link
Contributor

That makes sense, does Android allow querying for that path in any way or do apps have to know it in advance ?

@mrkajetanp
Copy link
Contributor Author

Ah apparently it can, Android's Context has a method for getting the application's data directory:

https://developer.android.com/reference/android/content/Context#getExternalFilesDir(java.lang.String)

We could always rename this from persistent_data_path to external_files_path or dir?

@marcbonnici
Copy link
Collaborator

Looking into this more it looks like the persistent_data_path or external_files_path term actually refers a specific applications data path. From [1].

Returns absolute paths to application-specific directories on all shared/external storage devices where the application can place persistent files it owns.

So we would be providing the root of this directory and users would need to manually join the package name to this path.

IIUC looking at how this path is generated [2] it uses the external directory function (which we expose with external_storage) along with the constant sub-path Android/data. Therefore I think we can avoid adding a new parameter for this and instead add a convenience property with the equivalent of target.path.join(target.external_storage', 'Android', 'data').

As for the name perhaps to keep inline with the android terminology we could expose something like external_storage_app_dir? (Open to better naming suggestions).

What do you think?

[1] https://developer.android.com/reference/android/content/Context#getExternalFilesDir(java.lang.String)
[2] https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/os/Environment.java#225

@mrkajetanp mrkajetanp force-pushed the persistent-data-path branch 2 times, most recently from 76cb39e to cb43dd4 Compare April 19, 2023 11:26
@mrkajetanp
Copy link
Contributor Author

mrkajetanp commented Apr 19, 2023

instead add a convenience property

Ah yeah that makes a lot of sense, /sdcard is a symlink to /storage/emulated/0 so that works. This also allows us to do it exclusively here instead of needing a separate WA PR so that's a win.

What do you think?

The name seems fine to me. I've updated this PR accordingly and will close the WA one.

devlib/target.py Outdated Show resolved Hide resolved
FEATURE

Add a convenience property for AndroidTarget to expose Android's
external storage app directory path.
This path is used for some applications (such as Unity games) to
store persistent application data instead of '/data/data'.

Signed-off-by: Kajetan Puchalski <kajetan.puchalski@arm.com>
@mrkajetanp
Copy link
Contributor Author

Updated, sorry for the delay..

@marcbonnici marcbonnici merged commit be988bb into ARM-software:master May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants