stlehmann / pyads

Python wrapper for TwinCAT ADS
MIT License
252 stars 93 forks source link

Testserver cannot register multiple device notification #326

Open StefanVanDyck opened 1 year ago

StefanVanDyck commented 1 year ago

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.

RobertoRoos commented 1 year ago

You are correct. Sample script showing the problem:

Code ```python 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