diff --git a/CHANGES.md b/CHANGES.md index 3c0d822a..54bfc2ac 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,9 @@ Unreleased * Clock data with a milliseconds digit above 9 is now rejected, by the client when reading the clock and by the server when setting it. Previously `12 a3` decoded to `.130` (#966). +* `parse_tag` now raises `ValueError` under `strict=True`, the default, for a type with no known size, such as + `DB1.DBB0:STRING` without a length. Previously this failed only when `.size` was read. `S5TIME` now has a size + and can be read, but not yet written (#974). * `read_szl` and `list_blocks_of_type` now check the PDU reference of follow-up replies, in both clients. Previously a stale fragment was added to the result (#967). diff --git a/doc/API/tags.rst b/doc/API/tags.rst index 094d0878..ce44429d 100644 --- a/doc/API/tags.rst +++ b/doc/API/tags.rst @@ -102,7 +102,7 @@ Supported types ``UINT``, ``WORD``, ``DINT``, ``UDINT``, ``DWORD``, ``LINT``, ``ULINT``, ``LWORD``, ``REAL``, ``LREAL``, ``TIME``, ``LTIME``, ``TOD``, ``LTOD``, ``DATE``, ``DT``, ``LDT``, ``DTL``, ``STRING[n]``, ``WSTRING[n]``, -``FSTRING[n]``. +``FSTRING[n]``. ``S5TIME`` can be read but not yet written. Arrays are supported for any fixed-size type via ``[count]`` suffix. diff --git a/snap7/tags.py b/snap7/tags.py index 79da0fab..a45986d6 100644 --- a/snap7/tags.py +++ b/snap7/tags.py @@ -62,6 +62,7 @@ "WORD": 2, "WCHAR": 2, "DATE": 2, + "S5TIME": 2, "DINT": 4, "UDINT": 4, "DWORD": 4, @@ -440,9 +441,11 @@ def parse_tag(address: str, *, strict: bool = True, name: str = "") -> Tag: Args: address: Tag address string. strict: When ``True`` (default), require one of the dialect markers - above. Bare short forms like ``M7.1`` or ``IW22`` raise + above and a type with a known size. Bare short forms like ``M7.1`` + or ``IW22``, unknown types and ``STRING`` without a length raise :class:`ValueError`. When ``False``, bare forms are dispatched - to the nodeS7 parser (which accepts them). + to the nodeS7 parser (which accepts them), and the type is not + checked until :attr:`Tag.size` is read. name: Optional tag name to store on the resulting Tag. Returns: @@ -450,20 +453,28 @@ def parse_tag(address: str, *, strict: bool = True, name: str = "") -> Tag: detected. Raises: - ValueError: If the input is ambiguous under strict mode, or if - the selected parser fails to parse. + ValueError: If the input is ambiguous or its type has no known size + under strict mode, or if the selected parser fails to parse. """ s = address.strip() + tag: Tag if "," in s: - return NodeS7Tag.parse(s, name) - if ":" in s: - return PLC4XTag.parse(s, name) - if strict: + tag = NodeS7Tag.parse(s, name) + elif ":" in s: + tag = PLC4XTag.parse(s, name) + elif strict: raise ValueError( f"Ambiguous tag syntax {address!r}: must contain ',' (nodeS7) " f"or ':TYPE' (PLC4X). Pass strict=False to accept bare short forms." ) - return NodeS7Tag.parse(s, name) + else: + tag = NodeS7Tag.parse(s, name) + if strict: + try: + tag.size + except ValueError as e: + raise ValueError(f"Tag address {address!r} has a type with no known size: {tag.datatype}") from e + return tag # --------------------------------------------------------------------------- @@ -527,6 +538,8 @@ def _decode_scalar(datatype: str, data: bytearray, bit: int) -> Any: return util.get_wchar(data, 0) if datatype == "DATE": return util.get_date(data, 0) + if datatype == "S5TIME": + return util.get_s5time(data, 0) if datatype == "DINT": return util.get_dint(data, 0) if datatype == "UDINT": diff --git a/tests/test_tags.py b/tests/test_tags.py index 868103e1..0655b232 100644 --- a/tests/test_tags.py +++ b/tests/test_tags.py @@ -6,6 +6,7 @@ import pytest from snap7.tags import NodeS7Tag, PLC4XTag, Tag, decode_tag, encode_tag, from_browse, load_csv, load_json, load_tia_xml, parse_tag +from snap7 import util from snap7.type import Area @@ -186,6 +187,31 @@ def test_private_names_still_work(self) -> None: assert _encode_tag is encode_tag +class TestParseTagKnownSize: + @pytest.mark.parametrize("address", ["DB1.DBW0:BOGUS", "DB1.DBB0:STRING", "DB1:0:WSTRING", "MW0:BOGUS"]) + def test_strict_refuses_a_type_without_a_size(self, address: str) -> None: + with pytest.raises(ValueError, match="no known size"): + parse_tag(address) + + def test_not_strict_leaves_the_type_unchecked(self) -> None: + tag = parse_tag("DB1.DBW0:BOGUS", strict=False) + assert tag.datatype == "BOGUS" + with pytest.raises(ValueError, match="Unknown S7 type"): + tag.size + + def test_explicit_parsers_are_unchanged(self) -> None: + assert PLC4XTag.parse("DB1.DBW0:BOGUS").datatype == "BOGUS" + + def test_s5time_has_a_size_and_decodes(self) -> None: + tag = parse_tag("DB1.DBW0:S5TIME") + assert tag.size == 2 + assert decode_tag(tag, bytearray(bytes.fromhex("0123"))) == util.get_s5time(bytearray(bytes.fromhex("0123")), 0) + + def test_s5time_write_is_refused(self) -> None: + with pytest.raises(ValueError, match="S5TIME"): + encode_tag(parse_tag("DB1.DBW0:S5TIME"), bytearray(2), "10ms") + + class TestLoadCsv: """Load tags from CSV files and strings."""