Skip to content

Conversation

@ASil-Scanway
Copy link

Fix registering multiple device notification on AdsTestServer variables.

Related issue: #471


The root cause is PLCVariable register_notification method using notification_count as instance field instead of class field.

It seems original intent was to use notification_count in static way

class PLCVariable:
    """Storage item for named data.

    Also include variable type so it can be retrieved later.
    This basically mirrors SAdsSymbolEntry or AdsSymbol, however we want to
    avoid using those directly since they are test subjects.
    """

    handle_count = 10000  # Keep track of the latest awarded handle
    notification_count = 10  # Keep track the latest notification handle

and in constructor, it's used this way

        PLCVariable.handle_count += 1

but register_notification uses it as instance field

    def register_notification(self) -> int:
        """Register a new notification."""

        handle = PLCVariable.notification_count
        self.notifications.append(handle)
        PLCVariable.notification_count += 1
        return handle

This leads to the situation when the same value is reused for multiple notifications if user tries to register notification for different variable (other instance of PLCVariable)

@ASil-Scanway
Copy link
Author

Something is still not right with indices of variables when adding notifications. This needs to be looked into. The test I added showed the issue.
Unfortunately I don't have time left today to handle it further.

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.

1 participant