diff --git a/CHANGELOG.md b/CHANGELOG.md index b13574f0..54799d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added +- [#261](https://github.com/stlehmann/pyads/issues/261) `AdsSymbol` now correctly handles function block properties with `{attribute 'monitoring' := 'call'}` (index group `0xF019`) by transparently switching to handle-based access + ### Changed - [#508](https://github.com/stlehmann/pyads/pull/508) Finally we directly use Beckhoff/ADS library, thanks to @pbruenn diff --git a/src/pyads/constants.py b/src/pyads/constants.py index ff6b04cc..4b5f23a3 100644 --- a/src/pyads/constants.py +++ b/src/pyads/constants.py @@ -254,6 +254,7 @@ def PLCTYPE_ARR_USINT(n: int) -> Type[Array]: ADSIGRP_SYM_UPLOADINFO2 = 0xF00F ADSIGRP_SYMNOTE = 0xF010 #: notification of named handle +ADSIGRP_SYM_FB_PROP_CALL = 0xF019 #: function block property with {attribute 'monitoring' := 'call'} ADSIGRP_IOIMAGE_RWIB = 0xF020 #: read/write input byte(s) ADSIGRP_IOIMAGE_RWIX = 0xF021 #: read/write input bit ADSIGRP_IOIMAGE_RWOB = 0xF030 #: read/write output byte(s) diff --git a/src/pyads/symbol.py b/src/pyads/symbol.py index 3ea6566b..4edd9b56 100644 --- a/src/pyads/symbol.py +++ b/src/pyads/symbol.py @@ -111,6 +111,7 @@ def __init__( self._plc = plc self._handles_list: List[Tuple[int, int]] = [] # Notification handles self._auto_update_handle: Optional[Tuple[int, int]] = None + self._acquired_handle: bool = False # True when get_handle() was called internally and must be released # Check if the required info is present: missing_info = index_group is None or index_offset is None or symbol_type is None @@ -162,7 +163,15 @@ def _create_symbol_from_info(self) -> None: info = adsGetSymbolInfo(self._plc._port, self._plc._adr, self.name) self.index_group = info.iGroup - self.index_offset = info.iOffs + if self.index_group == constants.ADSIGRP_SYM_FB_PROP_CALL: + # For function block properties with monitoring = call + # get symbol using handle instead + self.index_group = constants.ADSIGRP_SYM_VALBYHND + self.index_offset = self._plc.get_handle(self.name) + self._acquired_handle = True + else: + self.index_offset = info.iOffs + if info.comment: self.comment = info.comment @@ -233,6 +242,11 @@ def __del__(self) -> None: self.clear_device_notifications() except ADSError: pass # Quietly continue, without a connection no cleanup could be done + if self._acquired_handle: + try: + self._plc.release_handle(self.index_offset) + except ADSError: + pass def add_device_notification( self, diff --git a/tests/test_symbol.py b/tests/test_symbol.py index 02cb3a91..6f78565e 100644 --- a/tests/test_symbol.py +++ b/tests/test_symbol.py @@ -616,6 +616,98 @@ def test_write_control(self): self.plc.write_control(constants.ADSSTATE_IDLE, 0, 0, constants.PLCTYPE_INT) +class FBPropertySymbolTestCase(unittest.TestCase): + """Tests for function block properties with {attribute 'monitoring' := 'call'}. + + These return iGroup=ADSIGRP_SYM_FB_PROP_CALL (0xF019) from adsGetSymbolInfo + and cannot be accessed by offset — a handle must be used instead. + """ + + def _make_plc(self): + plc = mock.MagicMock() + plc._port = 851 + plc._adr = mock.MagicMock() + plc.get_handle.return_value = 42 + return plc + + def _make_info(self, iGroup, iOffs=0, symbol_type="LREAL", comment=""): + info = mock.MagicMock() + info.iGroup = iGroup + info.iOffs = iOffs + info.symbol_type = symbol_type + info.comment = comment + return info + + @mock.patch("pyads.symbol.adsGetSymbolInfo") + def test_fb_property_uses_handle(self, mock_get_info): + """Symbol with iGroup=0xF019 must acquire a handle and switch to VALBYHND.""" + mock_get_info.return_value = self._make_info( + iGroup=constants.ADSIGRP_SYM_FB_PROP_CALL, iOffs=999 + ) + plc = self._make_plc() + + symbol = AdsSymbol(plc, name="FB.Property") + + plc.get_handle.assert_called_once_with("FB.Property") + self.assertEqual(symbol.index_group, constants.ADSIGRP_SYM_VALBYHND) + self.assertEqual(symbol.index_offset, 42) + self.assertTrue(symbol._acquired_handle) + + @mock.patch("pyads.symbol.adsGetSymbolInfo") + def test_normal_symbol_no_handle(self, mock_get_info): + """Normal symbol must use offset directly and must not call get_handle.""" + mock_get_info.return_value = self._make_info( + iGroup=constants.INDEXGROUP_DATA, iOffs=8 + ) + plc = self._make_plc() + + symbol = AdsSymbol(plc, name="GVL.SomeVar") + + plc.get_handle.assert_not_called() + self.assertEqual(symbol.index_group, constants.INDEXGROUP_DATA) + self.assertEqual(symbol.index_offset, 8) + self.assertFalse(symbol._acquired_handle) + + @mock.patch("pyads.symbol.adsGetSymbolInfo") + def test_fb_property_handle_released_on_del(self, mock_get_info): + """release_handle must be called with the acquired handle on destruction.""" + mock_get_info.return_value = self._make_info( + iGroup=constants.ADSIGRP_SYM_FB_PROP_CALL + ) + plc = self._make_plc() + + symbol = AdsSymbol(plc, name="FB.Property") + del symbol + + plc.release_handle.assert_called_once_with(42) + + @mock.patch("pyads.symbol.adsGetSymbolInfo") + def test_normal_symbol_handle_not_released_on_del(self, mock_get_info): + """release_handle must NOT be called for ordinary symbols on destruction.""" + mock_get_info.return_value = self._make_info( + iGroup=constants.INDEXGROUP_DATA, iOffs=8 + ) + plc = self._make_plc() + + symbol = AdsSymbol(plc, name="GVL.SomeVar") + del symbol + + plc.release_handle.assert_not_called() + + @mock.patch("pyads.symbol.adsGetSymbolInfo") + def test_fb_property_handle_release_adsError_suppressed(self, mock_get_info): + """ADSError from release_handle in __del__ must not propagate.""" + from pyads.pyads_ex import ADSError + mock_get_info.return_value = self._make_info( + iGroup=constants.ADSIGRP_SYM_FB_PROP_CALL + ) + plc = self._make_plc() + plc.release_handle.side_effect = ADSError() + + symbol = AdsSymbol(plc, name="FB.Property") + del symbol # Must not raise + + class TypesTestCase(unittest.TestCase): """Basic test to cover the PLCTYPE_ARR_* functions"""