feat(managers): topic filter support for PullPoint (rebase of #142)#293
Draft
bluetoothbot wants to merge 2 commits into
Draft
feat(managers): topic filter support for PullPoint (rebase of #142)#293bluetoothbot wants to merge 2 commits into
bluetoothbot wants to merge 2 commits into
Conversation
Plumbs an optional topic_filter through create_pullpoint_manager so callers can subscribe to a subset of events via a WS-Topic expression (defaults to the ONVIF ConcreteSet dialect). Addresses the review feedback on the original PR: * Enforce the minimum subscription interval as a floor instead of a cap. The previous expression `interval if interval <= MIN else MIN` silently truncated every interval over 60s, a regression for existing callers. Replaced with `max(interval, MINIMUM_SUBSCRIPTION_INTERVAL)`. * Type topic_filter as `str | None` to match the actual value used. * Drop the lxml XPath validator (which never accepted ONVIF prefixed topic names like `tns1:Foo` anyway) and expose `topic_filter_dialect` so the grammar matches what is sent on the wire. Default remains ConcreteSet. * Serialise `Filter._value_1` as a list per the WS-Notification FilterType schema (xs:any maxOccurs="unbounded"). * Reject blank topic_filter strings up front with a clear ValueError rather than silently sending an empty Filter to the camera. Tests: * `tests/test_managers.py`: unit-tests for the interval clamp (including a regression test for the inverted cap) and topic_filter validation. * `tests/test_types.py`: covers TopicExpression construction and custom-dialect handling via a real zeep client. * `tests/test_hikvision_e2e.py`: end-to-end coverage that drives create_pullpoint_manager against the fake Hikvision camera and asserts on the serialised Filter / TopicExpression bytes, plus a regression test that no Filter element is sent when topic_filter is omitted. * `tests/fake_hikvision.py`: serve Unsubscribe / Renew on /onvif/Subscription so the e2e tests can exercise the full manager lifecycle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rebase follow-ups after porting openvideolibs#142 onto async: - assign the ValueError message to a variable (ruff EM101) - update test_create_pullpoint_manager for the new topic_filter / topic_filter_dialect arguments forwarded to PullPointManager
bluetoothbot
force-pushed
the
koan/topic-filter-rebase
branch
from
July 9, 2026 09:10
22e9bd7 to
f83b21b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What: Adds an optional
topic_filter(withtopic_filter_dialect) toPullPointManager/create_pullpoint_manager, so callers can restrict aPullPoint subscription to specific ONVIF topics.
Why: Rebases and finishes #142 by @matthax / @jamesst20, which stalled
with merge conflicts against
async(it was ~100 commits behind). Multipleusers in that thread needed topic-scoped subscriptions and hit the
Any element received object of type 'str'error when settingFilterbyhand. The feature commit is preserved under the original author; a follow-up
commit carries only the rebase lint/test fixups.
How:
TopicExpression(intypes.py) wraps the WS-BaseNotificationTopicExpressionTypeas a zeepAnyObjectwith the correctDialect,defaulting to ONVIF's ConcreteSet dialect;
Filter._value_1is sent as alist per the schema's unbounded
xsd:any.BaseManagerfloors the interval withmax(interval, MINIMUM_SUBSCRIPTION_INTERVAL)(the inverted clamp flagged in the original review is fixed here).
test_managers.pythat landed onasyncsince feat: introduce TopicFilter support #142 was opened (rather thanthe standalone file feat: introduce TopicFilter support #142 added); the
TopicExpressiontests were appended totest_types.py. Thefake_hikvision.py/test_hikvision_e2e.pyadditionswere dropped because those files no longer exist on
async.Testing:
pytest tests/→ 268 passed;ruff check .clean. New testscover the interval floor (incl. the regression guard against the old inverted
clamp),
topic_filterblank-string validation, andTopicExpressiondefault/custom dialect serialization.
Known gap: the
_startpath that actually attachesFiltertoCreatePullPointSubscriptionis not exercised end-to-end (the hikvision e2eharness was removed from
async), so that one branch stays unit-untested.Left for a follow-up rather than reintroducing a full fake camera.
Supersedes #142 — happy to close this in favor of the original if the authors
would rather rebase it themselves.
Quality Report
Changes: 87 files changed, 9814 insertions(+), 1474 deletions(-)
Code scan: 21 issue(s) found
README.rst:61— debug print statementREADME.rst:111— debug print statementexamples/events.py:20— debug print statementexamples/events.py:24— debug print statementexamples/events.py:25— debug print statementexamples/events.py:27— debug print statementexamples/events.py:29— debug print statementexamples/events.py:62— debug print statementexamples/events.py:74— debug print statementexamples/events.py:84— debug print statementTests: failed (FAILED)
Branch hygiene: clean
Generated by Kōan