Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved WS-Discovery standard compliance #1411

Merged
merged 5 commits into from
May 17, 2016

Conversation

altaroca
Copy link
Contributor

Some small changes to WS-Discovery Probe message for #1346 . As far as I can tell the resulting messages contain all the mandatory elements.
New: dependence on perl module Data::UUID

@@ -24,6 +24,11 @@ our $typemap_1 = {
'ProbeMatches/ProbeMatch/EndpointReference' => 'WSDiscovery10::Types::EndpointReferenceType',
'ProbeMatches/ProbeMatch/EndpointReference/ReferenceProperties' => 'WSDiscovery10::Types::ReferencePropertiesType',
'ProbeMatches/ProbeMatch/EndpointReference/PortType' => 'WSDiscovery10::Types::AttributedQName'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run this I get complaints about the comma missing at the end of this line

@jburgess777
Copy link
Contributor

I'm getting errors like this when trying this PR:

$ zmonvif-probe.pl probe
Can't locate object method "new" via package "WSDiscovery10::Elements::MessageID" (perhaps you forgot to load "WSDiscovery10::Elements::MessageID"?) at /usr/share/perl5/vendor_perl/SOAP/WSDL/XSD/Typelib/ComplexType.pm line 225.

The following fixed it for me but I don't know if this is correct or not:

diff --git a/onvif/proxy/lib/WSDiscovery10/Elements/Header.pm b/onvif/proxy/lib/WSDiscovery10/Elements/Header.pm
index df2d7f3..b68a6e2 100644
--- a/onvif/proxy/lib/WSDiscovery10/Elements/Header.pm
+++ b/onvif/proxy/lib/WSDiscovery10/Elements/Header.pm
@@ -3,6 +3,10 @@ package WSDiscovery10::Elements::Header;
 use strict;
 use warnings;

+use WSDiscovery10::Elements::MessageID;
+use WSDiscovery10::Elements::Action;
+use WSDiscovery10::Elements::ReplyTo;
+use WSDiscovery10::Elements::To;

 __PACKAGE__->_set_element_form_qualified(0);


After making these changes I see the messages going out on the wire and I get lots of responses back but none of them are decoded:

$ zmonvif-probe.pl probe
$

Adding '-v' I get the same error message for each response:

Error deserializing message. No message returned from deserializer.

@altaroca
Copy link
Contributor Author

@jburgess777 I undid the changes to the typemap. I think those were responsible for not recognizing your cameras' responses.
I do not need the four use statements. For me it works without them. The SOAP::WSDL generated proxy classes usually do not use their sub-elements' classes.

@jburgess777
Copy link
Contributor

@altaroca with your latest changes I still see the error about being unable to locate the 'new' method. I'm trying this on F23. I don't enough about the Perl object rules to know what the right answer is.

With these changes I get responses from 5 of my 6 cameras. The last camera seems to get confused by the way I quoted the namespace in the line below. Perhaps it can be rewritten to define and use an xmlns instead?

      Types => 'http://www.onvif.org/ver10/network/wsdl:NetworkVideoTransmitter http://www.onvif.org/ver10/device/wsdl:Device'

@altaroca
Copy link
Contributor Author

I am not sure how the perl class resolver works either.
I tried using namespace aliases with this code:

$result = $svc_discover->ProbeOp(
    { # WSDiscovery::Types::ProbeType
      xmlattr => { 'xmlns:dn'  => 'http://www.onvif.org/ver10/network/wsdl',
                   'xmlns:tds' => 'http://www.onvif.org/ver10/device/wsdl', },
      Types => 'dn:NetworkVideoTransmitter tds:Device', # QNameListType
      Scopes =>  { value => '' },
    }, ...

SOAP::WSDL does not serialize the attributes, though. I have to investigate further.

@jburgess777
Copy link
Contributor

I was looking into how to persuade the code to add the other namespaces but wasn't able to make it happen. For what it's worth, gsoap handles the serialization of QName fields using this code:

https://sourceforge.net/p/gsoap2/code/HEAD/tree/gsoap/stdsoap2.cpp#l14568

When parsing a QName in the form "X":Y it interprets this to mean that X is a namespace which it first tries to find in the existing namespace list and, if not found, it automatically adds it. Then the element is serialized using the appropriate namspace prefix. The equivalent SOAP::WSDL::XSD::Typelib::Builtin::QName.pm looks like it treats it like a string without any special serialization. This seems to be a missing feature in the SOAP-WSDL code.

@connortechnology
Copy link
Member

Looks like this adds libdata-uuid-perl as another dependency

@altaroca
Copy link
Contributor Author

altaroca commented Apr 16, 2016

The last commit enables us to use namespace aliases in the Types element. It is not beautiful - it is a workaround. The goal should be to fix SOAP::WSDL eventually.
Note: I added the namespaces to <Probe> because <Types> is not a ComplexType which makes things more difficult.

@connortechnology
Copy link
Member

So your comments imply that this is a temporary hack. Is it? Is there a better PR coming?

@altaroca
Copy link
Contributor Author

This PR is not a hack. The commit 5d3f5e5 is kind of a hack because it replaces SOAP::WSDL functionality. I added it to see if we get all of @jburgess777's and @knnniggett's cameras working. If we do then I propose to stick with this commit. The alternative would be updating or forking SOAP::WSDL -- which I do not have the time for.
On the other hand if the commit does not improve things then we can scrap it.

@knight-of-ni knight-of-ni mentioned this pull request May 16, 2016
@connortechnology connortechnology merged commit a826931 into ZoneMinder:master May 17, 2016
@knight-of-ni
Copy link
Member

knight-of-ni commented May 17, 2016

Hey, wait a second. We don't know if this PR makes the issue better or worse because I have not tested it yet.

@connortechnology
Copy link
Member

This code is better. It finds my Trendnet TV-IP862-IC

@knight-of-ni
Copy link
Member

knight-of-ni commented Jun 16, 2016

@altaroca the onvif-probe is no working at all for the following environments:

CentOS 7 - I get the same error @siigna reported here: #1397 (comment)

Fedora 23 - I get the same error @jburgess777 reported earlier in this thread: #1411 (comment)

@siigna
Copy link

siigna commented Jun 16, 2016

CentOS 7 - I get the same error @siigna reported here: #1346 (comment)

Sorry, deleted my comment from that issue and moved it over to #1397.

@knight-of-ni
Copy link
Member

@siigna I've updated my previous post to reflect your move.

We previously had the ONVIF probe working much better than in our previous release, but it looks like we've had a rather serious regression as it doesn't even run anymore the systems I test from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants