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

A ForegroundService for Lambdanative. #256

Closed
wants to merge 10 commits into from

Conversation

0-8-15
Copy link
Contributor

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

Exports a single procedure: foreground-service!

A NOOP on anything but Android. On Android:

(foreground-service! #t) Start the service.

(foreground-service! #f) Stop the service.

The service will send EVENT_IDLE every 30 seconds to your application.

The main use is to prevent Android from aggressivly stopping the
application when it should keep running.

Exports a single procedure: `foreground-service!`

A NOOP on anything but Android.  On Android:

(foreground-service! #t)  Start the service.

(foreground-service! #f)  Stop the service.

The service will send EVENT_IDLE every 30 seconds to your application.

The main use is to prevent Android from aggressivly stopping the
application when it should keep running.
@mgorges
Copy link
Contributor

mgorges commented Aug 19, 2019

Doesn't loaders/android/bootstrap.java.in#L116 give you an EVENT_IDLE event every 1/4 second, which should keep things alive?

@mgorges
Copy link
Contributor

mgorges commented Aug 19, 2019

Also we used to do that with a thread-sleep (as you can see in commit 89d4703), but then switched to using a Timer (see commit d06577f2) about two years ago. There was a good reason for it but its lost as institutional memory.

@0-8-15
Copy link
Contributor Author

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

@mgorges
Copy link
Contributor

mgorges commented Aug 19, 2019

Okay thanks for looking into this further. If you still have the SDK installed, simply change the ANDROIDAPI in the SETUP file, clear the android build cache (make scrub is a bit much) and you should be good to go.

@0-8-15
Copy link
Contributor Author

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

@0-8-15
Copy link
Contributor Author

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

@mgorges
Copy link
Contributor

mgorges commented Aug 21, 2019

As for the SDK, you probably only will need to add the missing one with the sdk manager something like /opt/local/android-sdk-macosx/tools/bin/sdkmanager 'platforms;android-24' wherever you might have placed this in your system?

As for calling new APIs, there isn't and #ifdef equivalent in Java. You might be able to some Build.VERSION.SDK_INT tests, but if the functions are not declared before this won't compile on earlier SDKs. E.g. see modules/localnotification/ANDROID_java_activityadditions#L36

0-8-15 added 3 commits August 28, 2019 14:46
See comments in ANDROID_xml_permissions and
ANDROID_java_activityadditions how to compile for older API versions
and possible issues wrt. restrictions regarding google play store.
See comments in ANDROID_xml_permissions and
ANDROID_java_activityadditions how to compile for older API versions
and possible issues wrt. restrictions regarding google play store.
@0-8-15
Copy link
Contributor Author

0-8-15 commented Aug 28, 2019

There are still issues. After removing and reinstalling, at least one device with Android 8.1 does no longer start LN applications build with API version 26 at all. Reason: None of the permissions requested via Manifest are granted at all. They appear as requested, but are disabled.

By now I don't yet know how to fix this.

@mgorges
Copy link
Contributor

mgorges commented Aug 28, 2019

Thanks for the update, and working on the battery optimization piece. I believe @bclayton05 also encountered permission problems over the summer, however, for camera permissions? I don't recall details, but it might be that the requestPermission(); call needs to be made explicitly now, while previously it was automatic?

@bclayton05
Copy link
Contributor

I never fully understood where the problem was coming from, but I ended up solving my permission issues by explicitly confirming permissions each time the user wanted to interact with the desired permission-blocked feature. As @mgorges mentioned, explicitly requesting permissions when they hadn’t been granted was the solution.

0-8-15 added 4 commits August 29, 2019 17:33
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 30, 2019

If this really is required we should consider adding this to modules/eventloop/eventloop.scm, in the e.g. in eventloop/eventloop.scm#L264 for EVENT_TERMINATE and eventloop/eventloop.scm#L186 EVENT_INIT, and make the eventloop module depend on the androidforeground module? However, I don't know if this would break existing EVENT_RESUME and EVENT_SUSPEND functionality?

Alternatively, you could make at least the (foreground-service! #t) call automatic at the 'androidforeground/androidforeground.scm' function, but I don't know if it would get terminated by default when the parent app crashes or is closed?

@0-8-15
Copy link
Contributor Author

0-8-15 commented Sep 2, 2019

If this really is required we should consider adding this to modules/eventloop/eventloop.scm

I'm unsure. The eventloop is essentially always required on Android, isn't it? Many applications may not need to run all the time. I'd rather keep the foregroundservice separate.

Alternatively, you could make at least the (foreground-service! #t) call automatic at the 'androidforeground/androidforeground.scm' function, but I don't know if it would get terminated by default when the parent app crashes or is closed?

This might be easy enough to add at the cost of some flexibility. Especially when it comes to termination. (So far I'd seen the app die and the foregroundservice still in the notifications.)

However I'll have to understand more about termination and Android. Some docs indicate that one should abstain from ever explicitly terminating. Dragons ahead: Simply calling (terminate) leaves my app unable to restart. I have no idea why. Currently I'm overwriting terminate with a log output and have it do nothing. My code does never calls it, but still it's being called. ... I'll need to learn.

@0-8-15
Copy link
Contributor Author

0-8-15 commented Sep 2, 2019

@bclayton05: I'm trying to solve the camera permission here too.

Right now the app always dies, which permission did you request explicitely and when/where in the source. Right now my LN app still dies as son as I try to access the camera. Though this might be some other stupidity on my part.

@mgorges
Copy link
Contributor

mgorges commented Sep 2, 2019

// make sure permissions are met. requestPermission() results in this being called for the specified permission.
@Override
public void onRequestPermissionsResult(int requestCode, String[] permissions, int[] grantResults) {
    super.onRequestPermissionsResult(requestCode, permissions, grantResults);

    switch (requestCode) {
        case REQUEST_CAMERA_PERMISSION_RESULT: // Camera access permission
            if(grantResults[0] != PackageManager.PERMISSION_GRANTED) { // If permission was not granted.
                Toast.makeText(getApplicationContext(),
                        "Application cannot run without access to camera", Toast.LENGTH_LONG).show(); // Tell user app can't run without it.
            }
            break;
        case REQUEST_WRITE_EXTERNAL_STORAGE_PERMISSION_RESULT: // Permission to write to storage. Necessary to make video and vital sign files.
            if(grantResults[0] != PackageManager.PERMISSION_GRANTED) { // If permission was not granted.
                Toast.makeText(getApplicationContext(),
                        "Application cannot run without permission to write to storage", Toast.LENGTH_LONG).show(); // Tell user app can't run without it.
            } else { // If permission was granted, update UI to make app seem snappier, even though the MediaRecorder still needs to start, 
                                                                                      //which takes a long time (relatively speaking)
                mRecordImageButton.setImageBitmap(squircle); // Show the "stop" button (squircle refers to square circle because it's a rounded square).
                oximeterStatus.setText("Recording"); // Notify user that the recording has begun.
                if(mPatientId == null) { // If patient ID doesn't exist (which is impossible, but checked here for robustness).
                    topToolbar.setBackgroundColor(0x00000000);
                    topToolbar.removeAllViews(); // Remove top toolbar to be more immersive.
                }
                timer.start(); // Start the timer that displays recording length.
                Toast.makeText(getApplicationContext(),
                        "Permission Granted Successfully", Toast.LENGTH_LONG).show(); // Notify user permission was successfully granted.
            }
            break;
        case REQUEST_INTERNET_PERMISSION_RESULT: // Internet USE permission
            if(grantResults[0] != PackageManager.PERMISSION_GRANTED) { // If permission isn't granted. (most phones don't even need to request for this one)
                Toast.makeText(getApplicationContext(),
                        "Application cannot run without access to internet", Toast.LENGTH_LONG).show(); // Tell user that the app needs it.
            }
            break;
        case REQUEST_ACCESS_NETWORK_STATE_PERMISSION_RESULT: // Internet CONNECTION state permission
            if(grantResults[0] != PackageManager.PERMISSION_GRANTED) { // If it isn't granted (most phones don't even need to request for this one)
                Toast.makeText(getApplicationContext(),
                        "Application cannot run without access to network state", Toast.LENGTH_LONG).show(); // Tell user app can't run without it.
            }
            break;
    }
}


// Connect camera using android.hardware.camera2 API
private void connectCamera() {
    CameraManager cameraManager = (CameraManager) getSystemService(Context.CAMERA_SERVICE); // Retrieve Camera Manager object.
    try {
        if(Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { // If our build is for marshmallow (API 23) or higher (which it always is, unless things have changed since 2019)
            if(getApplicationContext().checkSelfPermission(Manifest.permission.CAMERA) ==
                    PackageManager.PERMISSION_GRANTED) { // Check our permission for camera access. It's required for this step.
                cameraManager.openCamera(mCameraId, mCameraDeviceStateCallback, null); // open the camera!
            } else {
                if(shouldShowRequestPermissionRationale(Manifest.permission.CAMERA)) { // If we didn't get permission, check if the user has previously denied it.
                    Toast.makeText(this,
                            "VidOx requires access to camera", Toast.LENGTH_LONG).show(); // If they've denied it before, tell them again that we need it.
                }
                requestPermissions(new String[] {Manifest.permission.CAMERA}, REQUEST_CAMERA_PERMISSION_RESULT); // Ask for the permission again!
            }
        } else {
            cameraManager.openCamera(mCameraId, mCameraDeviceStateCallback, null); // Builds lower than API 23 don't need permission, so you can just open the camera.
        }
    } catch (CameraAccessException e){
        e.printStackTrace(); // Print the Exception to the stack trace.
    }

}

@mgorges
Copy link
Contributor

mgorges commented Sep 2, 2019

There might be more pieces - this is from an app doing video things by @bclayton05

@0-8-15
Copy link
Contributor Author

0-8-15 commented Sep 3, 2019

Looks like the culprit is another pitfall in newer Android APIs (here 26), which makes the approach to use startActivityForResult to obtain a photo fragile.

Question: How would I redirect (log or log alike) messages from Java code to the LN logfile?

The exception I ran into with simply requesting CAMERA permissions (after those being granted):

Error android.os.FileUriExposedException: file:///sdcard/front/_camera_tmp.jpg exposed beyond app through ClipData.Item.getUri()
android.os.FileUriExposedException: file:///sdcard/front/_camera_tmp.jpg exposed beyond app through ClipData.Item.getUri()
        at android.os.StrictMode.onFileUriExposed(StrictMode.java:1960)
        at android.net.Uri.checkFileUriExposed(Uri.java:2356)
        at android.content.ClipData.prepareToLeaveProcess(ClipData.java:942)
        at android.content.Intent.prepareToLeaveProcess(Intent.java:9850)
        at android.content.Intent.prepareToLeaveProcess(Intent.java:9835)
        at android.app.Instrumentation.execStartActivity(Instrumentation.java:1610)
        at android.app.Activity.startActivityForResult(Activity.java:4501)
        at android.app.Activity.startActivityForResult(Activity.java:4459)
        at net.softeyes.front.front.startCamera(front.java:371)
        at net.softeyes.front.myRenderer.nativeEvent(Native Method)
        at net.softeyes.front.myRenderer.pointerEvent(front.java:548)
        at net.softeyes.front.xGLSurfaceView$2.run(front.java:477)
        at android.opengl.GLSurfaceView$GLThread.guardedRun(GLSurfaceView.java:1500)
        at android.opengl.GLSurfaceView$GLThread.run(GLSurfaceView.java:1270)

@mgorges
Copy link
Contributor

mgorges commented Sep 4, 2019

I am currently not aware of a way to redirect Android crash dumps into LambdaNative through the JNI interface or any other approach. In the Debugging section of the Wiki we do refer only to logcat which is what you also used here.
In principle I do think this is something that would make things easier from our perspective, but it might also confuse people who use the typical Android approach and expect it to show there (too)? Thus if you come across something that allows us to get a redirected copy that would be great.

@0-8-15
Copy link
Contributor Author

0-8-15 commented Sep 5, 2019 via email

@0-8-15
Copy link
Contributor Author

0-8-15 commented Sep 5, 2019 via email

@mgorges
Copy link
Contributor

mgorges commented Sep 5, 2019

The open question for me is how I would achieve to use just one logfile.

If you can get it into JNI, there is a log_c function, which you can call with a string.

@mgorges
Copy link
Contributor

mgorges commented Sep 5, 2019

The issue is related to android.os.StrictMode.onFileUriExposed.

Did you look at this: https://stackoverflow.com/questions/44821017/fileuriexposedexception-using-android-7, in particular their workaround?

@0-8-15
Copy link
Contributor Author

0-8-15 commented Sep 5, 2019 via email

@0-8-15
Copy link
Contributor Author

0-8-15 commented Sep 6, 2019 via email

@mgorges
Copy link
Contributor

mgorges commented Oct 8, 2020

It seems that this is becoming a needed feature for #339, however it has merge conflicts in loaders/android/boostrap.java.in and also unrelated changes to packtool, some more of the ifdef API23 pieces we already have? Can I rebase this against master in a branch I don't own to start a new clean copy or would you have to?

@0-8-15
Copy link
Contributor Author

0-8-15 commented Oct 8, 2020 via email

@mgorges
Copy link
Contributor

mgorges commented Oct 8, 2020

Okay, then I will await an update and in the meantime leave this untouched. Thanks.

@0-8-15
Copy link
Contributor Author

0-8-15 commented Oct 12, 2020 via email

@mgorges
Copy link
Contributor

mgorges commented Oct 13, 2020

Maybe some of the others on our team can help with this guidance? Otherwise we'll have to defer this by two weeks as I need to focus on a grant submission deadline. Sorry, but I simply don't have the capacity left as I am already behind.

Please don't remove support for older versions.  I still need to
compile for at least API 19.
@0-8-15
Copy link
Contributor Author

0-8-15 commented Nov 30, 2020

BTW: I have to blame myself wrt. this module/PR no longer working since two days.

Apparently I managed to NOT include all the required files AND delete them after the move from the development directory into the pull request. Which amounts to me loosing a couple of days of research and testing.

I still need this to be resolved, but ... I'll have to redo it. This testing area was yet under version control even in the private branch. Too bad. My bad.

@mgorges
Copy link
Contributor

mgorges commented Nov 30, 2020

Sorry to hear that - I can relate how frustrating this is; the last time I did something like it TimeMachine saved me but that was lucky as my work machine didn't have it.

@0-8-15
Copy link
Contributor Author

0-8-15 commented Dec 2, 2020

Replacement version in PR #404

@0-8-15
Copy link
Contributor Author

0-8-15 commented Dec 2, 2020

Closing in favor of #404

@0-8-15 0-8-15 closed this Dec 2, 2020
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