Exception in TeleportService

[davidvavra]

Sometimes I get Exception in TeleportService running on the wearable, usually it's happening when I install the app for the first time and launch it. Stacktrace:

07-26 18:41:49.694 17549-17576/? E/AndroidRuntime﹕ FATAL EXCEPTION: WearableListenerService Process: cz.destil.wearsquare, PID: 17549 java.lang.IllegalStateException: Cannot execute task: the task is already running. at android.os.AsyncTask.executeOnExecutor(AsyncTask.java:576) at android.os.AsyncTask.execute(AsyncTask.java:535) at com.mariux.teleport.lib.TeleportService.onDataChanged(TeleportService.java:70) at com.google.android.gms.wearable.WearableListenerService$a$1.run(Unknown Source) at android.os.Handler.handleCallback(Handler.java:733) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:136) at android.os.HandlerThread.run(HandlerThread.java:61) 07-26 18:41:49.874 17549-17549/? E/TeleportClient﹕ disconnect

[Mariuxtheone]

Looks like you're syncing multiple Data on app start, launching more sync methods. It generates exception because the AsyncTask onSyncDataItemTask is called more than once.

[davidvavra]

Isn't that something that the library should handle? Why not to sync multiple data? On Jul 27, 2014 8:57 AM, "Mario Viviani" notifications@github.com wrote:

Looks like you're syncing multiple Data on app start, launching more sync methods. It generates exception because the AsyncTask onSyncDataItemTask is called more than once.

— Reply to this email directly or view it on GitHub https://github.com/Mariuxtheone/Teleport/issues/3#issuecomment-50258433.

[Mariuxtheone]

The app can sync multiple data (the sync methods are asynchronous, so should be no problems there). Looks like the issue here is with your wear activity not resetting the OnSyncDataItemTask and reusing the same task instance. Can you please provide snippets of your app data syncing and receiving methods?

[davidvavra]

Hi, thanks the code is here:

GitHub.com/destil/wearsquare On Jul 28, 2014 11:08 AM, "Mario Viviani" notifications@github.com wrote:

The app can sync multiple data (the sync methods are asynchronous, so should be no problems there). Looks like the issue here is with your wear activity not resetting the OnSyncDataItemTask and reusing the same task instance. Can you please provide snippets of your app data syncing and receiving methods?

— Reply to this email directly or view it on GitHub https://github.com/Mariuxtheone/Teleport/issues/3#issuecomment-50321060.

[Mariuxtheone]

Can you please share the full stacktrace of the error so I can track down the issue? Thanks

[davidvavra]

Full stacktrace is in the description, there is nothing more in the log On Jul 28, 2014 12:05 PM, "Mario Viviani" notifications@github.com wrote:

Can you please share the full stacktrace of the error so I can track down the issue? Thanks

— Reply to this email directly or view it on GitHub https://github.com/Mariuxtheone/Teleport/issues/3#issuecomment-50325512.

[davidvavra]

Hi, I did some more testing and the error is still there.

When I'm reusing same task instance, it crashes every time after first launch.

But it crashes sometimes even when I'm always creating a new instance.

Recent stacktrace:

08-07 08:51:59.564    2239-2260/? E/AndroidRuntime﹕ FATAL EXCEPTION: WearableListenerService
    Process: cz.destil.wearsquare, PID: 2239
    java.lang.IllegalStateException: Cannot execute task: the task is already running.
            at android.os.AsyncTask.executeOnExecutor(AsyncTask.java:576)
            at android.os.AsyncTask.execute(AsyncTask.java:535)
            at com.mariux.teleport.lib.TeleportService.onDataChanged(TeleportService.java:70)
            at com.google.android.gms.wearable.WearableListenerService$a$1.run(Unknown Source)
            at android.os.Handler.handleCallback(Handler.java:733)
            at android.os.Handler.dispatchMessage(Handler.java:95)
            at android.os.Looper.loop(Looper.java:136)
            at android.os.HandlerThread.run(HandlerThread.java:61)

Related class:

public class ListenerService extends TeleportService {

    @Override
    public void onCreate() {
        super.onCreate();
        setOnSyncDataItemTask(new SyncDataTask());
    }

    class SyncDataTask extends OnSyncDataItemTask {
        @Override
        protected void onPostExecute(DataMap data) {
            if (data.containsKey("error_message")) {
                DebugLog.d("error");
                App.bus().post(new ErrorEvent(data.getString("error_message")));
            } else if (data.containsKey("check_in_venues")) {
                DebugLog.d("check in");
                new CheckInVenuesTask(data).start();
            } else if (data.containsKey("explore_venues")) {
                DebugLog.d("explore");
                new ExploreVenuesTask(data).start();
            } else if (data.containsKey("image_url")) {
                new ProcessImageTask(data).start();
            }
            setOnSyncDataItemTask(new SyncDataTask());
        }
    }

    class CheckInVenuesTask extends BaseAsyncTask {

        private DataMap data;

        CheckInVenuesTask(DataMap data) {
            this.data = data;
        }

        @Override
        public void inBackground() {
            List<CheckInAdapter.Venue> venues = new ArrayList<CheckInAdapter.Venue>();
            ArrayList<DataMap> dataVenues = data.getDataMapArrayList("check_in_venues");
            for (DataMap dataVenue : dataVenues) {
                venues.add(new CheckInAdapter.Venue(dataVenue.getString("id"), dataVenue.getString("name"), dataVenue.getString("image_url")));
            }
            App.bus().post(new CheckInVenueListEvent(venues));
        }

        @Override
        public void postExecute() {

        }
    }

    class ExploreVenuesTask extends BaseAsyncTask {

        private DataMap data;

        ExploreVenuesTask(DataMap data) {
            this.data = data;
        }

        @Override
        public void inBackground() {
            List<ExploreAdapter.Venue> venues = new ArrayList<ExploreAdapter.Venue>();
            ArrayList<DataMap> dataVenues = data.getDataMapArrayList("explore_venues");
            for (DataMap dataVenue : dataVenues) {
                venues.add(new ExploreAdapter.Venue(dataVenue.getString("id"), dataVenue.getString("name"), dataVenue.getString("tip"),
                        dataVenue.getDouble("latitude"), dataVenue.getDouble("longitude"), dataVenue.getString("image_url")));
            }
            App.bus().post(new ExploreVenueListEvent(venues));
        }

        @Override
        public void postExecute() {

        }
    }

    class ProcessImageTask extends BaseAsyncTask {

        private DataMap data;

        ProcessImageTask(DataMap data) {
            this.data = data;
        }

        @Override
        public void inBackground() {
            Asset asset = data.getAsset("asset");
            String imageUrl = data.getString("image_url");
            Bitmap bitmap = null;
            if (asset != null) {
                bitmap = loadBitmapFromAsset(asset);
            }
            DebugLog.d("image loaded: " + imageUrl);
            App.bus().post(new ImageLoadedEvent(imageUrl, bitmap));
        }

        @Override
        public void postExecute() {

        }
    }
}

Full sourcecode is available here: https://github.com/destil/wearsquare

Can you help please?

[thedamfr]

@Mariuxtheone I really think you should replace the setter by a factory method. I have the same problem. The wearable is sending data every 500ms to the phone and I got the error most of the time.

[davidvavra]

@thedamfr Do you have some workaround that fixes this issue? It's blocking me.

[davidvavra]

I found out that the AsyncTask is not neccessary at all. See referenced workaround.

[Mariuxtheone]

@destil I can see what you did. The fact is that it could be done for TeleportService (because the Services needs to extend TeleportService) but TeleportClient is a wrapper class, so you can't override a method there. So while it's true that an AsyncTask could not be the perfect way to go for a Service, we don't want the user to reimplement onDataChanged() in their classes. Or do you think that TeleportClient too should be a parent class?

[davidvavra]

@Mariuxtheone Just make the developer implement abstract method with simpler version of onDataChanged() when extending from TeleportService. Activities can stay the same, I don't like libraries which required inheriting from some BaseActivity or BaseFragment.

[thedamfr]

You don't need to do inheritance or else.

But instead of setting a single fire and forget asynctask, the developper should implement a factory method so each time an event comes a new asynctask is built.

mTeleportClient.setOnDataChangedAsyncTaskFactory (
    new OnDataChangedAsyncTaskFactory () {
         @Override
         public void makeAsyncTask() {
             return new CustomAsyncTask();
         }
    }
 );

Thus, the corner case where the AsyncTask is not finished when the event come should not be possible any more.

[Mariuxtheone]

@thedamfr got it. I will start experimenting with this, because I think it could be possible to preserve the actual behavior and add the factory method pattern. However, if you feel confident and want to propose a pull request, I will more than gladly consider it!

[thedamfr]

I've done it on local to fix the bug for me. I'm going to push it. And you are right, I thing we can do both, but one should override this. If there is a Factory set and an AsyncTask set, which should be used ?

[thedamfr]

Okay !

I've done two PR.

The first one solve the problem using a Factory like previously said. The second one solve the problem using a simple Callback rather than an AsyncTask. Since the AsyncTask does no job, the CallBack is a solution in a certain way. It makes the user have to handle threading by himself.

I let you pick one or both.

[Mariuxtheone]

@thedamfr Excellent, let me check both. I'm testing an alternative solution with preserving the AsyncTask, creating a new instance of the tasks without the need of a builder. However, I'm inclined to go for the Builder pattern more than the Callback one.

[davidvavra]

@Mariuxtheone I would be for callback - if there is no need to create AsyncTask, don't use it.

[thedamfr]

Moreover, Asynctask have quiet memory footprint. building serveral of them would hit badly the DVM.

I would put both actually.

Note that those pull-request don't remove the actual asynctask system.

[Mariuxtheone]

Could be a good solution to add both. Usually I tend to prefer asynchronous tasks or handlers due to not impact on the main UI thread, but a sync solution with Callback might do the trick in some cases.

[thedamfr]

You may not be on the UI Thread when the message come. And you could want to do threading yourself or with Robospice. Moreover you could have no background work to do, just a UI change to do. In this case, the Asynctask cost more than the callback

[Mariuxtheone]

@thedamfr right. I think having all of these solutions (simple fire-and-forget AsyncTask, AsyncTask builders for intensive background tasks and Callbacks for non-UI-related tasks) could provide sufficient flexibility to Teleport. I'm thinking of embedding all the two pull requests into a brand new release without directly merging them. Is it ok?

[thedamfr]

I would really like you to keep my authorship on my commits :-/

[Mariuxtheone]

@thedamfr no problem! Would you like to submit a unique PR with both features included to ease the merge process? :-)

[thedamfr]

I can do that :)

[thedamfr]

And that's done

[Mariuxtheone]

@thedamfr cool, let me check it then I'll merge it. I see you only updated the TeleportClient, I will take care of updating TeleportService too.

[davidvavra]

@Mariuxtheone How is new version of Teleport coming?

[thedamfr]

@DavidVavra You can still use my fork if you need a quick fix.

Damien

---

Ce message, ainsi que l'ensemble des fichiers qui lui sont attachés, contiennent des informations strictement confidentielles destinées exclusivement à son destinataire ou à toute autre personne autorisée à y avoir accès. Si vous n'êtes pas le destinataire de ce message ou que vous l'avez reçu par erreur, vous êtes tenus de nous prévenir en réponse à cet e-mail puis d'effacer le message de votre ordinateur. Toute copie ou autre utilisation du contenu de ce message sont strictement interdites et punies par la loi.

2014-08-25 12:04 GMT+02:00 David Vavra notifications@github.com:

@Mariuxtheone https://github.com/Mariuxtheone How is new version of Teleport coming?

— Reply to this email directly or view it on GitHub https://github.com/Mariuxtheone/Teleport/issues/3#issuecomment-53247396.

[Mariuxtheone]

Hi guys, sorry for the long delay. I just reviewed the Pull Request and I'm in the process of merging it. Thanks again @thedamfr for it, well done! :-D

[thedamfr]

Thanks to you :)