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

Testserver cannot register multiple device notification #326

Open
StefanVanDyck opened this issue Sep 11, 2022 · 1 comment · May be fixed by #329
Open

Testserver cannot register multiple device notification #326

StefanVanDyck opened this issue Sep 11, 2022 · 1 comment · May be fixed by #329

Comments

@StefanVanDyck
Copy link

StefanVanDyck commented Sep 11, 2022

I was trying to use the test server to mock the behavior of the PLC.
Everything worked great, but I ran into some issues when using device notifications.

First I didn't realize until I started debugging the code that the:
The test server has to run in the same python process as the calling code to be able to work.
Seems like it relies on this global callback store variable to be populated by the client somehow?
Not quite sure if that's really the issue, but I just never got it to work running in a forked process or a docker container.
If this is the case, it might be useful to add some additional asterisk in the docs to make this extra clear.

Secondly, when registering multiple device notifications on different variables:
The test server reuses the same handle.
The offending line seems to me to be this one: https://github.com/stlehmann/pyads/blob/master/pyads/testserver/advanced_handler.py#L167
This will not increment handles across different PLCVariables.
Instead changing it to PLCVariable.notification_count kinda makes it work.
Unless device notifications are deleted and then the whole handle things goes out of whack anyway.

In conclusion I suppose the problem is the python code is trying to guess what the handle is the C-library will use, but that guessing is not entirely correct. Not sure how to fix that.
Might be totally wrong about this though, not really sure I understand all the stuff that's happening here.

@StefanVanDyck StefanVanDyck changed the title Testserver cqnnot register multiple device notification Testserver cannot register multiple device notification Sep 11, 2022
@RobertoRoos
Copy link
Contributor

RobertoRoos commented Oct 13, 2022

You are correct. Sample script showing the problem:

Code
import pyads
from pyads.testserver import AdsTestServer, AdvancedHandler, PLCVariable
from pyads import constants

from time import sleep


def main():
    handler = AdvancedHandler()
    test_server = AdsTestServer(handler=handler, logging=False)
    test_server.start()

    test_vars = [
        PLCVariable(
            "Test1", bytes(8), ads_type=constants.ADST_REAL64, symbol_type="LREAL"
        ),
        PLCVariable(
            "Test2", bytes(8), ads_type=constants.ADST_REAL64, symbol_type="LREAL"
        ),
    ]

    for var in test_vars:
        handler.add_variable(var)

    sleep(1)

    plc = pyads.Connection("127.0.0.1.1.1")

    @plc.notification(pyads.PLCTYPE_LREAL)
    def callback_1(*args):
        print("Callback 1:", args)

    @plc.notification(pyads.PLCTYPE_LREAL)
    def callback_2(*args):
        print("Callback 2:", args)

    with plc:
        symbols = [
            plc.get_symbol("Test1"),
            plc.get_symbol("Test2"),
        ]

        symbols[0].add_device_notification(callback_1)
        symbols[1].add_device_notification(callback_2)

        symbols[0].write(64984.262)
        symbols[1].write(-78978.855)

        del symbols

    sleep(1)

    test_server.stop()

    sleep(1)


if __name__ == "__main__":
    main()

Instead, self should be replaced with the class name. It looks like each PlcVariable keeps his own callbacks, but really they are stored in the pyads_ex.callback_store and they need the right reference.

This has no reference to C handles (fortunately maybe). This bit short circumvent the C library completely.

I'll make a tiny PR in a minute. EDIT: #329

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 a pull request may close this issue.

2 participants