Project

General

Profile

Bug #1411

Feature #1461: Preparation for beta release

Auto detection of local/remote

Added by megabitdragon about 7 years ago. Updated about 7 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
-
Target version:
-
Start date:
07/12/2014
Due date:
% Done:

100%


Description

We need to detect when network is changing to switch from proxy to local and viceversa.

network.log View (8.01 KB) ming, 06/10/2014 04:09 PM

log.txt View (5.68 KB) megabitdragon, 06/11/2014 07:21 PM

log.txt View (1.64 KB) megabitdragon, 07/09/2014 06:10 PM

log2.txt View (1.72 KB) megabitdragon, 07/09/2014 06:11 PM


Subtasks

Bug #1471: App crashes when network changesClosedmegabitdragon

History

#1 Updated by megabitdragon about 7 years ago

  • Priority changed from Normal to Medium

#2 Updated by megabitdragon about 7 years ago

  • Priority changed from Medium to High

#3 Updated by ming about 7 years ago

Need a clarification here. I assume that reconnection should occur only for files lists, mostly for new requests. Is it correct?

#4 Updated by ming about 7 years ago

  • Status changed from New to Feedback
  • Assignee changed from ming to megabitdragon

#5 Updated by megabitdragon about 7 years ago

  • Status changed from Feedback to Assigned
  • Assignee changed from megabitdragon to ming

The way I see it, when the network changes from local to remote, the next request should be made to the proxy and the other way around (remote->local, next request on the local ip). Also the case when network is changing while you watch a movie or listen to audio must be addressed as well.

#6 Updated by ming about 7 years ago

I didn’t mentioned audio and video because while changing network it will stop anyway. This part is tricky.

#7 Updated by cpg about 7 years ago

A little late to this discussion, ... I should have chimed in earlier: streaming seamlessly while a movie is playing (i.e. changing from local to remote or back dynamically) is not a design requirement (or a goal for the app) at the moment.

#8 Updated by ming about 7 years ago

Having a trouble here. I’m following this tip. At the attached log you can see intents that are received after registering BroadcastReceiver. Basically system fires too many events and I see no pattern to follow. More events are usually better, but when I cannot filter it to just two events (change network one way and another) it will be too many useless reconnections. Will eat battery very fast, yum-yum (just kidding). Do you see any pattern?

#9 Updated by megabitdragon about 7 years ago

I don't see anything in the log but I found this on stackoverflow. Can we use something like this?

Never mind I didn't read thoroughly. Since we don't want to address the movie playing for now, maybe we can check for connectivity changes only before performing a new request to the hda/proxy? I really don't know what the impact will be on performance.

#10 Updated by ming about 7 years ago

My solution was similar, I tried some checking mentioned at the thread as well—no luck, still too many events, 3 for reconnection actually. About checking before every request—well, count it as twice everything from time to battery. Carlos, how did you deal with this on iOS? Maybe there is some policy or trick for that?

#11 Updated by cpg about 7 years ago

in iOS we went in steps. we had manual mode first. no dynamic autodetection. this was mostly to release soon to testers and also to not complicate the state and algorithm to detect changes and autodetect the local server.

eventually we did something like this.

there is a call with notification of changes into our code. this gets processed and autodetection is performed.

we implemented a minimum period allowed (in seconds) for local reachability tests to occur: at most every 2 seconds

there are the following states:

A) one of three possible modes of operation for the app (Auto[the default], LAN or Remote) and
B) one of two two possible API endpoints

these are two separate pieces of states, A) is the UI, B) is the state of the current API endpoint base URL

it has been quite stable. the code is all in about 200 lines of code.

#12 Updated by ming about 7 years ago

All right, here are details. Carlos gave me an idea of caching a current network type and checking it before sending an internal event of connection change. BroadcastReceiver can be registered on the application level using the AndroidManifest.xml. But! Every connection event will recreate the BroadcastReceiver object, so caching at its field is not available. On other hand it is possible to register the BroadcastReceiver at Activity. It kind of works, but I don’t like the implementation because suddenly UI should manipulate the network which doesn’t sound right. Can we move on to something next? I’ll push my changes to a branch so you can try it.

#13 Updated by ming about 7 years ago

It was pushed.

#14 Updated by megabitdragon about 7 years ago

I tried this branch and the app crashed. Here is what I did:
1. Start app in remote mode and wait for shares to load
2. turn wifi on
3. app crashes (log attached)

#15 Updated by megabitdragon about 7 years ago

Maybe let's put a switch somewhere in the drawer to toggle local and remote and we will brainstorm a little more about how to do this.

#16 Updated by ming about 7 years ago

I actually kind of fixed it (again!), will push it. But your log shows connection timeout what I wasn’t able to catch.

There is an another headache—Retrofit doesn’t allow to set timeouts easy way.

Can we delay it? I want to think about it better without hot fixing and all thesis stuff kind of clouds my view.

#17 Updated by megabitdragon about 7 years ago

As I state let's put the auto detection on the side for now and use a manual switch if it's not too much of a hassle to implement. Once you are done with thesis and stuff we can revisit the auto detection.

#18 Updated by megabitdragon about 7 years ago

  • Priority changed from High to Low

#19 Updated by megabitdragon about 7 years ago

  • Subject changed from Local remote is only detected at startup to Auto detection of local/remote
  • Priority changed from Low to Normal
  • Parent task set to #1461

We need this to work before the beta release. I know you had a few ways of doing it and I have another one just to make things more confusing :).
What is we use this and we check if we are remote/local before any request to the endpoint only when this flag has changed.

#20 Updated by ming about 7 years ago

I tried exactly the same method, it even was pushed as a branch.

#21 Updated by megabitdragon about 7 years ago

I just read back through the tracker. I didn't remember that. Anyway back to square 1 or 1.1 since we found a few solutions that don't really work.

#22 Updated by megabitdragon about 7 years ago

  • Priority changed from Normal to Medium

#23 Updated by ming about 7 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from ming to megabitdragon

Kind of ready but please do not release it to users, it is not even close for them. You may notice a great delay between switching from WiFi to Mobile and UI updating—that’s because Retrofit doesn’t allow changing timeout directly, so some magic should be applied.

#24 Updated by ming about 7 years ago

During the implementation I noticed that settings + the connection indication at the action bar add some amount of boilerplate code. So I’m gonna raise this one last time. I still think that settings are useless (reasons pointed out before) as well as connection indication. From the code perspective each Activity will have boilerplate code for switching action bar background—no exception. Even if I move it into another class for composition—there should be listeners for network switch and update calls set carefully during the lifecycle. This doesn’t matter because nobody see the code. From the UI perspective I as a user fully understand that if I am on the network—I should care about traffic (and I really do that, for all apps and even system), when I am on WiFi—I don’t care about traffic. This is very simple rule. I don’t think that Google Music or Spotify change UI when there is no WiFi—and it is much easier to forget about it using these apps. Another thing is about colors—I still don’t understand how color can be associated with connection. Anyway, this is the last time I raised it. No explanations required—you have your point of view, I have mine. Decision is yours and I’m gonna keep it implemented or not depending on it only.

#25 Updated by megabitdragon about 7 years ago

When I start the app, no shares are retrieved. If I switch to a different server the app crashes (log.txt). If I get into the setting and switch to LAN the app crashes again (log2.txt). I couldn't do much testing. All these are on my Nexus 4 running CM11.

#26 Updated by megabitdragon about 7 years ago

cpg and I discussed the issues you raised about the connection indication and settings menu. Here is the summary.
1. Based on your arguments, you can remove the indicator for LAN/remote. The action bar should stay blue all the time. A question for my personal education; why do you use a PNG for the blue instead of just using the hex code for the color?
2. The selection for Auto/LAN/Remote will remain in the setting menu until auto detection works flawlessly.
3. The rest of the options will remain in settings

#27 Updated by megabitdragon about 7 years ago

  • Priority changed from Medium to High

#28 Updated by ming about 7 years ago

Regarding crashes—are you using mobile network or WiFi? It is probably related to lifecycle, thank you for logs.

Regarding UI—when using a color there will be no slight border on the bottom of the action bar. It is not really visible on Nexus 4 and blue action bars, but you can look at the Google Play app for better example. It gives some depth, don’t know why it is not applied automatically based on color, probably because there is no support for drawable tinting until L.

#29 Updated by megabitdragon about 7 years ago

I was on Wi-Fi at that time.

#30 Updated by megabitdragon about 7 years ago

Regarding the UI let's just use plain color (Amahi blue) instead of this "hack".

#31 Updated by ming about 7 years ago

It is not a hack but a consistent behavior. In that case drawer shadow and its icon would be hacks as well ;-)

#32 Updated by megabitdragon about 7 years ago

Let's just forget about it and move on. We have more important issues on the table and honestly I am tired of talking details while losing touch with the big picture.

#33 Updated by ming about 7 years ago

Sorry. I am from phone right now, can you change Server.java to return proper session and try again? I forgot to restore it after testing at the local portable server. Will be home in an hour or so.

#34 Updated by megabitdragon about 7 years ago

I am at work and I'll not get home for a while. I guess you will beat me to it. :)

#35 Updated by ming about 7 years ago

Pushed.

#36 Updated by megabitdragon about 7 years ago

I'll give it a try tonight. Let's move to the next item on the list.

#37 Updated by megabitdragon about 7 years ago

  • Status changed from Feedback to Assigned
  • Assignee changed from megabitdragon to ming

I managed to try the auto detection for a little bit. The good news is that the app didn't crash :).
I can confirm that switching from cellular to wifi is much faster than the other way around.
When on remote I frequently get connection error screen. I looked at the log and it seems like it is still trying to connect on the local IP (log1.txt). Maybe is related to the slow switching.
Also at startup/resume it always defaults to lan. It should check what the connection actually is.
I also got some very strange UI artifacts. I put the picture in the Dropbox folder. However I can't reproduce them anymore and I am not sure if they are related with the auto detection.

#38 Updated by megabitdragon about 7 years ago

Can we use something like this to set a short timeout for the LAN connection?

#39 Updated by ming about 7 years ago

It is trying to connect to the local address for auto-detection purposes. The slow switching is related to timeout for auto-detection connection. You showed a link to a possible solution, but I’ll try a couple of others which are better for the current architecture.

Didn’t get your point about defaults to LAN.

Artifacts are related to #1413.

#40 Updated by ming about 7 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from ming to megabitdragon

Changed timeout, should be better now. If detection itself is fine I would prefer to merge and fix related UI issues later, because drawer-related changes can just cross current ones.

#41 Updated by megabitdragon about 7 years ago

Works pretty good for me except the error I pointed in the other bug. You can go ahead, merge and fix the bug or fix and merge. Your call.

#42 Updated by megabitdragon about 7 years ago

  • Status changed from Feedback to Assigned
  • Assignee changed from megabitdragon to ming

#43 Updated by ming about 7 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from ming to megabitdragon

Merged.

#44 Updated by cpg about 7 years ago

ok, i set it to Auto and i start using the app. how do i know whether it's using the local server or the proxy?

#45 Updated by megabitdragon about 7 years ago

for debug purposes look at the log. From the user perspective why do you care? As a user you know if you are on wifi or cellular data from the status bar. If you are on wifi (home ore away) you shouldn't care about data caps, you just want your app to work and stream.

#46 Updated by cpg about 7 years ago

what is the exact information to watch for in the log to know what is the current status at any given time?

more importantly, when i am with the phone around in coffee shops or other places testing, i am not connected to a pc to see the log.

#47 Updated by megabitdragon about 7 years ago

If I remember correctly, in the log you just limit the search to "D/CONNECTION" string. I will double check when I get home.

In the coffee shop, I would say it is irrelevant. If you can access your hda you are on remote, period. The only exception is if you are connected with VPN. I didn't tested this case and I am not sure if it was considered during development. I'll test it later and report back.

What exactly are you trying to test? How the switching is done, if it is done or you just want the network indicator back? :)

#48 Updated by megabitdragon about 7 years ago

  • Status changed from Feedback to Closed

I will close this since it is implemented and if there are problems we will file new bugs

Also available in: Atom