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

Alter device.freeDisk to collect usable space in data directory #589

Merged
merged 3 commits into from
Sep 12, 2019

Conversation

fractalwrench
Copy link
Contributor

Goal

device.freeDisk currently collects the size of the external storage directory, which is not helpful
when debugging errors that occurred in a low disk space situation. This changeset gets the /data directory and calculates the usable space in bytes (to non-root users) which is a more useful measure of the device's disk space.

Tests

Verified that the reported value was approximately 5.1Gb on a physical Nexus 4 device.

…ge /data directory

device.freeDisk currently collects the size of the external storage directory, which is not helpful
when debugging low disk space errors. This changeset gets the /data directory and calculates the
usable space in bytes (to non-root users) which is a more useful measure of whether the device is
nearly out of space
Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

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

I think I understand, but I don't know what the tradeoffs are here.

  • Is anything being lost here by changing implementations?
  • Was the new implementation not available as an option when calculateFreeDisk() was initially implemented, or did it serve some other purpose?
  • Should the old value (or the new value) be present in metadata as a separate key?
  • The old implementation appears to display the smaller of two values, comparing internal vs external available space. Is this no longer feasible or not important?

@fractalwrench
Copy link
Contributor Author

The old calculation for freeDisk calculated the total amount of disk space and is therefore misleading in the way we present that information, as the name suggests its the amount of available free bytes, rather than the total.

Android distinguishes between internal storage (which is always present) and external storage (which can be unmounted/detached from the device). The majority of apps write their files to internal storage, because writing files to external storage requires additional permissions that must be added to the manifest. Additionally, there are upcoming changes in the form of scoped storage which will make use of external storage less desirable for most use-cases.

This change intends to capture the most useful information by default. Although I agree it's worth thinking about whether it makes sense to collect other information around external storage disk space by default too as some separate metadata tab.

@fractalwrench fractalwrench dismissed kattrali’s stale review September 12, 2019 08:08

review points addressed

@fractalwrench fractalwrench merged commit bd54113 into master Sep 12, 2019
@fractalwrench fractalwrench deleted the alter-free-disk branch September 12, 2019 08:41
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