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
Exception in TeleportService #3
Comments
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. |
Isn't that something that the library should handle? Why not to sync
|
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? |
Hi, thanks the code is here: GitHub.com/destil/wearsquare
|
Can you please share the full stacktrace of the error so I can track down the issue? Thanks |
Full stacktrace is in the description, there is nothing more in the log
|
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:
Related class:
Full sourcecode is available here: https://github.com/destil/wearsquare Can you help please? |
@Mariuxtheone I really think you should replace the setter by a factory method. |
@thedamfr Do you have some workaround that fixes this issue? It's blocking me. |
I found out that the AsyncTask is not neccessary at all. See referenced workaround. |
@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? |
@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. |
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. |
@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! |
I've done it on local to fix the bug for me. I'm going to push it. |
Okay ! I've done two PR. The first one solve the problem using a Factory like previously said. I let you pick one or both. |
@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. |
@Mariuxtheone I would be for callback - if there is no need to create AsyncTask, don't use it. |
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. |
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. |
You may not be on the UI Thread when the message come. |
@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 would really like you to keep my authorship on my commits :-/ |
@thedamfr no problem! Would you like to submit a unique PR with both features included to ease the merge process? :-) |
I can do that :) |
And that's done |
@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. |
@Mariuxtheone How is new version of Teleport coming? |
@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, 2014-08-25 12:04 GMT+02:00 David Vavra notifications@github.com:
|
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 |
Thanks to you :) |
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
The text was updated successfully, but these errors were encountered: