FreeOpcUa / freeopcua

Open Source C++ OPC-UA Server and Client Library
http://freeopcua.github.io/
GNU Lesser General Public License v3.0
712 stars 341 forks source link

Listing node children is truncated #153

Open tomkcook opened 9 years ago

tomkcook commented 9 years ago

When I repeatedly request the list of children of a node, after the first time, the list of children is truncated. The following code demonstrates the problem:

#include <opc/ua/client/client.h>
#include <opc/ua/node.h>
#include <opc/ua/subscription.h>

#include <iostream>
#include <stdexcept>

int main(int argc, char** argv)
{
  try
  {
    std::string endpoint = "opc.tcp://10.59.121.56:4840/";
    std::cout << "Connecting to: " << endpoint << std::endl;
    OpcUa::UaClient client(false);
    client.Connect(endpoint);

    OpcUa::Node objects = client.GetObjectsNode();

    OpcUa::Node wtc = objects.GetChild(std::vector < std::string > { "4:PLC", "6:Modules", "6:::", "6:WTC" });
    for(int ii = 0; ii < 5; ++ii) {
      auto children = wtc.GetChildren();
      std::cout << "Node has " << children.size() << " children" << std::endl;
    }

    std::cout << "Disconnecting" << std::endl;
    client.Disconnect();
    getchar();
    return 0;
  }
  catch (const std::exception& exc)
  {
    std::cout << exc.what() << std::endl;
  }
  catch (...)
  {
    std::cout << "Unknown error." << std::endl;
  }
  return -1;
}

For me, with a B&R X20CP1585 as the OPC UA server, this outputs:

Connecting to: opc.tcp://10.59.121.56:4840/
Connection was closed by host. No error.
Node has 430 children
Node has 100 children
Node has 100 children
Node has 100 children
Node has 100 children
Disconnecting
Connection was closed by host. No error.
oroulet commented 9 years ago

Is it always å high number then å lower one? Server might be truncating data. Should be code to resume browsing but since I never encountered such a server there might be bug....

On Wed, Jun 17, 2015, 18:25 tomkcook notifications@github.com wrote:

When I repeatedly request the list of children of a node, after the first time, the list of children is truncated. The following code demonstrates the problem:

include <opc/ua/client/client.h>

include <opc/ua/node.h>

include <opc/ua/subscription.h>

include

include

int main(int argc, char\ argv) { try { std::string endpoint = "opc.tcp://10.59.121.56:4840/"; std::cout << "Connecting to: " << endpoint << std::endl; OpcUa::UaClient client(false); client.Connect(endpoint);

OpcUa::Node objects = client.GetObjectsNode();

OpcUa::Node wtc = objects.GetChild(std::vector < std::string > { "4:PLC", "6:Modules", "6:::", "6:WTC" });
for(int ii = 0; ii < 5; ++ii) {
  auto children = wtc.GetChildren();
  std::cout << "Node has " << children.size() << " children" << std::endl;
}

std::cout << "Disconnecting" << std::endl;
client.Disconnect();
getchar();
return 0;

} catch (const std::exception& exc) { std::cout << exc.what() << std::endl; } catch (...) { std::cout << "Unknown error." << std::endl; } return -1; }

For me, with a B&R X20CP1585 as the OPC UA server, this outputs:

Connecting to: opc.tcp://10.59.121.56:4840/ Connection was closed by host. No error. Node has 430 children Node has 100 children Node has 100 children Node has 100 children Node has 100 children Disconnecting Connection was closed by host. No error.

— Reply to this email directly or view it on GitHub https://github.com/FreeOpcUa/freeopcua/issues/153.

oroulet commented 9 years ago

look there https://github.com/FreeOpcUa/freeopcua/blob/master/src/client/binary_client.cpp at line 572 there is the handling of continuation point. you can try to add print statement or use gdb to find out what happens.

On Wed, 17 Jun 2015 at 19:15 Olivier Roulet-Dubonnet < olivier.roulet@gmail.com> wrote:

Is it always å high number then å lower one? Server might be truncating data. Should be code to resume browsing but since I never encountered such a server there might be bug....

On Wed, Jun 17, 2015, 18:25 tomkcook notifications@github.com wrote:

When I repeatedly request the list of children of a node, after the first time, the list of children is truncated. The following code demonstrates the problem:

include <opc/ua/client/client.h>

include <opc/ua/node.h>

include <opc/ua/subscription.h>

include

include

int main(int argc, char\ argv) { try { std::string endpoint = "opc.tcp://10.59.121.56:4840/"; std::cout << "Connecting to: " << endpoint << std::endl; OpcUa::UaClient client(false); client.Connect(endpoint);

OpcUa::Node objects = client.GetObjectsNode();

OpcUa::Node wtc = objects.GetChild(std::vector < std::string > { "4:PLC", "6:Modules", "6:::", "6:WTC" });
for(int ii = 0; ii < 5; ++ii) {
  auto children = wtc.GetChildren();
  std::cout << "Node has " << children.size() << " children" << std::endl;
}

std::cout << "Disconnecting" << std::endl;
client.Disconnect();
getchar();
return 0;

} catch (const std::exception& exc) { std::cout << exc.what() << std::endl; } catch (...) { std::cout << "Unknown error." << std::endl; } return -1; }

For me, with a B&R X20CP1585 as the OPC UA server, this outputs:

Connecting to: opc.tcp://10.59.121.56:4840/ Connection was closed by host. No error. Node has 430 children Node has 100 children Node has 100 children Node has 100 children Node has 100 children Disconnecting Connection was closed by host. No error.

— Reply to this email directly or view it on GitHub https://github.com/FreeOpcUa/freeopcua/issues/153.

tomkcook commented 9 years ago

Yes, I always get the full list the first time, then a truncated list after that. It's not always a full list then 100 each time after that - I have seen 430, then 300, then 100.

Looking at the debug output, it seems the server always sends children back in blocks of 100, with BrowseNext being called repeatedly to get the full list. After the first time, this doesn't return all the children, but I'm not sure yet whether it's the server limiting its output or a bug in the client library. I'll keep digging.

oroulet commented 9 years ago

Then there is bug in the continuation stuff. Look at my last epost. It should not be hard to fix when you have a server using it

On Thu, 18 Jun 2015 at 10:01 tomkcook notifications@github.com wrote:

Yes, I always get the full list the first time, then a truncated list after that. It's not always a full list then 100 each time after that - I have seen 430, then 300, then 100.

Looking at the debug output, it seems the server always sends children back in blocks of 100, with BrowseNext being called repeatedly to get the full list. After the first time, this doesn't return all the children, but I'm not sure yet whether it's the server limiting its output or a bug in the client library. I'll keep digging.

— Reply to this email directly or view it on GitHub https://github.com/FreeOpcUa/freeopcua/issues/153#issuecomment-113068216 .

tomkcook commented 9 years ago

Okay, I've found the problem. And I've also go a solution, though I'm not sure what else it might break.

About line 539 of binary_client.cpp:

  for ( BrowseResult result : response.Results )
  {
    if (! result.ContinuationPoint.empty())
    {
      ContinuationPoints.push_back(result.ContinuationPoint);
    }
  }

The problem is that ContinuationPoints is never cleared. So the first time the list is requested, everything works, but the second time there is still the continuation point from the first request in this list and the server complains about an invalid continuation point.

The fix is to change the above code to this:

  for ( BrowseResult result : response.Results )
  {
    if (! result.ContinuationPoint.empty())
    {
      ContinuationPoints.clear();
      ContinuationPoints.push_back(result.ContinuationPoint);
    }
  }

This fixes the current problem and, as far as I can tell, doesn't break anything else - but I'm not sure.

I note that the BinaryClient class has a method called Release which calls ContinuationPoints.clear(), but it seems this is never called. The comment alongside it says it should be removed and replaced with a release option to BrowseNext. But I'm not sure how that would work - wouldn't that require that you know in advance whether there was any more data? Why not just use my fix above and remove the Release method?

If you're happy with the above, then I'll commit it and create a pull request.

oroulet commented 9 years ago

your solution is probably still truncating result. . the cycle is : call browse call browseNext as long as ContinationPoints is not empty but ContinuationsPoints is never updated in BrowseNext and not cleaned in browse So I think we need to add early in browse call we need to reset continuationPoints in browsenext we need to empty continuationPoints and update it with call result We do not need release method (as far as I can see)

On Thu, 18 Jun 2015 at 11:35 tomkcook notifications@github.com wrote:

Okay, I've found the problem. And I've also go a solution, though I'm not sure what else it might break.

About line 539 of binary_client.cpp:

for ( BrowseResult result : response.Results ) { if (! result.ContinuationPoint.empty()) { ContinuationPoints.push_back(result.ContinuationPoint); } }

The problem is that ContinuationPoints is never cleared. So the first time the list is requested, everything works, but the second time there is still the continuation point from the first request in this list and the server complains about an invalid continuation point.

The fix is to change the above code to this:

for ( BrowseResult result : response.Results ) { if (! result.ContinuationPoint.empty()) { ContinuationPoints.clear(); ContinuationPoints.push_back(result.ContinuationPoint); } }

This fixes the current problem and, as far as I can tell, doesn't break anything else - but I'm not sure.

I note that the BinaryClient class has a method called Release which calls ContinuationPoints.clear(), but it seems this is never called. The comment alongside it says it should be removed and replaced with a release option to BrowseNext. But I'm not sure how that would work - wouldn't that require that you know in advance whether there was any more data? Why not just use my fix above and remove the Release method?

If you're happy with the above, then I'll commit it and create a pull request.

— Reply to this email directly or view it on GitHub https://github.com/FreeOpcUa/freeopcua/issues/153#issuecomment-113090496 .

tomkcook commented 9 years ago

The change I made above works for me, against the OPC UA server that I have - I guess it must send back the same continuation point for each continuation of a single request.

I've implemented the fix you suggested, but I'm having a bit of trouble merging it with the current master. I'll get it done on Monday.

tomkcook commented 9 years ago

See pull request #154.