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

fix: prefer given SimpleItem if available #502

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

swoga
Copy link
Contributor

@swoga swoga commented Oct 8, 2024

fixes: home-assistant/core#123617

Starting with Axis OS 11.11, several SimpleItem entries are sent, which means that the existing method of determining the value fails.

I have extended extract_name_value to prefer a certain SimpleItem active if it exists.
I have tried to keep it as downward compatible as possible, but I can only test with Axis OS 11.11.109 and ObjectAnalytics.

payload example:

<?xml version="1.0" encoding="UTF-8"?>
<tt:MetadataStream xmlns:tt="http://www.onvif.org/ver10/schema">
    <tt:Event>
        <wsnt:NotificationMessage xmlns:tns1="http://www.onvif.org/ver10/topics"
            xmlns:tnsaxis="http://www.axis.com/2009/event/topics"
            xmlns:wsnt="http://docs.oasis-open.org/wsn/b-2"
            xmlns:wsa5="http://www.w3.org/2005/08/addressing">
            <wsnt:Topic Dialect="http://docs.oasis-open.org/wsn/t-1/TopicExpression/Simple">tnsaxis:CameraApplicationPlatform/ObjectAnalytics/Device1Scenario2</wsnt:Topic>
            <wsnt:ProducerReference>
                <wsa5:Address>uri://cfbf96f2-9dc7-437f-aa12-155aacbb0b8f/ProducerReference</wsa5:Address>
            </wsnt:ProducerReference>
            <wsnt:Message>
                <tt:Message UtcTime="2024-10-07T21:09:26.046851Z" PropertyOperation="Changed">
                    <tt:Source></tt:Source>
                    <tt:Key></tt:Key>
                    <tt:Data>
                        <tt:SimpleItem Name="classTypes" Value="human"/>
                        <tt:SimpleItem Name="active" Value="1"/>
                        <tt:SimpleItem Name="objectId" Value="234232"/>
                    </tt:Data>
                </tt:Message>
            </wsnt:Message>
        </wsnt:NotificationMessage>
    </tt:Event>
</tt:MetadataStream>

@Kane610
Copy link
Owner

Kane610 commented Oct 10, 2024

Awesome! I'll get back to you during the weekend, you can start by adding a test that covers this situation

@swoga swoga force-pushed the fix-prefer-simpleitem branch from 49908c8 to c04212c Compare October 10, 2024 17:31
@swoga
Copy link
Contributor Author

swoga commented Oct 10, 2024

added a test!
do you happen to have a payload for Axis OS < 11.11 so we can test for regression?

@Kane610
Copy link
Owner

Kane610 commented Oct 12, 2024

added a test! do you happen to have a payload for Axis OS < 11.11 so we can test for regression?

I'll try to fix one tomorrow, is it a property changed you want? what should the configuration be?

@swoga swoga force-pushed the fix-prefer-simpleitem branch from c04212c to ebb3c10 Compare October 12, 2024 21:26
@swoga
Copy link
Contributor Author

swoga commented Oct 12, 2024

I'll try to fix one tomorrow, is it a property changed you want? what should the configuration be?

After further thinking, I don't think we need a payload to test for backwards compatibility after this fix, this should have already been done by other test payloads (like VMD4_ANY_CHANGE).

I have corrected the formatting objected to by the workflow and pushed the branch.

swoga added 2 commits October 13, 2024 00:29
starting with Axis OS 11.11, several SimpleItem entries are sent, which means that the existing method of determining the value fails
@swoga swoga force-pushed the fix-prefer-simpleitem branch from ebb3c10 to 9cbe29a Compare October 12, 2024 22:34
@swoga
Copy link
Contributor Author

swoga commented Oct 12, 2024

rewrote filter to generator expression: python/mypy#12682 (comment)

-            item = next(filter(lambda x: x.get("@Name", "") == prefer, item), item[0])
+            item = next(
+               (item for item in item if item.get("@Name", "") == prefer), item[0]
+            )

Comment on lines +130 to +135
if prefer is None:
item = item[0]
else:
item = next(
(item for item in item if item.get("@Name", "") == prefer), item[0]
)
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry for not commenting earlier, I've been thinking if this would better keep the whole dict as the other information also could be interesting to expose. What do you think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be interesting, but I didn't quite understand the interaction of all components and just wanted a minimally invasive fix for the bug.

@Kane610
Copy link
Owner

Kane610 commented Oct 21, 2024

I will try to get back to you by tomorrow night

Copy link
Owner

@Kane610 Kane610 left a comment

Choose a reason for hiding this comment

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

Great!

@Kane610 Kane610 merged commit 60b812a into Kane610:master Oct 21, 2024
1 check passed
@swoga swoga deleted the fix-prefer-simpleitem branch October 21, 2024 21:18
@swoga
Copy link
Contributor Author

swoga commented Oct 31, 2024

Just a note in case it becomes relevant later, this issue resolved itself on my Axis cameras before the version with this fix was rolled out.
I have changed a few other (seemingly unrelated) settings on the cameras and also restarted them a few times.
Suddenly the SimpleItems are again in a sequence (active in first place) where they can be correctly evaluated by the previous logic.

@Kane610
Copy link
Owner

Kane610 commented Nov 1, 2024

If it happened once it can happen again. So its good to keep. Thanks for the update

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

Successfully merging this pull request may close these issues.

Event from Axis Object Analysis not reported
2 participants