search

benjymous / MWM-for-Android

Meta Watch Manager Android SDK project for watch wrangling, remote protocol handling and notification routing for the Meta Watch wearable development systems. Includes an Intents based API for quick experiments.
www.metawatch.org
Apache License 2.0
59 stars 30 forks source link

MetawatchService Performance Issues with ICS #60

Closed rndusr0 closed 11 years ago

rndusr0 commented 11 years ago

Hi - I sent you a message over on the Metawatch forums, but not sure how often you are on there. I'm new to the whole github thing, so I hope this is okay...

I recently upgraded my phone to an ICS based ROM, and I was finding that if my watch was out of range, and MWM was in the 'Connecting' state, my Wifi connection would die, and MWM would seem to be working the CPU a lot, which was odd. So I did a bit of digging in the MWM service, and in the process of trying to work out what was going on, reworked a few things - I think that the trouble was that the main thread that does the communication with the watch was using a while loop, and Thread.sleep() for 3s in the run() method when connecting, and from what I could make out, the thread was not sleeping properly, and kept calling connect() in very quick succession which, for some completely random reason (Android driver issue, perhaps?) was causing my WiFi connection to drop out.

Anyway, I reworked the thread in the service to use a Looper/Handler message queue for its run() method, which I believe is the correct Android way of doing these things, and increased the reconnect try time to 10s, which seems to have fixed my dropped WiFi and high CPU usage problems. While I was in there, it seems that partial wakelocks were being taken out in a couple of places for 5s or 10s, but not explicitly released if the process completed before that, which is most of the time, so I put in some calls to explicitly release the locks too.

Also, it seemed that it was possible for the service start/stop button to get out of sync with the actual service state, which I think I've fixed.

I've been using my version for a couple of days, and my dropped wifi problem has not re-appeared, and my battery usage by MWM & Bluetooth seems to have improved. That said, this is my first bit of Android development, although I am a commercial programmer, so my changes might not be even near 100% right :-)

Give me a shout back if this is of any use...

Cheers,

Chris

benjymous commented 11 years ago

Awesome, I'll take a look and merge it in when I get a free moment.

I tend to check the forums daily, but annoyingly there's no kind of notification when you receive a private message, so I often miss those!

I noticed a similar problem with constant reconnect attempts when you disable bluetooth, and have the "connect at startup" option enabled - it constantly spams you with "bluetooth disabled" toasts - by the sounds of things your changes should help there too.

rndusr0 commented 11 years ago

Hi,

The bluetooth disabled toasts should now only appear every 10s, as the connect() method gets called from the looper in the ServiceThread. I'm not sure it even needs to be a toast, it could maybe just be another ConnectionState and handled in ProcessState(), what do you think?

One odd thing I found was this bit in MetaWatch.onCreate() that I changed -

173 **

174172**

   startService();

175173**

 }

176174**

 }

which seemed to be restarting the service if it was already running - Anyway, one of the side effects of my changes seems to be that now the service gets stopped cleanly when you click the stop service button, its onDestroy() method now gets called and removes the service from the foreground and removes the notification - which is correct, but is that the idea of the stop service button? Or do you think it should just pause processing of the ServiceThread, but keep the service running so as to keep the notification and keep the service in the foreground, if that makes any sense :-)

Also, a related side-effect - as the service stops cleanly now if it throws an exception, it removes the notification and the app from the foreground

Hopefully I should get time to have another play over the weekend.

As I said, I'm very new to Android development, so let me know if you spot any problems with these changes

Cheers,

Chris

On 29 August 2012 08:10, Richard Munn notifications@github.com wrote:

Awesome, I'll take a look and merge it in when I get a free moment.

I tend to check the forums daily, but annoyingly there's no kind of notification when you receive a private message, so I often miss those!

I noticed a similar problem with constant reconnect attempts when you disable bluetooth, and have the "connect at startup" option enabled - it constantly spams you with "bluetooth disabled" toasts - by the sounds of things your changes should help there too.

— Reply to this email directly or view it on GitHubhttps://github.com/benjymous/MWM-for-Android/pull/60#issuecomment-8117067.

benjymous commented 11 years ago

The only problem I found in testing is that the wake lock releases fail with an "already releqsed" exception sometimes - I removed the explicit timeout, and the that seems to have sorted it.

Otherwise it all looks good - I'll run it for a few days to be certain that there aren't any other problems hiding!

benjymous commented 11 years ago

(I hadn't done any Android dev before this either, so I'm sure there are lots of things that aren't being done the proper android way!)

MartinDetech commented 11 years ago

Have you ever incounter the ioexception ERROR : wrp_wsock_create: wrp_sock_create : out of wsock blocks because you call the bluetoothSocket = bluetoothDevice.createRfcommSocketToServiceRecord(uuid) to many times, I cant see that your are closing the socket before calling it agian on line 603-608 when it tries to connect? I migth be blind

firetech commented 11 years ago

Regarding the Bluetooth disabled toasts, adding a state for that may very well be a better solution. I threw those toasts in there a while ago as a quick and dirty solution after fooling myself with Bluetooth off for 10 minutes (and wondering why the *&#@ the watch wouldn't connect). I figured a toast was better than nothing... ;)

rndusr0 commented 11 years ago

Thanks - I added a bluetoothSocket.close() if the connection fails.

On 30 August 2012 11:00, Martin Trædholm notifications@github.com wrote:

Have you ever incounter the ioexception ERROR : wrp_wsock_create: wrp_sock_create : out of wsock blocks because you call the bluetoothSocket = bluetoothDevice.createRfcommSocketToServiceRecord(uuid) to many times, I cant see that your are closing the socket before calling it agian on line 603-608 when it tries to connect? I migth be blind

— Reply to this email directly or view it on GitHubhttps://github.com/benjymous/MWM-for-Android/pull/60#issuecomment-8154603.

rndusr0 commented 11 years ago

Thanks for that - I've updated and will be testing too over the next few days. I'll try to get time to look at making bluetooth not available, and bluetooth switched off into connection statuses over the weekend too if you like...

On 30 August 2012 08:37, Richard Munn notifications@github.com wrote:

The only problem I found in testing is that the wake lock releases fail with an "already releqsed" exception sometimes - I removed the explicit timeout, and the that seems to have sorted it.

Otherwise it all looks good - I'll run it for a few days to be certain that there aren't any other problems hiding!

— Reply to this email directly or view it on GitHubhttps://github.com/benjymous/MWM-for-Android/pull/60#issuecomment-8151590.

benjymous commented 11 years ago

Reopen due to related issues #61 and #62