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

542 write external storage deprecated #543

Merged

Conversation

HilaryN
Copy link
Contributor

@HilaryN HilaryN commented Dec 29, 2023

To fix #542

I haven't touched the READ permission request in MapPack as I think that isn't working at the moment?

Write permissions for app-specific storage (i.e. map tiles) is no longer required. Same for Read permission for app-specific storage (which doesn't seem to get explicitly requested, presumably as it's granted as part of the write permission). Removed for Android 13 upwards. (CycleMapFragment)
Note that because permission for map tiles is no longer granted, the old cache location won't get removed – I'm not sure if this is an issue? (See CycleMapFragment.onRequestPermissionsResult)

CycleMapView – I've just changed the log message in here.

Write permission to store photos should be handled by the camera app – removed for Android 13 upwards. (AddPhotoFragment)

Read permission to access photos - changed to READ_MEDIA_IMAGES for Android 13 upwards.

Tested on Pixel 3 API 28 (Android 9) emulator, Fairphone 2 (Android 10), Fairphone 3 (Android 13).

…gher.

Remove @RequiresApi = M as minSDK is now 23 (=M).
… as this can be handled by the camera app.

Change READ_EXTERNAL_STORAGE to READ_MEDIA_IMAGE for Android 13 upwards.
@mvl22
Copy link
Member

mvl22 commented Dec 30, 2023

Thanks for looking at this, Hilary. I'll leave others to review but it's great to see this being addressed.

@mvl22
Copy link
Member

mvl22 commented Jan 28, 2024

@jezhiggins @oliverlockwood Could one of you kindly review this? Seems this will also partly deal with #544.

@jezhiggins jezhiggins merged commit 957844c into cyclestreets:master Jan 31, 2024
1 check passed
@mvl22
Copy link
Member

mvl22 commented Jan 31, 2024

Thanks for the review :)

Are we able to roll a release? Another report about missing map tiles recently.

@jezhiggins
Copy link
Member

Now this has been merged, the beta release should go out automatically and chances are people will have it by the morning. Assuming that looks ok, we can promote it to a regular release in the next few days.

@mvl22
Copy link
Member

mvl22 commented Feb 1, 2024

Awesome, thanks all. Will look forward to the release notes appearing.

@HilaryN
Copy link
Contributor Author

HilaryN commented Feb 3, 2024

I don't think this has made it to the Play Store yet. I tried re-installing from there but the issue is still occurring.

@jezhiggins
Copy link
Member

jezhiggins commented Feb 3, 2024 via email

@jezhiggins
Copy link
Member

Something up with Travis - it's stopped building because some kind of problem with our plan. We're on their open source software plan so not sure what's going on, hopefully just a cock up. I've emailed support, they were very quick to respond last time so fingers crossed.

@oliverlockwood
Copy link
Member

FYI, this happened to the OSS repos we (mostly I) maintain at my employer, too. Travis seems to be gradually making more and more hoops for OSS to jump through, it seems as though in their push to become profitable they're trying to discourage OSS usage while still claiming they support it for free.

I've migrated those to Github Actions, it wasn't too tough and I have a reference point there (though admittedly not for an Android build). I'll take an action to look into doing likewise for the CycleStreets CI at some point.

@oliverlockwood
Copy link
Member

@jezhiggins
Copy link
Member

Travis have done whatever the need to do, and the build is running again for now. Beta has been pushed up to Google Play so hopefully will make it's way out this evening

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.

Can't add photos to photomap any more
4 participants