Introduce TopicFilter support

[matthax]

Hey, I'll start by saying I'm new to this and I just ran into an issue where I could not control topic subscriptions, so I've tried to add it a few ways and I wanted to bring that to this PR and get some feedback.

My first attempt was to simply set the Filter something like

subscription_params["Filter"] = {
    "_value_1": "*",
}

This doesn't break, but it doesn't actually work. The actual definition can be found in the spec

<xsd:complexType name="TopicExpressionType" mixed="true">
  <xsd:simpleContent>
    <xsd:extension base="xsd:string">
      <xsd:attribute name="Dialect" type="xsd:anyURI" use="required" />
      <xsd:anyAttribute/>
    </xsd:extension>
  </xsd:simpleContent>
</xsd:complexType>

So I added a constructor to types.py but I'm not sure that's really right, and the reason is because I'm relying on the zeep client to actually build the TopicExpressionType. Maybe there's a better way to do this without requiring the client binding?

But, this approach does make it easy for clients to call in, you just specify a filter as a string, and the dialect is set automatically for you. Painless, but you lose the ability to specify a different dialect (This could also be added as a parameter to the constructor too, if you'd prefer that). It also makes it a little easier to do some validation, (e.g. the XPATH is valid syntactically).

Another approach would be to allow the filter to be passed in fully, and make it the callers problem to build the TopicExpression. Maybe it's still worthwhile to provide a constructor or utility to make life easier, but that would give callers the ability to set their own dialect and move the whole zeep client dependency coupling totally out of the manager. It's also worth noting that cameras with FixedTopicSet = True don't seem to actually support XPATHs, they expect a specific selection. If the selection is invalid, you'll get a pretty gnarly exception. In my application I use GetEventProperties to validate the selection, it didn't seem to be something that would really fit in the manager class, but if you wanted that as an example or something I could probably pull it together.

Let me know what you think would be the right approach or if I can make any changes, just thought it was useful to limit the subscriptions.

I also fixed an issue with the MINIMUM_SUBSCRIPTION_SECONDS, I should probably make that a different PR but if you specify an interval under the 60s, the subscription won't renew in time, subsequent calls fail on connection timeouts and things are sad. I just enforced the minimum to get around this.

[codecov]

Codecov Report

❌ Patch coverage is 56.00000% with 11 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
onvif/managers.py	36.36%	7 Missing ⚠️
onvif/types.py	76.92%	3 Missing ⚠️
onvif/client.py	0.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
onvif/client.py	67.28% <0.00%> (ø)
onvif/types.py	91.02% <76.92%> (-2.83%)	⬇️
onvif/managers.py	32.72% <36.36%> (+0.67%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jamesst20]

Thank you @matthax you've helped me figure out why I couldn't get Subscribe to work.

I kept getting errors like

Any element received object of type 'str', expected lxml.etree._Element or builtins.dict or zeep.xsd.valueobjects.AnyObject
See http://docs.python-zeep.org/en/master/datastructures.html#any-objects for more information

and I ended up on this PR and I also had the same error with your implementation. This is when I figured out something else must have been wrong instead of my payload and I ended up editing b-2.xsd like this:

and it worked right away! Just though I would share! In my case I went for a simpler approach that is equivalent to yours I believe

        if filter_expression:
            topic_expr_element = self.client.get_element("wsnt:TopicExpression")
            topic_expr_type = self.client.get_type("wsnt:TopicExpressionType")
            topic_expr = topic_expr_type(
                filter_expression, Dialect="http://www.onvif.org/ver10/tev/topicExpression/ConcreteSet"
            )
            any_topic_expr = AnyObject(topic_expr_element, topic_expr)
            message["Filter"] = {"_value_1": [any_topic_expr]}

I am not sure why the definitions of b-2.xsd are different for TopicExpressionType here http://docs.oasis-open.org/wsn/b-2.xsd

[matthax]

Thank you @matthax you've helped me figure out why I couldn't get Subscribe to work.

I kept getting errors like

Any element received object of type 'str', expected lxml.etree._Element or builtins.dict or zeep.xsd.valueobjects.AnyObject

See http://docs.python-zeep.org/en/master/datastructures.html#any-objects for more information

and I ended up on this PR and I also had the same error with your implementation. This is when I figured out something else must have been wrong instead of my payload and I ended up editing b-2.xsd like this:

and it worked right away! Just though I would share! In my case I went for a simpler approach that is equivalent to yours I believe

        if filter_expression:

            topic_expr_element = self.client.get_element("wsnt:TopicExpression")

            topic_expr_type = self.client.get_type("wsnt:TopicExpressionType")

            topic_expr = topic_expr_type(

                filter_expression, Dialect="http://www.onvif.org/ver10/tev/topicExpression/ConcreteSet"

            )

            any_topic_expr = AnyObject(topic_expr_element, topic_expr)

            message["Filter"] = {"_value_1": [any_topic_expr]}

I am not sure why the definitions of b-2.xsd are different for TopicExpressionType here http://docs.oasis-open.org/wsn/b-2.xsd

Ah! I should have thought of just editing the file, this is much cleaner as an approach, nice work! Maybe we just need that change in the upstream onvif library then and I could clean up the manual construction I'm doing?

[jamesst20]

<xsd:extension base="xsd:string">

You don't need to edit the file, I've actually extracted the definition from https://github.com/openvideolibs/python-onvif-zeep-async/blob/async/onvif/wsdl/b-2.xsd#L51 because the latest version here http://docs.oasis-open.org/wsn/b-2.xsd appears to not be working. My demo should work for python-onvif-zeep-async as well without any extra work.

In case it complains about not finding the namespace wsnt, you can simply register the alias to the client or use the full URL

client.set_ns_prefix("wsnt", "http://docs.oasis-open.org/wsn/b-2")