Handle network change

[simonc]

(02-07-2015 01:40 PM)SiMet Wrote:  When do you plan introduce this feature?

Thanks for the reminder. I'll try to get this added in the next couple of weeks. I'll post to this thread when the changes are available.

[simonc]

(02-07-2015 04:22 PM)simonc Wrote:  

(02-07-2015 01:40 PM)SiMet Wrote:  When do you plan introduce this feature?

Thanks for the reminder. I'll try to get this added in the next couple of weeks. I'll post to this thread when the changes are available.

The changes are now committed. See https://github.com/openhome/ohNet/commit...5fa7a1c719 for API names for your chosen language.

[SiMet]

Thank you for this feature.
I've checked it and when I connect to other wifi source I get SubnetListChanged callback and also I see on my wireshark on computer that device sents NOTIFYs with alive.
However, I found that after switch to other wifi Location header in notify message has port set to 0.

ControlPoint stack also doesn't discover devices which connected to new network.

I'm starting UpnpLibrary using startCombined(...)

Do I have to do something extra?

Additionally when I back to network which I was connected earlier it works properly

[simonc]

(05-08-2015 12:07 PM)SiMet Wrote:  Thank you for this feature.
I've checked it and when I connect to other wifi source I get SubnetListChanged callback and also I see on my wireshark on computer that device sents NOTIFYs with alive.
However, I found that after switch to other wifi Location header in notify message has port set to 0.

I'm not sure what's causing this.

Could you add the logging below please and let me know what output it generates when you switch to a subnet that doesn't work (reports Location with port 0) please?

Code:

diff --git a/OpenHome/Net/Device/DviServer.cpp b/OpenHome/Net/Device/DviServer.cpp
index 9174fa9..9fdc0c2 100644
--- a/OpenHome/Net/Device/DviServer.cpp
+++ b/OpenHome/Net/Device/DviServer.cpp
@@ -70,6 +70,9 @@ void DviServer::AddServer(NetworkAdapter& aNif)
{
     SocketTcpServer* tcpServer = CreateServer(aNif);
     DviServer::Server* server = new DviServer::Server(tcpServer, aNif);
+    char* nifName = aNif.FullName();
+    Log::Print("Started server on %s, port=%u\n", nifName, server->Port());
+    free(nifName);
     iServers.push_back(server);
}

[SiMet]

I've added code you provided.
I started my app on 192.168.43.57, then switched to 192.168.1.37.

When I switch to network I didn't start your AddServer method is not called.
When I back to this network I see your logs.

SubnetListChanged and line which begins with -> are in my SubnetListChanged listener.

Code:

SubnetListChanged
-> en0 192.168.1.37 (en0)
SubnetListChanged
-> lo0 127.0.0.1 (lo0)
> Started server on 192.168.43.57 (en0), port=54685
SubnetListChanged
-> en0 192.168.43.57 (en0)
(...)
SubnetListChanged
-> en0 192.168.1.37 (en0)

[SiMet]

I'd like to remind about this issue

[SiMet]

Hello,
I started debug this problem.

I start whole Upnp using StartCombined.

When I get SubnetListChanged I call SetCurrentSubnet with new Subnet. Without this call my CP doesn't work.

When I added this SetCurrentSubnet call to SubnetListChange handler my ControlPoint started discover other devices after switch network.

However, Device is not working properly. Currently I see that DviServer::AddServer is called and port is correct. But ALIVE Notify messages has port 0. I've checked it and DviProtocolUpnp:AddInterface creates DviProtocolUpnpAdapterSpecificData with port value 0.

I do not know why is it working in that way.
If I could help, give you anything which could help you to reproduce this issue do not bother to ask.

Currently I'm testing it with iOS simulator. I reconnect my device from one WIFI network to another.

EDIT:
I found that in some cases (most of cases) DviProtocolUpnp::AddInterface is called before DviServer::AddServer and this cause that port in NOTIFY messages is 0 ( in DviServer::Port(TIpAddress aInterface) iServers.size() == 0 and 0 is returned )

[SiMet]

I debug and debug and probably found solution for my issue.
Could you check if that patch could be applied to official repo

Code:

diff --git a/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp b/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
index 38c6db3..75538f4 100644
--- a/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
+++ b/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
@@ -138,11 +138,15 @@ DviProtocolUpnpAdapterSpecificData* DviProtocolUpnp::AddInterface(const NetworkA
     TIpAddress addr = aAdapter.Address();
     Bws<Uri::kMaxUriBytes> uriBase;
     TUint port = iServer->Port(addr);
-    DviDevice* root = (iDevice.IsRoot()? &iDevice : iDevice.Root());
-    root->GetUriBase(uriBase, addr, port, *this);
-    DviProtocolUpnpAdapterSpecificData* adapter = new DviProtocolUpnpAdapterSpecificData(iDvStack, *this, aAdapter, uriBase, port);
-    iAdapters.push_back(adapter);
-    return adapter;
+    if(port != 0){
+        DviDevice* root = (iDevice.IsRoot()? &iDevice : iDevice.Root());
+        root->GetUriBase(uriBase, addr, port, *this);
+        DviProtocolUpnpAdapterSpecificData* adapter = new DviProtocolUpnpAdapterSpecificData(iDvStack, *this, aAdapter, uriBase, port);
+        iAdapters.push_back(adapter);
+        return adapter;
+    }else{
+        return NULL;
+    }
}

void DviProtocolUpnp::HandleInterfaceChange()
@@ -166,13 +170,13 @@ void DviProtocolUpnp::HandleInterfaceChange()
                     pendingDelete.push_back(iAdapters[i]);
                     iAdapters.erase(iAdapters.begin() + i);
                 }
-                // add listener if 'current' is a new subnet
-                if (iAdapters.size() == 0) {
-                    iDvStack.SsdpNotifierManager().Stop(iDevice.Udn());
-                    DviProtocolUpnpAdapterSpecificData* adapter = AddInterface(*current);
-                    if (iDevice.Enabled()) {
-                        adapter->SendByeByeThenAlive(*this);
-                    }
+            }
+            // add listener if 'current' is a new subnet
+            if (iAdapters.size() == 0) {
+                iDvStack.SsdpNotifierManager().Stop(iDevice.Udn());
+                DviProtocolUpnpAdapterSpecificData* adapter = AddInterface(*current);
+                if (adapter!= NULL && iDevice.Enabled()) {
+                    adapter->SendByeByeThenAlive(*this);
                 }
             }
         }

[simonc]

Thanks for looking into this. Your conclusions were very helpful!

While I'm sure your patch works, I think a smaller set of changes might be possible. Could you check whether the patch below also fixes the bug please?

Code:

diff --git a/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp b/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
index 38c6db3..0533b95 100644
--- a/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
+++ b/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
@@ -155,10 +155,10 @@ void DviProtocolUpnp::HandleInterfaceChange()
         AutoNetworkAdapterRef ref(iDvStack.Env(), "DviProtocolUpnp::HandleInterfaceChange");
         const NetworkAdapter* current = ref.Adapter();
         TUint i = 0;
-        if (current != NULL) {
+        if (adapterList.SingleSubnetModeEnabled()) {
             // remove listeners whose interface is no longer available
             while (i<iAdapters.size()) {
-                if (iAdapters[i]->Interface() == current->Address()) {
+                if (current != NULL && iAdapters[i]->Interface() == current->Address()) {
                     i++;
                 }
                 else {
@@ -167,7 +167,7 @@ void DviProtocolUpnp::HandleInterfaceChange()
                     iAdapters.erase(iAdapters.begin() + i);
                 }
                 // add listener if 'current' is a new subnet
-                if (iAdapters.size() == 0) {
+                if (iAdapters.size() == 0 && current != NULL) {
                     iDvStack.SsdpNotifierManager().Stop(iDevice.Udn());
                     DviProtocolUpnpAdapterSpecificData* adapter = AddInterface(*current);
                     if (iDevice.Enabled()) {

[SiMet]

(03-09-2015 01:33 PM)simonc Wrote:  Thanks for looking into this. Your conclusions were very helpful!

While I'm sure your patch works, I think a smaller set of changes might be possible. Could you check whether the patch below also fixes the bug please?

Code:

diff --git a/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp b/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
index 38c6db3..0533b95 100644
--- a/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
+++ b/OpenHome/Net/Device/Upnp/DviProtocolUpnp.cpp
@@ -155,10 +155,10 @@ void DviProtocolUpnp::HandleInterfaceChange()
         AutoNetworkAdapterRef ref(iDvStack.Env(), "DviProtocolUpnp::HandleInterfaceChange");
         const NetworkAdapter* current = ref.Adapter();
         TUint i = 0;
-        if (current != NULL) {
+        if (adapterList.SingleSubnetModeEnabled()) {
             // remove listeners whose interface is no longer available
             while (i<iAdapters.size()) {
-                if (iAdapters[i]->Interface() == current->Address()) {
+                if (current != NULL && iAdapters[i]->Interface() == current->Address()) {
                     i++;
                 }
                 else {
@@ -167,7 +167,7 @@ void DviProtocolUpnp::HandleInterfaceChange()
                     iAdapters.erase(iAdapters.begin() + i);
                 }
                 // add listener if 'current' is a new subnet
-                if (iAdapters.size() == 0) {
+                if (iAdapters.size() == 0 && current != NULL) {
                     iDvStack.SsdpNotifierManager().Stop(iDevice.Udn());
                     DviProtocolUpnpAdapterSpecificData* adapter = AddInterface(*current);
                     if (iDevice.Enabled()) {

I've just checked and it doesn't fix this issue. I do not investigate it a lot.
In my opinion clue is to return NULL adapter if port is 0