PileProject / drive

The drive project
Other
5 stars 4 forks source link

Add RxObservableProgram for program execution #55

Closed mandaiy closed 8 years ago

mandaiy commented 8 years ago

This commit removes ExecutionThread class. Instead of the class, RxObservableProgram is added, which is Rx version of the Thread class. This commit also renames the name of MachineController#finalize' toMachineController#close'

Changes of ExecutionActivity are also made in this commit.

Closes #33

mandaiy commented 8 years ago

Note that despite the implementation is exactly same as the ExecutionThread, the execution will be stopped unexpectedly if you

  1. stop (pause in this commit-term) the program and then
  2. restart the program The cause was accessing out of index in ExecutionCondition.
mandaiy commented 8 years ago

Note that despite the implementation is exactly same as the ExecutionThread, the execution will be stopped unexpectedly if you

  1. stop (pause in this commit-term) the program and then
  2. restart the program The cause was accessing out of index in ExecutionCondition.

Problem solved. The cause was that decrementProgramCount was called multiple times in here

tiwanari commented 8 years ago

NOTE: I replied a comment about END

mandaiy commented 8 years ago

@tiwanari I added some changes so if you have time please take a look on it.

tiwanari commented 8 years ago

FYI, you may dismiss a review by following this article to request another review when you modify codes significantly. This is a somewhat dangerous feature though.

tiwanari commented 8 years ago

Oh, I'm very sorry. The problem I described in previous two comments (UPDATE: I deleted them) was caused by a problem of mine. I checked they work correctly for the cases with the latest commit 👍

By the way, I found our app crashes when I enable Bluetooth function after pushing Execute button. If the Bluetooth function is off, the app asks me to turn on the Bluetooth and I push OK then I get the message "NxtDrive crashed unexpectedly."

I dug into this and found this is caused by ProgressFragment. Please see the following log;

11-04 00:30:50.975: E/AndroidRuntime(23044): FATAL EXCEPTION: main
11-04 00:30:50.975: E/AndroidRuntime(23044): java.lang.RuntimeException: Failure delivering result ResultInfo{who=null, request=1, result=-1, data=null} to activity {com.pileproject.drive.nxt/com.pileproject.drive.execution.ExecutionActivity}: java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.app.ActivityThread.deliverResults(ActivityThread.java:3071)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.app.ActivityThread.handleSendResult(ActivityThread.java:3114)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.app.ActivityThread.access$1100(ActivityThread.java:127)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1211)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.os.Handler.dispatchMessage(Handler.java:99)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.os.Looper.loop(Looper.java:137)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.app.ActivityThread.main(ActivityThread.java:4558)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at java.lang.reflect.Method.invokeNative(Native Method)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at java.lang.reflect.Method.invoke(Method.java:511)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:784)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:551)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at dalvik.system.NativeStart.main(Native Method)
11-04 00:30:50.975: E/AndroidRuntime(23044): Caused by: java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.support.v4.app.FragmentManagerImpl.checkStateLoss(FragmentManager.java:1493)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.support.v4.app.FragmentManagerImpl.enqueueAction(FragmentManager.java:1511)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.support.v4.app.BackStackRecord.commitInternal(BackStackRecord.java:638)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.support.v4.app.BackStackRecord.commit(BackStackRecord.java:617)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.support.v4.app.DialogFragment.show(DialogFragment.java:139)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at com.pileproject.drive.util.fragment.ProgressDialogFragment.showDialog(ProgressDialogFragment.java:53)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at com.pileproject.drive.execution.ExecutionActivity.connectToMachine(ExecutionActivity.java:234)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at com.pileproject.drive.execution.ExecutionActivity.onActivityResult(ExecutionActivity.java:282)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.app.Activity.dispatchActivityResult(Activity.java:4839)
11-04 00:30:50.975: E/AndroidRuntime(23044):    at android.app.ActivityThread.deliverResults(ActivityThread.java:3067)
11-04 00:30:50.975: E/AndroidRuntime(23044):    ... 11 more

Could you fix it in this PR?

mandaiy commented 8 years ago

The problem I described in previous two comments was caused by a problem of mine.

What was your problem? just curious.

I found our app crashes when I enable Bluetooth function after pushing Execute button. If the Bluetooth function is off, the app asks me to turn on the Bluetooth and I push OK then I get the message "NxtDrive crashed unexpectedly."

Might be same cause as the AlertFragmentDialog's one. Basically the ProgressFragments do not have to store their states so the bug could be fixed by the same approach.

tiwanari commented 8 years ago

What was your problem? just curious.

I had a trouble with installing this app into a device via micro-USB (I figured out that it was the problem of a micro-USB of mine), so I made an apk and installed it by using Dropbox. Then, when I tried to execute a program with the installed app, I caught that exception. But it doesn't occur when I install the app via micro-USB.

I don't know why but I guess this will not happen in the standard way (e.g., downloading it from PlayStore). I'm really sorry for that.

tiwanari commented 8 years ago

Here is a brief patch (diff.patch) I used to solve the bug;

diff --git app/src/main/java/com/pileproject/drive/util/fragment/ProgressDialogFragment.java app/src/main/java/com/pileproject/drive/util/fragment/ProgressDialogFragment.java
index 9446d38..03d8d8e 100644
--- app/src/main/java/com/pileproject/drive/util/fragment/ProgressDialogFragment.java
+++ app/src/main/java/com/pileproject/drive/util/fragment/ProgressDialogFragment.java
@@ -23,6 +23,7 @@ import android.os.Bundle;
 import android.support.annotation.StringRes;
 import android.support.v4.app.DialogFragment;
 import android.support.v4.app.FragmentManager;
+import android.support.v4.app.FragmentTransaction;
 import android.text.TextUtils;

 /**
@@ -50,7 +51,10 @@ public class ProgressDialogFragment extends DialogFragment {
      */
     public static void showDialog(FragmentManager manager, String title, String message, String tag) {
         ProgressDialogFragment f = newInstance(title, message);
-        f.show(manager, tag);
+
+        FragmentTransaction transaction = manager.beginTransaction();
+        transaction.add(f, tag);
+        transaction.commitAllowingStateLoss();
     }

     /**

Please use this with the following command in your drive repository;

patch -p0 < diff.patch
mandaiy commented 8 years ago

Hey @tiwanari Sorry for my late reply. Applied your patch and if Travis approve the patch, I'll merge this. Thanks.

tiwanari commented 8 years ago

It's all right! :) This PR looks so nice! Thanks, too! 👍