fix: probe the local server address without switching onto it - #1098
herrerad85 wants to merge 3 commits into
Conversation
MainActivity moved the in use address onto the local one to find out whether it answered. Off that network it cannot, so every screen request went to an address with no route, the restored play queue was built with stream URLs carrying that address, and resetView repaired the screens by clearing the activity's whole ViewModelStore. That ran on every foreground while the phone was away from the local network. probeLocalAddress pings a client pinned to the local address and moves the in use address only once an answer comes back. On a network that cannot reach the local server the probe is the only request that goes there, nothing else changes, and no screen is torn down. Back on the local network the probe answers, the address moves and the screens are built again. A probe still outstanding is what stops a second one going out, so the switch window is stamped only where the app acts on an answer. That window is stored in preferences and outlives the activity while the probe does not, so stamping it at send left an activity recreated before the answer unable to probe again. While a probe is outstanding the public ping is not sent, and a probe that fails asks the public address itself. They used to run at once, so either answer could settle a question the other was still deciding. The saved play queue is held until the outstanding pings answer, so no stream URL is built from an address that has not been tested. On a cold start away from home the saved address is the local one and every URL built from it was dead. Two paths restore that queue, and the one in MediaManager now leaves it to the media service. The wait is capped at the configured ping timeout plus a second, and a service started with no activity behind it never waits. SystemClient.ping takes its timeout from the address its own client points at instead of the address in use, since a probe runs while the in use address is still the public one. LoginFragment writes in_use_server_address when the stored one belongs to another server, so a login inherits no address from the server selected before it. Tested: 78 unit tests on one flavor and 75 on the other, and on a phone at home and on cellular in both directions.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@herrerad85 I had to create a ping function for the connection test button on tempus/app/src/main/java/com/eddyizm/tempus/ui/login/LoginServerFragment.kt Lines 385 to 463 in ec931ca Since you had to create functions in |
The connection test button on the login screen built its own request, with a hand written URL, its own salt and token, a bare OkHttpClient and JSONObject parsing. It now goes through SystemRepository.checkUserCredential, which takes the client, and App.getSubsonicClientInstance builds one from a saved server row without writing anything to the preferences. So the test announces the same api version as the rest of the app and takes the same ping timeout. A failure at the transport still says so, through the callback's new onNetworkFailure.
|
Another feat that may be of relevance is #431 |
|
Ohhh, this is helpful. Understood |
Co-authored-by: Tom Villegas <tvillega@mailbox.org>
This branch has not been deployed
2 participants
What is broken
Tempus lets you set a local network address next to the public one, and it picks between them by switching onto the local address and pinging it there. Everything the app is doing in that moment is aimed at an address that may have no route, including the saved play queue it rebuilds at launch. Start the app away from home and the whole restored queue points at your home network, so play and next do nothing while shuffle all works. When the ping fails the app switches back and reloads every screen.
The local address is now tested on a connection of its own, while the app carries on using the address it is already on. A test that gets no answer changes nothing, and the screens are reloaded only when one succeeds. The queue restore waits for that first answer, so no link is built from an address nothing has tried. Logging in also records which of a server's two addresses is in use, which it did not do before, so a login no longer inherits the address the previous server left behind.
Tested
On a phone, both directions. At home every link in the restored queue is built on the local address. Away from home the local address times out, the app moves to the public one, the queue is built there and playback starts. Also tested with a long ping timeout. Unit tests cover the address matching.