From 5035665f4b903c2d2f356a3a3263b3fbc4647628 Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Fri, 14 Jul 2023 10:54:05 +0200 Subject: [PATCH] refactor the length key code now, instead of using hacks with ODXLINK IDs, proper references to `LengthKeyParameter` are used by `ParamLengthInfoType`. This necessitates to include the diag coded types in the reference resolution machinery because `ParamLengthInfoType` is a diag coded type and it needs to resolve the reference to the length key parameter. Signed-off-by: Andreas Lauser Signed-off-by: Gerrit Ecke --- odxtools/dataobjectproperty.py | 8 ++- odxtools/diagcodedtypes.py | 62 +++++++++++++++------- odxtools/parameters/codedconstparameter.py | 23 ++++++-- odxtools/parameters/lengthkeyparameter.py | 19 +++++++ odxtools/parameters/nrcconstparameter.py | 23 ++++++-- tests/test_diag_coded_types.py | 60 +++++++++++++-------- 6 files changed, 142 insertions(+), 53 deletions(-) diff --git a/odxtools/dataobjectproperty.py b/odxtools/dataobjectproperty.py index fed7b808..618d4f9c 100644 --- a/odxtools/dataobjectproperty.py +++ b/odxtools/dataobjectproperty.py @@ -166,12 +166,16 @@ def from_et(et_element, doc_frags: List[OdxDocFragment]) -> "DataObjectProperty" return dop def _build_odxlinks(self) -> Dict[OdxLinkId, Any]: - return super()._build_odxlinks() + result = super()._build_odxlinks() + result.update(self.diag_coded_type._build_odxlinks()) + return result def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase): """Resolves the reference to the unit""" super()._resolve_odxlinks(odxlinks) + self.diag_coded_type._resolve_odxlinks(odxlinks) + self._unit: Optional[Unit] = None if self.unit_ref: self._unit = odxlinks.resolve(self.unit_ref, Unit) @@ -179,6 +183,8 @@ def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase): def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None: super()._resolve_snrefs(diag_layer) + self.diag_coded_type._resolve_snrefs(diag_layer) + @property def unit(self) -> Optional[Unit]: return self._unit diff --git a/odxtools/diagcodedtypes.py b/odxtools/diagcodedtypes.py index bd1fc2b0..b4fcaed4 100644 --- a/odxtools/diagcodedtypes.py +++ b/odxtools/diagcodedtypes.py @@ -2,7 +2,7 @@ # Copyright (c) 2022 MBition GmbH import abc import math -from typing import Any, List, Optional, Union +from typing import TYPE_CHECKING, Any, List, Optional, Union import bitstruct @@ -10,9 +10,13 @@ from .encodestate import EncodeState from .exceptions import DecodeError, EncodeError from .globals import logger, xsi -from .odxlink import OdxDocFragment, OdxLinkId +from .odxlink import OdxDocFragment, OdxLinkDatabase, OdxLinkId, OdxLinkRef from .odxtypes import DataType, odxstr_to_bool +if TYPE_CHECKING: + from .diaglayer import DiagLayer + from .parameters.lengthkeyparameter import LengthKeyParameter + ODX_TYPE_TO_FORMAT_LETTER = { DataType.A_INT32: "s", DataType.A_UINT32: "u", @@ -40,6 +44,17 @@ def __init__( self.base_type_encoding = base_type_encoding self.is_highlow_byte_order_raw = is_highlow_byte_order_raw + def _build_odxlinks(self): + return {} + + def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None: + """Recursively resolve any odxlinks references""" + pass + + def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None: + """Recursively resolve any short-name references""" + pass + @property def is_highlow_byte_order(self) -> bool: return self.is_highlow_byte_order_raw in [None, True] @@ -468,7 +483,7 @@ def __init__( self, *, base_data_type: Union[str, DataType], - length_key_id: OdxLinkId, + length_key_ref: OdxLinkRef, base_type_encoding: Optional[str], is_highlow_byte_order_raw: Optional[bool], ): @@ -478,11 +493,28 @@ def __init__( base_type_encoding=base_type_encoding, is_highlow_byte_order_raw=is_highlow_byte_order_raw, ) - self.length_key_id = length_key_id + self.length_key_ref = length_key_ref + + def _build_odxlinks(self): + return super()._build_odxlinks() + + def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None: + """Recursively resolve any odxlinks references""" + super()._resolve_odxlinks(odxlinks) + + self._length_key = odxlinks.resolve(self.length_key_ref) + + def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None: + """Recursively resolve any short-name references""" + super()._resolve_snrefs(diag_layer) + + @property + def length_key(self) -> "LengthKeyParameter": + return self._length_key def convert_internal_to_bytes(self, internal_value, encode_state: EncodeState, bit_position: int) -> bytes: - bit_length = encode_state.length_keys.get(self.length_key_id, None) + bit_length = encode_state.length_keys.get(self.length_key.odx_id, None) if bit_length is None: if self.base_data_type in [ @@ -502,7 +534,7 @@ def convert_internal_to_bytes(self, internal_value, encode_state: EncodeState, bit_length = math.ceil(bit_length / 8.0) * 8 assert bit_length is not None - encode_state.length_keys[self.length_key_id] = bit_length + encode_state.length_keys[self.length_key.odx_id] = bit_length return self._to_bytes( internal_value, @@ -519,14 +551,14 @@ def convert_bytes_to_internal(self, decode_state: DecodeState, bit_position: int # if isinstance(param_value.parameter, LengthKeyParameter) would be prettier, # but leads to cyclic import... if (parameter.parameter_type == "LENGTH-KEY" and - parameter.odx_id == self.length_key_id # type: ignore + parameter.odx_id == self.length_key.odx_id # type: ignore ): # The bit length of the parameter to be extracted is given by the length key. assert isinstance(value, int) bit_length = value break - assert bit_length is not None, f"Did not find any length key with ID {self.length_key_id}" + assert bit_length is not None, f"Did not find any length key with ID {self.length_key.odx_id}" # Extract the internal value and return. return self._extract_internal( @@ -539,7 +571,7 @@ def convert_bytes_to_internal(self, decode_state: DecodeState, bit_position: int ) def __repr__(self) -> str: - repr_str = f"ParamLengthInfoType(base_data_type='{self.base_data_type}', length_key_id={self.length_key_id}" + repr_str = f"ParamLengthInfoType(base_data_type='{self.base_data_type}', length_key={self.length_key.short_name}" if self.base_type_encoding is not None: repr_str += f", base_type_encoding={self.base_type_encoding}" if not self.is_highlow_byte_order: @@ -654,19 +686,11 @@ def create_any_diag_coded_type_from_et(et_element, doc_frags: List[OdxDocFragmen is_highlow_byte_order_raw=is_highlow_byte_order_raw, ) elif dct_type == "PARAM-LENGTH-INFO-TYPE": - # TODO: This is a bit hacky: we make an ID where the data - # specifies a reference. The reason is that we need to store - # the result in an associative array which is not possible - # with references. Note that we currently ignore the DOCREF - # attribute of the reference, so if there are any ID conflicts - # between document fragments, the encoding process will go - # wrong... - length_key_elem = et_element.find("LENGTH-KEY-REF") - length_key_id = OdxLinkId(length_key_elem.attrib["ID-REF"], doc_frags) + length_key_ref = OdxLinkRef.from_et(et_element.find("LENGTH-KEY-REF"), doc_frags) return ParamLengthInfoType( base_data_type=base_data_type, - length_key_id=length_key_id, + length_key_ref=length_key_ref, base_type_encoding=base_type_encoding, is_highlow_byte_order_raw=is_highlow_byte_order_raw, ) diff --git a/odxtools/parameters/codedconstparameter.py b/odxtools/parameters/codedconstparameter.py index d1577f8a..738684d1 100644 --- a/odxtools/parameters/codedconstparameter.py +++ b/odxtools/parameters/codedconstparameter.py @@ -1,15 +1,19 @@ # SPDX-License-Identifier: MIT # Copyright (c) 2022 MBition GmbH import warnings -from typing import ByteString, Union +from typing import TYPE_CHECKING, Any, ByteString, Dict, List, Union from ..decodestate import DecodeState from ..diagcodedtypes import DiagCodedType from ..encodestate import EncodeState from ..exceptions import DecodeError +from ..odxlink import OdxLinkDatabase, OdxLinkId from ..odxtypes import DataType from .parameterbase import Parameter +if TYPE_CHECKING: + from ..diaglayer import DiagLayer + class CodedConstParameter(Parameter): @@ -17,13 +21,22 @@ def __init__(self, *, diag_coded_type: DiagCodedType, coded_value: Union[int, By **kwargs): super().__init__(parameter_type="CODED-CONST", **kwargs) - self._diag_coded_type = diag_coded_type + self.diag_coded_type = diag_coded_type assert isinstance(coded_value, (int, bytes, bytearray)) self.coded_value = coded_value - @property - def diag_coded_type(self): - return self._diag_coded_type + def _build_odxlinks(self) -> Dict[OdxLinkId, Any]: + result = super()._build_odxlinks() + + result.update(self.diag_coded_type._build_odxlinks()) + + return result + + def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None: + super()._resolve_odxlinks(odxlinks) + + def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None: + super()._resolve_snrefs(diag_layer) @property def bit_length(self): diff --git a/odxtools/parameters/lengthkeyparameter.py b/odxtools/parameters/lengthkeyparameter.py index 873f4d45..b77826de 100644 --- a/odxtools/parameters/lengthkeyparameter.py +++ b/odxtools/parameters/lengthkeyparameter.py @@ -1,8 +1,14 @@ # SPDX-License-Identifier: MIT # Copyright (c) 2022 MBition GmbH +from typing import TYPE_CHECKING, Any, Dict, Tuple + from ..encodestate import EncodeState +from ..odxlink import OdxLinkDatabase, OdxLinkId, OdxLinkRef from .parameterwithdop import ParameterWithDOP +if TYPE_CHECKING: + from diaglayer import DiagLayer + class LengthKeyParameter(ParameterWithDOP): """Length Keys specify the bit (!) length of another parameter. @@ -18,6 +24,19 @@ def __init__(self, *, odx_id, **kwargs): super().__init__(parameter_type="LENGTH-KEY", **kwargs) self.odx_id = odx_id + def _build_odxlinks(self) -> Dict[OdxLinkId, Any]: + result = super()._build_odxlinks() + + result[self.odx_id] = self + + return result + + def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None: + super()._resolve_odxlinks(odxlinks) + + def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None: + super()._resolve_snrefs(diag_layer) + def is_required(self): return False diff --git a/odxtools/parameters/nrcconstparameter.py b/odxtools/parameters/nrcconstparameter.py index bf75fcb9..504fa044 100644 --- a/odxtools/parameters/nrcconstparameter.py +++ b/odxtools/parameters/nrcconstparameter.py @@ -1,15 +1,19 @@ # SPDX-License-Identifier: MIT # Copyright (c) 2022 MBition GmbH import warnings -from typing import List +from typing import TYPE_CHECKING, Any, Dict, List from ..decodestate import DecodeState from ..diagcodedtypes import DiagCodedType from ..encodestate import EncodeState from ..exceptions import DecodeError, EncodeError +from ..odxlink import OdxLinkDatabase, OdxLinkId from ..odxtypes import DataType from .parameterbase import Parameter +if TYPE_CHECKING: + from ..diaglayer import DiagLayer + class NrcConstParameter(Parameter): """A param of type NRC-CONST defines a set of values to be matched. @@ -24,14 +28,23 @@ class NrcConstParameter(Parameter): def __init__(self, *, diag_coded_type: DiagCodedType, coded_values: List[int], **kwargs): super().__init__(parameter_type="NRC-CONST", **kwargs) - self._diag_coded_type = diag_coded_type + self.diag_coded_type = diag_coded_type # TODO: Does it have to be an integer or is that just common practice? assert all(isinstance(coded_value, int) for coded_value in coded_values) self.coded_values = coded_values - @property - def diag_coded_type(self): - return self._diag_coded_type + def _build_odxlinks(self) -> Dict[OdxLinkId, Any]: + result = super()._build_odxlinks() + + result.update(self.diag_coded_type._build_odxlinks()) + + return result + + def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None: + super()._resolve_odxlinks(odxlinks) + + def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None: + super()._resolve_snrefs(diag_layer) @property def bit_length(self): diff --git a/tests/test_diag_coded_types.py b/tests/test_diag_coded_types.py index 669f62c1..b789e3de 100644 --- a/tests/test_diag_coded_types.py +++ b/tests/test_diag_coded_types.py @@ -333,31 +333,31 @@ class TestParamLengthInfoType(unittest.TestCase): def test_decode_param_info_length_type_uint(self): length_key_id = OdxLinkId("param.length_key", doc_frags) + length_key_ref = OdxLinkRef.from_id(length_key_id) + length_key = LengthKeyParameter( + odx_id=length_key_id, + short_name="length_key", + long_name=None, + description=None, + semantic=None, + sdgs=[], + dop_ref=OdxLinkRef("DOP.uint8", doc_frags), + dop_snref=None, + byte_position=1, + bit_position=None, + ) dct = ParamLengthInfoType( base_data_type="A_UINT32", base_type_encoding=None, - length_key_id=length_key_id, + length_key_ref=length_key_ref, is_highlow_byte_order_raw=None, ) + odxlinks = OdxLinkDatabase() + odxlinks.update({length_key_id: length_key}) + dct._resolve_odxlinks(odxlinks) state = DecodeState( bytes([0x10, 0x12, 0x34, 0x56]), - [ - ParameterValuePair( - parameter=LengthKeyParameter( - short_name="length_key", - long_name=None, - description=None, - semantic=None, - odx_id=length_key_id, - dop_ref=OdxLinkRef("some_dop", doc_frags), - dop_snref=None, - byte_position=None, - bit_position=None, - sdgs=[], - ), - value=16, - ) - ], + [ParameterValuePair(parameter=length_key, value=16)], next_byte_position=1, ) internal, next_byte = dct.convert_bytes_to_internal(state, bit_position=0) @@ -366,12 +366,28 @@ def test_decode_param_info_length_type_uint(self): def test_encode_param_info_length_type_uint(self): length_key_id = OdxLinkId("param.length_key", doc_frags) + length_key = LengthKeyParameter( + odx_id=length_key_id, + short_name="length_key", + long_name=None, + description=None, + semantic=None, + sdgs=[], + dop_ref=OdxLinkRef("DOP.uint8", doc_frags), + dop_snref=None, + byte_position=1, + bit_position=None, + ) + length_key_ref = OdxLinkRef.from_id(length_key_id) dct = ParamLengthInfoType( base_data_type="A_UINT32", base_type_encoding=None, - length_key_id=length_key_id, + length_key_ref=OdxLinkRef.from_id(length_key_id), is_highlow_byte_order_raw=None, ) + odxlinks = OdxLinkDatabase() + odxlinks.update({length_key_id: length_key}) + dct._resolve_odxlinks(odxlinks) state = EncodeState(bytes([0x10]), {}, length_keys={length_key_id: 40}) byte_val = dct.convert_internal_to_bytes(0x12345, state, bit_position=0) self.assertEqual(byte_val, bytes([0x0, 0x0, 0x1, 0x23, 0x45])) @@ -392,8 +408,7 @@ def test_end_to_end(self): ParamLengthInfoType( base_data_type="A_UINT32", base_type_encoding=None, - length_key_id=OdxLinkId( - "BV.dummy_DL.RQ.sendCertificate.lengthOfCertificateClient", doc_frags), + length_key_ref=OdxLinkRef("param.dummy_length_key", doc_frags), is_highlow_byte_order_raw=None, ), } @@ -470,8 +485,7 @@ def test_end_to_end(self): description="Length parameter for certificateClient.", semantic=None, # LengthKeyParams have an ID to be referenced by a ParamLengthInfoType (which is a diag coded type) - odx_id=diagcodedtypes["length_key_id_to_lengthOfCertificateClient"] - .length_key_id, + odx_id=OdxLinkId("param.dummy_length_key", doc_frags), byte_position=1, bit_position=None, # The DOP multiplies the coded value by 8, since the length key ref expects the number of bits.