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

Handle compilation and permissions for ANDROIDAPI>22. #257

Merged
merged 3 commits into from
Aug 30, 2019

Conversation

0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented Aug 29, 2019

Since Marshmallow permissions are not automatically granted as
specified in the Manifest.

As a consequence LN apps die when accessing the SDCard, e.g. when
unpacking embedded files.

This patch adds a fragile hack to enable an ifdef-alike trick to
exclude code portions when targeting older APIs and uses it to request
the permission at startup. Plus create the system-directory if it
does not already exists.

Since Marshmallow permissions are not automatically granted as
specified in the Manifest.

As a consequence LN apps die when accessing the SDCard, e.g. when
unpacking embedded files.

This patch adds a fragile hack to enable an ifdef-alike trick to
exclude code portions when targeting older APIs and uses it to request
the permission at startup.  Plus create the system-directory if it
does not already exists.
@mgorges
Copy link
Contributor

mgorges commented Aug 29, 2019

The conditional substitution of comments start and end tags is an interesting workaround! It is the requestPermissions which needs the 'Android Support Library' of at least version 24.1.0, so we'll need to make it mandatory, it previously was not. Also I thought the minimum supported API level for support libraries is API level 14, in which case we wouldn't need your ifdef like construct?

@0-8-15
Copy link
Contributor Author

0-8-15 commented Aug 29, 2019 via email

@0-8-15
Copy link
Contributor Author

0-8-15 commented Aug 29, 2019

The other pull request now includes the latest change, which preserves syntax highlighting at least.

@0-8-15
Copy link
Contributor Author

0-8-15 commented Aug 29, 2019

As it I might understand what you meant by "minumim API level 14", i.e., I could simply abstain from using "API level 23 and above": recent Android versions appear to kill my app while it is supposed to supply an ongoing service. The solution - which still requires further testing to build confidence, seems to be to go through more recent APIs (i.e., at least level 23) to convince Android to leave it running. That's the purpose the ForegroundService attempts to achieve.

This pull request is only a spin-off of the changes required, which need to go into LN core parts.

As a side effect I expect goggle to to deprecate more aspects of older API, like permission handling, which would eventually force users to change the minimum API level for one reason or another.

@mgorges
Copy link
Contributor

mgorges commented Aug 30, 2019

You need to target at minimum API 26 to submit to the play store, but your minimum API level can be as low as you like. This is relevant particularly for the global health app space, in which we do some things, so I don't want to raise it beyond what we actually need to. The minimum according to the example would be API 19, so going from 19 to 23 would drop almost 15% of devices currently in use.

From the documentation it seems that requestPermissions is part of the Android Support
Library, but even when I remove it I can still compile so I will merge this request shortly. I'd also prefer to merge this one and then the other one, instead of doing a single large merge, to keep things reasonably easy to figure out when they break.

@mgorges mgorges merged commit dfa0547 into part-cw:master Aug 30, 2019
@0-8-15 0-8-15 deleted the support-androidapi-above-22 branch September 2, 2019 17:18
@0-8-15 0-8-15 restored the support-androidapi-above-22 branch September 24, 2019 17:08
@0-8-15 0-8-15 deleted the support-androidapi-above-22 branch September 24, 2019 17:09
@0-8-15 0-8-15 restored the support-androidapi-above-22 branch September 24, 2019 17:14
@0-8-15 0-8-15 deleted the support-androidapi-above-22 branch September 24, 2019 17:15
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.

2 participants