MythTV-Clients / MythtvPlayerForAndroid

MythTV Player is an Android client for accessing content from a MythTV Backend
https://play.google.com/store/apps/details?id=org.mythtv.android
GNU General Public License v3.0
16 stars 7 forks source link

Implement backend discovery via SSDP #178

Open dekarl opened 8 years ago

dekarl commented 8 years ago

It would be very nice to automagically configure the backend connection in local network environments iff the backend URL contains the default value and only one backend is found via SSDP. E.g. search for deviceType urn:schemas-mythtv-org:device:MasterMediaServer:1, then use the URL base for the backend connection (see http://www.upnp-database.info/device.jsp?deviceId=812 for an example)

Android Network Discovery Documentation https://developer.android.com/training/connect-devices-wirelessly/nsd.html

dekarl commented 7 years ago

231 Re-discovering the backend in case of connection problems (e.g. reinstalled with a different IP) would be nice, too.

dmfrey commented 7 years ago

I envision discovery happening every time the application loads and either updates the shared preferences or leaves it as is if it is unchanged, then you proceed to the main interface. The big question would be those users who have multiple master backends on the network. I know in most cases, there will be just 1. But there are uses like @billmeek who runs a main system based on the latest release and also a system based on the git head. He can clarify his setup further.

On Mon, Jan 23, 2017 at 3:11 PM Karl Dietz notifications@github.com wrote:

231 https://github.com/MythTV-Clients/MythtvPlayerForAndroid/issues/231

Re-discovering the backend in case of connection problems (e.g. reinstalled with a different IP) would be nice, too.

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/MythTV-Clients/MythtvPlayerForAndroid/issues/178#issuecomment-274602761, or mute the thread https://github.com/notifications/unsubscribe-auth/AA5yzP8oZ6aBxMBvAdtaQsoRmsoGaynqks5rVQlngaJpZM4IxNt1 .

billmeek commented 7 years ago

I've even had an old box running 25, 26, 27 and 28 (doesn't record much, and not of use now with the recordedid only available in 0.28+).

Recall that there were one, or a few, users that didn't want SSDP running on their backends. Not clear why, but they didn't. This was back in the days of Android Frontend.

@dekarl take a peek at #231. Not the automatic solution, but one for a more casual user that needs help diagnosing connection failures.

dmfrey commented 7 years ago

@dekarl its a start ^^^

dmfrey commented 7 years ago

@billmeek @dekarl This is ready for testing. Please report any bugs here or as new issues if they warrant one

billmeek commented 7 years ago

@dmfrey you already predicted the 1st issue. I deleted the data for this app (because if I delete the backend IP/hostname in settings, the existing one is just restored.)

Then when I stopped/started the app, it worked, but selected the backend running on my office computer, not my production machine.

Looking in LogCat with a pattern of nsd, I came up with this:

01-26 10:53:57.880 22046-22046/org.mythtv.android D/NsdActivity: onCreate : enter
01-26 10:53:57.954 22046-22046/org.mythtv.android D/NsdFragment: onAttach : enter
01-26 10:53:57.954 22046-22046/org.mythtv.android D/NsdFragment: onAttach : exit
01-26 10:53:58.073 22046-22046/org.mythtv.android D/NsdActivity: onCreate : exit
01-26 10:53:58.073 22046-22046/org.mythtv.android D/NsdFragment: onActivityCreated : enter
01-26 10:53:58.073 22046-22046/org.mythtv.android D/NsdFragment: lookupBackend : enter
01-26 10:53:58.326 22046-22046/org.mythtv.android D/NsdFragment: lookupBackend : exit
01-26 10:53:58.326 22046-22046/org.mythtv.android D/NsdFragment: onActivityCreated : exit
01-26 10:53:58.644 22046-22046/org.mythtv.android I/NsdFragment: lookupBackend : 192.168.1.204:6544
01-26 10:53:58.646 22046-22046/org.mythtv.android D/NsdActivity: onDiscoveryComplete : enter
01-26 10:53:58.653 22046-22046/org.mythtv.android D/NsdActivity: onDiscoveryComplete : exit
01-26 10:53:58.653 22046-22046/org.mythtv.android I/NsdFragment: lookupBackend : 192.168.1.200:6544
01-26 10:53:58.653 22046-22046/org.mythtv.android D/NsdActivity: onDiscoveryComplete : enter
01-26 10:53:58.657 22046-22046/org.mythtv.android D/NsdActivity: onDiscoveryComplete : exit

and note that my production BE's IP is there (192.168.1.204) but my office IP .200 is the 2nd one in LogCat and the one selected by the app.

dmfrey commented 7 years ago

@billmeek It sounds like whenever each sends out a broadcast, we get an event per backend. I am open to options on how to handle this.

billmeek commented 7 years ago

Another edge case. Stop the app, delete data, turn Wi-Fi off.

Start the app and the screenshot below appears. Pressing the back arrow at the top does nothing. The app can be exited with the back icon.

If Wi-Fi is turned on, while on the screen below, the app takes off as expected. Perhaps this will be solved by #231.

In this case, my 192.168.1.204 showed up AFTER my .200 address in LogCat and my production BE was used and stored in Settings.

screenshot_20170126-111359

dmfrey commented 7 years ago

We should come up with a few rules around this.

  1. How often should this run?
    • Should it run at every startup
    • Should it only run from the prefernces screen?
  2. How should we display the results?
    • Should we pop a dialog fragment with a continually updating list of backends that you select from?
  3. Once set, should we lock it?
    • Subsequent ZeroConf events don't update it unless you go through the above process?

I am open to other questions/options as well

billmeek commented 7 years ago

Replying to the earlier question about " an event per backend":

@dmfrey I'd suggest a screen that presents the user with a list of BEs and lets them select one.

Or, at least a message that says multiple BEs were detected and that manual changes must be made.

billmeek commented 7 years ago

Getting tougher. I turned up my slave backend and now see the following:

01-26 12:45:57.648 6608-6608/org.mythtv.android I/NsdFragment: lookupBackend : 192.168.1.221:6544
01-26 12:45:57.657 6608-6608/org.mythtv.android I/NsdFragment: lookupBackend : 192.168.1.204:6544
01-26 12:45:57.664 6608-6608/org.mythtv.android I/NsdFragment: lookupBackend : 192.168.1.200:6544

The .221 address being the slave (and there's no indication with the ZeroConf Browser app that it's a slave.) The only difference is that the hostname (that I chose) is slave-mc1.

In 0.26:

type '_mythbackend-master._tcp.'

was returned. In 0.27 and beyond:

name 'Mythbackend on mc0' type '_mythbackend._tcp.'

is returned.

This is partially good. When I turn on my 0.2[678], 29-pre host, the 0.26 instance of the backend isn't displayed with the ZeroConf Browser app, and this app only detects 3 of the 4 instances, although I can only assume it's missing the 0.26 one.

billmeek commented 7 years ago

Now to take a stab at @dmfrey's earlier questions: We should come up with a few rules around this.

  1. How often should this run?
    • Should it run at every startup

Probably not. Not for me with an unusual number of backends, but especially for folks with slave BEs, if the master is off-line and the slave is on (odd), then the wrong host would be selected. Once the master is up, the app must be 'force stopped' to recognize the addition. Just restarting the app doesn't help as it stands now. Interestingly, GetRecordedList presents the correct list of recordings, but no images are returned and recordings don't play.

This conflicts with @dekarl's case (above) where a BE's IP has changed and it would be desirable to detect that.

For most users, having a pop-up alerting them that another BE has come on-line would probably become annoying once their Master has been set-up. Maybe a single Toast or SnackBar would be OK.

That may be a better solution. It's what the Mythtv Android Frontend did as I recall. The user had the option of doing detection.

  1. How should we display the results?
    • Should we pop a dialog fragment with a continually updating list of backends that you select from?

I'd prefer that. If so, then the IP as well as the returned name should be displayed. Hopefully, users would choose names for slave BEs that were meaningful (and recognize them if presented in the list.)

  1. Once set, should we lock it?
    • Subsequent ZeroConf events don't update it unless you go through the above process?

Again, conflicts with the "only the BE's IP changed" case. But yes, I believe it should be locked.

billmeek commented 7 years ago

Note that the backend's build information will tell you if mythbackend was built with discovery turned on:

$ api --host mc0 --endpoint Myth/GetBackendInfo
{
    "BackendInfo": {
        "Build": {
            "LibDNS_SD": "true", <------------------------
            "LibX264": "true",
            "Version": "v29-pre-194-g2a1ccc0"
        },
billmeek commented 7 years ago

A little Wireshark. Note the presence of the TXT below and indication of master with a 6 byte length. I think the 5 byte slave responses should be ignored.

Internet Protocol Version 4, Src: mc4.local (192.168.1.224), Dst: 224.0.0.251 (224.0.0.251)
User Datagram Protocol, Src Port: mdns (5353), Dst Port: mdns (5353)
Multicast Domain Name System (response)
    Transaction ID: 0x0000
    Flags: 0x8400 Standard query response, No error
    Questions: 0
    Answer RRs: 11
    Authority RRs: 0
    Additional RRs: 0
    Answers
        Mythbackend on mc4-29._mythbackend._tcp.local: type TXT, class IN, cache flush
            Name: Mythbackend on mc4-29._mythbackend._tcp.local
            Type: TXT (Text strings) (16)
            .000 0000 0000 0001 = Class: IN (0x0001)
            1... .... .... .... = Cache flush: True
            Time to live: 4500
            Data length: 7
            TXT Length: 6
            TXT: master
        Mythbackend on mc4-29._mythbackend._tcp.local: type SRV, class IN, cache flush, priority 0, weight 0, port 2944, target mc4.local
billmeek commented 7 years ago

I'm not pushing the following because it only works if the backend is modified to return TXT in the format of master=true rather than the existing master.

diff --git a/app/src/main/java/org/mythtv/android/presentation/view/fragment/NsdFragment.java b/app/src/main/java/org/mythtv/android/presentation/view/fragment/NsdFragment.java
index 82d7b20..0f5b116 100644
--- a/app/src/main/java/org/mythtv/android/presentation/view/fragment/NsdFragment.java
+++ b/app/src/main/java/org/mythtv/android/presentation/view/fragment/NsdFragment.java
@@ -72,15 +72,17 @@ public class NsdFragment extends AbstractBaseFragment {
     private void lookupBackend() {
         Log.d( TAG, "lookupBackend : enter" );

-        Subscription s = ZeRXconf.startDiscovery( getActivity(), "_mythbackend._tcp." )
+        Subscription s = ZeRXconf.startDiscovery( getActivity(), "_mythbackend._tcp.", /*needsTxtRecord*/ true )
                             .subscribe(
                                     nsdInfo -> {
                                         Log.i( TAG, "lookupBackend : " + nsdInfo.getAddress().getHostAddress() + ":" + nsdInfo.getServicePort() );
-                                        SharedPreferences.Editor editor = sharedPreferences.edit();
-                                        editor.putString( SettingsKeys.KEY_PREF_BACKEND_URL, nsdInfo.getAddress().getHostAddress() );
-                                        editor.putString( SettingsKeys.KEY_PREF_BACKEND_PORT, String.valueOf( nsdInfo.getServicePort() ) );
-                                        editor.apply();
+                                        if( nsdInfo.getAttributes().containsKey( "master" ) ) {
+                                            SharedPreferences.Editor editor = sharedPreferences.edit();
+                                            editor.putString(SettingsKeys.KEY_PREF_BACKEND_URL, nsdInfo.getAddress().getHostAddress());
+                                            editor.putString(SettingsKeys.KEY_PREF_BACKEND_PORT, String.valueOf(nsdInfo.getServicePort()));
+                                            editor.apply();

+                                        }
                                         listener.onDiscoveryComplete();
                                     },
                                     e -> {

But one point is clear, ZeRXconf.startDiscovery needs the true argument to get the TXT. It seems like a ZeRXconf issue that it doesn't honor TXT keys that don't have an =value portion. The specs I've seen say that in the case of a missing value, the key should be treated as a bool, therefore true in our case.

Ref. Section 6.4 here: http://files.dns-sd.org/draft-cheshire-dnsext-dns-sd.txt Also: http://www.zeroconf.org/Rendezvous/txtrecords.html in the: "Rules for Names in DNS-SD Name/Value pairs" section. For the record: https://github.com/Ennova-IT/ZeRXconf.