From 1ed22dc0a78e19591e8c4255361eb416a4f44156 Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Sun, 10 Nov 2024 17:24:26 +0100 Subject: [PATCH 1/9] creating RawDataRecord class with unittests, reorganizing Data Records file structure --- .../database/data_record/__init__.py | 0 .../test_abstract_data_record.py | 4 +- .../data_record/test_raw_data_record.py | 26 ++++++ uds/database/__init__.py | 2 +- uds/database/data_record/__init__.py | 6 ++ .../{ => data_record}/abstract_data_record.py | 2 +- uds/database/data_record/raw_data_record.py | 89 +++++++++++++++++++ 7 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 tests/software_tests/database/data_record/__init__.py rename tests/software_tests/database/{ => data_record}/test_abstract_data_record.py (91%) create mode 100644 tests/software_tests/database/data_record/test_raw_data_record.py create mode 100644 uds/database/data_record/__init__.py rename uds/database/{ => data_record}/abstract_data_record.py (98%) create mode 100644 uds/database/data_record/raw_data_record.py diff --git a/tests/software_tests/database/data_record/__init__.py b/tests/software_tests/database/data_record/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/software_tests/database/test_abstract_data_record.py b/tests/software_tests/database/data_record/test_abstract_data_record.py similarity index 91% rename from tests/software_tests/database/test_abstract_data_record.py rename to tests/software_tests/database/data_record/test_abstract_data_record.py index 87aff108..5a141a2f 100644 --- a/tests/software_tests/database/test_abstract_data_record.py +++ b/tests/software_tests/database/data_record/test_abstract_data_record.py @@ -1,9 +1,9 @@ import pytest from mock import Mock, patch -from uds.database.abstract_data_record import AbstractDataRecord +from uds.database.data_record import AbstractDataRecord -SCRIPT_LOCATION = "uds.database.abstract_data_record" +SCRIPT_LOCATION = "uds.database.data_record" class TestAbstractDataRecord: diff --git a/tests/software_tests/database/data_record/test_raw_data_record.py b/tests/software_tests/database/data_record/test_raw_data_record.py new file mode 100644 index 00000000..af5a94b2 --- /dev/null +++ b/tests/software_tests/database/data_record/test_raw_data_record.py @@ -0,0 +1,26 @@ +import pytest + +from uds.database.data_record import RawDataRecord +from uds.database.data_record.abstract_data_record import DataRecordType + + +def test_raw_data_record_initialization(): + record = RawDataRecord(name="TestRawDataRecord", length=16) + assert record.name == "TestRawDataRecord" + assert record.length == 16 + assert record.data_record_type == DataRecordType.RAW + assert record.is_reoccurring is False + assert record.min_occurrences == 1 + assert record.max_occurrences == 1 + assert record.contains == () + assert record.decode(1234) == 1234 + assert record.encode(1234) == 1234 + + +def test_raw_data_record_invalid_name(): + with pytest.raises(TypeError): + RawDataRecord(name=123, length=16) + +def test_raw_data_record_invalid_length(): + with pytest.raises(ValueError): + RawDataRecord(name="TestRecord", length=-1) diff --git a/uds/database/__init__.py b/uds/database/__init__.py index 7cac16e1..ba7915a0 100644 --- a/uds/database/__init__.py +++ b/uds/database/__init__.py @@ -4,4 +4,4 @@ Tools for decoding and encoding information from/to diagnostic messages. """ -from .abstract_data_record import AbstractDataRecord, DataRecordType, DecodedDataRecord +from .data_record import RawDataRecord diff --git a/uds/database/data_record/__init__.py b/uds/database/data_record/__init__.py new file mode 100644 index 00000000..5d82b4fa --- /dev/null +++ b/uds/database/data_record/__init__.py @@ -0,0 +1,6 @@ +""" +Package with implementation for all type of Data Records. +""" + +from .abstract_data_record import AbstractDataRecord, DataRecordType, DecodedDataRecord +from .raw_data_record import RawDataRecord \ No newline at end of file diff --git a/uds/database/abstract_data_record.py b/uds/database/data_record/abstract_data_record.py similarity index 98% rename from uds/database/abstract_data_record.py rename to uds/database/data_record/abstract_data_record.py index c7811186..6a8010c6 100644 --- a/uds/database/abstract_data_record.py +++ b/uds/database/data_record/abstract_data_record.py @@ -28,12 +28,12 @@ class DataRecordType(ValidatedEnum): """All Data Record types.""" # TODO: fill with following tasks: - # - https://github.com/mdabrowski1990/uds/issues/2 # - https://github.com/mdabrowski1990/uds/issues/6 # - https://github.com/mdabrowski1990/uds/issues/8 # - https://github.com/mdabrowski1990/uds/issues/9 # - https://github.com/mdabrowski1990/uds/issues/10 + RAW = "RAW" class AbstractDataRecord(ABC): """Common implementation and interface for all Data Records.""" diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py new file mode 100644 index 00000000..c1aad80e --- /dev/null +++ b/uds/database/data_record/raw_data_record.py @@ -0,0 +1,89 @@ +from typing import Optional, Tuple + +from .abstract_data_record import AbstractDataRecord, DataRecordType + + +class RawDataRecord(AbstractDataRecord): + """Common implementation and interface for Raw Data Record.""" + + def __init__(self, name: str, length: int) -> None: + """ + Initialization of Raw Data Record. + + :param name: Name to assign to this Data Record. + :param length: Number of bits that this Raw Data Record is stored over. + + :raise TypeError: Provided name is not str type. + :raise ValueError: Provided length is not a positive integer. + """ + super().__init__(name) + if not isinstance(length, int) or length <= 0: + raise ValueError("Length must be a positive integer.") + self.__length = length + + @property + def data_record_type(self) -> DataRecordType: + """Type of this Data Record.""" + return DataRecordType.RAW + + @property + def length(self) -> int: + """Get number of bits that this Data Record is stored over.""" + return self.__length + + @property + def is_reoccurring(self) -> bool: + """ + Whether this Data Record might occur multiple times. + + Values meaning: + - False - exactly one occurrence in every diagnostic message + - True - number of occurrences might vary + """ + return False + + @property + def min_occurrences(self) -> int: + """ + Minimal number of this Data Record occurrences. + + .. note:: Relevant only if :attr:`~uds.database.data_record.raw_data_record.RawDataRecord.is_reoccurring` + equals True. + """ + return 1 + + @property + def max_occurrences(self) -> Optional[int]: + """ + Maximal number of this Data Record occurrences. + + .. note:: Relevant only if :attr:`~uds.database.data_record.raw_data_record.RawDataRecord.is_reoccurring` + equals True. + .. warning:: No maximal number (infinite number of occurrences) is represented by None value. + """ + return 1 + + @property + def contains(self) -> Tuple[AbstractDataRecord, ...]: + """Get Data Records contained by this Data Record.""" + return () + + def decode(self, raw_value: int) -> int: + """ + Decode physical value for provided raw value. + + :param raw_value: Raw (bit) value of Data Record. + + :return: Dictionary with physical value for this Data Record. + """ + return raw_value + + def encode(self, physical_value: int) -> int: + """ + Encode raw value for provided physical value. + + :param physical_value: Physical (meaningful e.g. float, str type) value of this Data Record. + + :return: Raw Value of this Data Record. + """ + return physical_value \ No newline at end of file From d42a59a581e06a97e2953a0f1622c51c94c525de Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Sun, 10 Nov 2024 18:32:55 +0100 Subject: [PATCH 2/9] mypy fixes --- uds/database/data_record/__init__.py | 1 + uds/database/data_record/abstract_data_record.py | 4 ++-- uds/database/data_record/raw_data_record.py | 10 +++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/uds/database/data_record/__init__.py b/uds/database/data_record/__init__.py index 5d82b4fa..1b23f16f 100644 --- a/uds/database/data_record/__init__.py +++ b/uds/database/data_record/__init__.py @@ -1,6 +1,7 @@ """ Package with implementation for all type of Data Records. """ +__all__ = ["AbstractDataRecord", "DataRecordType", "DecodedDataRecord", "RawDataRecord"] from .abstract_data_record import AbstractDataRecord, DataRecordType, DecodedDataRecord from .raw_data_record import RawDataRecord \ No newline at end of file diff --git a/uds/database/data_record/abstract_data_record.py b/uds/database/data_record/abstract_data_record.py index 6a8010c6..ebca1d12 100644 --- a/uds/database/data_record/abstract_data_record.py +++ b/uds/database/data_record/abstract_data_record.py @@ -5,7 +5,7 @@ meaningful information (e.g. physical value, text). """ -__all__ = ["DataRecordType", "AbstractDataRecord", "DecodedDataRecord"] +__all__ = ["DataRecordType", "AbstractDataRecord", "DecodedDataRecord", "DataRecordPhysicalValueAlias"] from abc import ABC, abstractmethod from typing import Tuple, TypedDict, Union, Optional @@ -33,7 +33,7 @@ class DataRecordType(ValidatedEnum): # - https://github.com/mdabrowski1990/uds/issues/9 # - https://github.com/mdabrowski1990/uds/issues/10 - RAW = "RAW" + RAW: "DataRecordType" = "RAW" # type: ignore class AbstractDataRecord(ABC): """Common implementation and interface for all Data Records.""" diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py index c1aad80e..cb67b317 100644 --- a/uds/database/data_record/raw_data_record.py +++ b/uds/database/data_record/raw_data_record.py @@ -1,6 +1,6 @@ from typing import Optional, Tuple -from .abstract_data_record import AbstractDataRecord, DataRecordType +from .abstract_data_record import AbstractDataRecord, DataRecordType, DataRecordPhysicalValueAlias, DecodedDataRecord class RawDataRecord(AbstractDataRecord): @@ -68,7 +68,7 @@ def contains(self) -> Tuple[AbstractDataRecord, ...]: """Get Data Records contained by this Data Record.""" return () - def decode(self, raw_value: int) -> int: + def decode(self, raw_value: int) -> DecodedDataRecord: """ Decode physical value for provided raw value. @@ -76,9 +76,9 @@ def decode(self, raw_value: int) -> int: :return: Dictionary with physical value for this Data Record. """ - return raw_value + return DecodedDataRecord(name=self.name, raw_value=raw_value, physical_value="Raw Data Record") - def encode(self, physical_value: int) -> int: + def encode(self, physical_value: DataRecordPhysicalValueAlias) -> int: """ Encode raw value for provided physical value. @@ -86,4 +86,4 @@ def encode(self, physical_value: int) -> int: :return: Raw Value of this Data Record. """ - return physical_value \ No newline at end of file + return physical_value # type: ignore From 3cfe885744e5b96acdc2ff28dd36118e8bab6dfe Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Mon, 11 Nov 2024 22:13:07 +0100 Subject: [PATCH 3/9] addressing review comments --- .../data_record/test_abstract_data_record.py | 8 +++-- .../data_record/test_raw_data_record.py | 8 ++--- uds/database/__init__.py | 4 ++- uds/database/data_record/__init__.py | 10 ++++-- .../data_record/abstract_data_record.py | 26 +++------------ uds/database/data_record/raw_data_record.py | 33 +++++++++++++------ 6 files changed, 48 insertions(+), 41 deletions(-) diff --git a/tests/software_tests/database/data_record/test_abstract_data_record.py b/tests/software_tests/database/data_record/test_abstract_data_record.py index 5a141a2f..46339e22 100644 --- a/tests/software_tests/database/data_record/test_abstract_data_record.py +++ b/tests/software_tests/database/data_record/test_abstract_data_record.py @@ -1,9 +1,9 @@ import pytest from mock import Mock, patch -from uds.database.data_record import AbstractDataRecord +from uds.database.data_record.abstract_data_record import AbstractDataRecord -SCRIPT_LOCATION = "uds.database.data_record" +SCRIPT_LOCATION = "uds.database.data_record.abstract_data_record" class TestAbstractDataRecord: @@ -35,3 +35,7 @@ def test_init__valid(self, mock_isinstance): def test_name(self): self.mock_data_record._AbstractDataRecord__name = Mock() assert AbstractDataRecord.name.fget(self.mock_data_record) == self.mock_data_record._AbstractDataRecord__name + + def test_data_record_type(self): + self.mock_data_record.__class__.__name__ = "MockDataRecord" + assert AbstractDataRecord.data_record_type.fget(self.mock_data_record) == "MockDataRecord" diff --git a/tests/software_tests/database/data_record/test_raw_data_record.py b/tests/software_tests/database/data_record/test_raw_data_record.py index af5a94b2..f6c84616 100644 --- a/tests/software_tests/database/data_record/test_raw_data_record.py +++ b/tests/software_tests/database/data_record/test_raw_data_record.py @@ -1,19 +1,18 @@ import pytest -from uds.database.data_record import RawDataRecord -from uds.database.data_record.abstract_data_record import DataRecordType +from uds.database.data_record import RawDataRecord, DecodedDataRecord def test_raw_data_record_initialization(): record = RawDataRecord(name="TestRawDataRecord", length=16) assert record.name == "TestRawDataRecord" assert record.length == 16 - assert record.data_record_type == DataRecordType.RAW + assert record.data_record_type == "RawDataRecord" assert record.is_reoccurring is False assert record.min_occurrences == 1 assert record.max_occurrences == 1 assert record.contains == () - assert record.decode(1234) == 1234 + assert record.decode(1234) == DecodedDataRecord(name="TestRawDataRecord", raw_value=1234, physical_value=1234) assert record.encode(1234) == 1234 @@ -21,6 +20,7 @@ def test_raw_data_record_invalid_name(): with pytest.raises(TypeError): RawDataRecord(name=123, length=16) + def test_raw_data_record_invalid_length(): with pytest.raises(ValueError): RawDataRecord(name="TestRecord", length=-1) diff --git a/uds/database/__init__.py b/uds/database/__init__.py index ba7915a0..57f5f951 100644 --- a/uds/database/__init__.py +++ b/uds/database/__init__.py @@ -4,4 +4,6 @@ Tools for decoding and encoding information from/to diagnostic messages. """ -from .data_record import RawDataRecord +__all__ = ["AbstractDataRecord", "DecodedDataRecord", "RawDataRecord"] + +from .data_record import AbstractDataRecord, DecodedDataRecord, RawDataRecord diff --git a/uds/database/data_record/__init__.py b/uds/database/data_record/__init__.py index 1b23f16f..4ffa158f 100644 --- a/uds/database/data_record/__init__.py +++ b/uds/database/data_record/__init__.py @@ -1,7 +1,11 @@ """ Package with implementation for all type of Data Records. + +Each Data Record contains mapping (translation) of raw data (sequence of bits in diagnostic message payload) to some +meaningful information (e.g. physical value, text). """ -__all__ = ["AbstractDataRecord", "DataRecordType", "DecodedDataRecord", "RawDataRecord"] -from .abstract_data_record import AbstractDataRecord, DataRecordType, DecodedDataRecord -from .raw_data_record import RawDataRecord \ No newline at end of file +__all__ = ["AbstractDataRecord", "DecodedDataRecord", "RawDataRecord"] + +from .abstract_data_record import AbstractDataRecord, DecodedDataRecord +from .raw_data_record import RawDataRecord diff --git a/uds/database/data_record/abstract_data_record.py b/uds/database/data_record/abstract_data_record.py index ebca1d12..2e9560dc 100644 --- a/uds/database/data_record/abstract_data_record.py +++ b/uds/database/data_record/abstract_data_record.py @@ -1,16 +1,11 @@ """ -Definition of all Data Records types. - -Each Data Record contains mapping (translation) of raw data (sequence of bits in diagnostic message payload) to some -meaningful information (e.g. physical value, text). +Definition of AbstractDataRecord which is a base class for all Data Records. """ -__all__ = ["DataRecordType", "AbstractDataRecord", "DecodedDataRecord", "DataRecordPhysicalValueAlias"] +__all__ = ["AbstractDataRecord", "DataRecordPhysicalValueAlias", "DecodedDataRecord"] from abc import ABC, abstractmethod -from typing import Tuple, TypedDict, Union, Optional - -from uds.utilities import ValidatedEnum +from typing import Optional, Tuple, TypedDict, Union DataRecordPhysicalValueAlias = Union[int, float, str, Tuple["DecodedDataRecord", ...]] """Alias of Data Records' physical value.""" @@ -24,17 +19,6 @@ class DecodedDataRecord(TypedDict): physical_value: DataRecordPhysicalValueAlias # noqa: F841 -class DataRecordType(ValidatedEnum): - """All Data Record types.""" - - # TODO: fill with following tasks: - # - https://github.com/mdabrowski1990/uds/issues/6 - # - https://github.com/mdabrowski1990/uds/issues/8 - # - https://github.com/mdabrowski1990/uds/issues/9 - # - https://github.com/mdabrowski1990/uds/issues/10 - - RAW: "DataRecordType" = "RAW" # type: ignore - class AbstractDataRecord(ABC): """Common implementation and interface for all Data Records.""" @@ -56,9 +40,9 @@ def name(self) -> str: return self.__name @property # noqa: F841 - @abstractmethod - def data_record_type(self) -> DataRecordType: + def data_record_type(self) -> str: """Type of this Data Record.""" + return self.__class__.__name__ @property # noqa: F841 @abstractmethod diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py index cb67b317..60615d5b 100644 --- a/uds/database/data_record/raw_data_record.py +++ b/uds/database/data_record/raw_data_record.py @@ -1,6 +1,10 @@ +""" +Definition of RawDataRecord. +""" + from typing import Optional, Tuple -from .abstract_data_record import AbstractDataRecord, DataRecordType, DataRecordPhysicalValueAlias, DecodedDataRecord +from .abstract_data_record import AbstractDataRecord, DataRecordPhysicalValueAlias, DecodedDataRecord class RawDataRecord(AbstractDataRecord): @@ -17,20 +21,29 @@ def __init__(self, name: str, length: int) -> None: :raise ValueError: Provided length is not a positive integer. """ super().__init__(name) - if not isinstance(length, int) or length <= 0: - raise ValueError("Length must be a positive integer.") - self.__length = length - - @property - def data_record_type(self) -> DataRecordType: - """Type of this Data Record.""" - return DataRecordType.RAW + self.length = length @property def length(self) -> int: """Get number of bits that this Data Record is stored over.""" return self.__length + @length.setter + def length(self, value: int) -> None: + """ + Set the length, ensuring it's an integer and within an acceptable range. + + :param value: Number of bits that this Data Record is stored over. + + :raise TypeError: Provided value is not int type. + :raise ValueError: Provided value is less or equal 0. + """ + if not isinstance(value, int): + raise TypeError("Length must be an integer.") + if value <= 0: + raise ValueError("Length must be a positive integer.") + self.__length = value + @property def is_reoccurring(self) -> bool: """ @@ -76,7 +89,7 @@ def decode(self, raw_value: int) -> DecodedDataRecord: :return: Dictionary with physical value for this Data Record. """ - return DecodedDataRecord(name=self.name, raw_value=raw_value, physical_value="Raw Data Record") + return DecodedDataRecord(name=self.name, raw_value=raw_value, physical_value=raw_value) def encode(self, physical_value: DataRecordPhysicalValueAlias) -> int: """ From 7547ea034635dd841c83c05bc4f0966fd5e038ea Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Tue, 12 Nov 2024 21:32:30 +0100 Subject: [PATCH 4/9] improving unit tests for RawDataRecord --- .../data_record/test_raw_data_record.py | 98 +++++++++++++++---- uds/database/services/abstract_service.py | 2 +- 2 files changed, 81 insertions(+), 19 deletions(-) diff --git a/tests/software_tests/database/data_record/test_raw_data_record.py b/tests/software_tests/database/data_record/test_raw_data_record.py index f6c84616..9b4c8038 100644 --- a/tests/software_tests/database/data_record/test_raw_data_record.py +++ b/tests/software_tests/database/data_record/test_raw_data_record.py @@ -1,26 +1,88 @@ import pytest +from mock import patch -from uds.database.data_record import RawDataRecord, DecodedDataRecord +from uds.database.data_record.raw_data_record import RawDataRecord +SCRIPT_LOCATION = "uds.database.data_record.raw_data_record" -def test_raw_data_record_initialization(): - record = RawDataRecord(name="TestRawDataRecord", length=16) - assert record.name == "TestRawDataRecord" - assert record.length == 16 - assert record.data_record_type == "RawDataRecord" - assert record.is_reoccurring is False - assert record.min_occurrences == 1 - assert record.max_occurrences == 1 - assert record.contains == () - assert record.decode(1234) == DecodedDataRecord(name="TestRawDataRecord", raw_value=1234, physical_value=1234) - assert record.encode(1234) == 1234 +class TestRawDataRecord: -def test_raw_data_record_invalid_name(): - with pytest.raises(TypeError): - RawDataRecord(name=123, length=16) + def setup_method(self): + self.name = "TestRawDataRecord" + self.length = 8 + self.raw_data_record = RawDataRecord(self.name, self.length) + # __init__ -def test_raw_data_record_invalid_length(): - with pytest.raises(ValueError): - RawDataRecord(name="TestRecord", length=-1) + def test_init_valid(self): + assert self.raw_data_record.name == self.name + assert self.raw_data_record.length == self.length + + def test_init_type_error_for_name(self): + with pytest.raises(TypeError): + RawDataRecord(123, self.length) + + def test_init_value_error_for_length(self): + with pytest.raises(ValueError): + RawDataRecord(self.name, -5) + + def test_init_type_error_for_length(self): + with pytest.raises(TypeError): + RawDataRecord(self.name, "eight") + + # length + + def test_length_getter(self): + assert self.raw_data_record.length == self.length + + def test_length_setter_valid(self): + self.raw_data_record.length = 16 + assert self.raw_data_record.length == 16 + + def test_length_setter_type_error(self): + with pytest.raises(TypeError): + self.raw_data_record.length = "sixteen" + + def test_length_setter_value_error(self): + with pytest.raises(ValueError): + self.raw_data_record.length = -10 + + # is_reoccurring + + def test_is_reoccurring(self): + assert self.raw_data_record.is_reoccurring is False + + # min_occurrences + + def test_min_occurrences(self): + assert self.raw_data_record.min_occurrences == 1 + + # max_occurrences + + def test_max_occurrences(self): + assert self.raw_data_record.max_occurrences == 1 + + # contains + + def test_contains(self): + assert self.raw_data_record.contains == () + + # decode + + @patch(f"{SCRIPT_LOCATION}.DecodedDataRecord") + def test_decode(self, mock_decoded_data_record): + raw_value = 42 + result = self.raw_data_record.decode(raw_value) + mock_decoded_data_record.assert_called_once_with( + name=self.name, + raw_value=raw_value, + physical_value=raw_value + ) + assert result == mock_decoded_data_record.return_value + + # encode + + def test_encode(self): + physical_value = 42 + assert self.raw_data_record.encode(physical_value) == physical_value diff --git a/uds/database/services/abstract_service.py b/uds/database/services/abstract_service.py index 79b74d7f..0bef924c 100644 --- a/uds/database/services/abstract_service.py +++ b/uds/database/services/abstract_service.py @@ -8,7 +8,7 @@ from uds.message import RequestSID, ResponseSID from uds.utilities import RawBytesAlias, RawBytesListAlias -from ..abstract_data_record import DecodedDataRecord +from ..data_record import DecodedDataRecord DataRecordValueAlias = Union[int, float, str, Iterable[Dict[str, "DataRecordValueAlias"]]] "Alias of input with Data Records values." From 6d0f8ef8ec70c6c5f6f85137a80452fb973878ac Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Tue, 12 Nov 2024 21:59:56 +0100 Subject: [PATCH 5/9] fixes for static code analyzer --- .../data_record/abstract_data_record.py | 4 +--- uds/database/data_record/raw_data_record.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/uds/database/data_record/abstract_data_record.py b/uds/database/data_record/abstract_data_record.py index 2e9560dc..8a7a7a52 100644 --- a/uds/database/data_record/abstract_data_record.py +++ b/uds/database/data_record/abstract_data_record.py @@ -1,6 +1,4 @@ -""" -Definition of AbstractDataRecord which is a base class for all Data Records. -""" +"""Definition of AbstractDataRecord which is a base class for all Data Records.""" __all__ = ["AbstractDataRecord", "DataRecordPhysicalValueAlias", "DecodedDataRecord"] diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py index 60615d5b..ac1851e2 100644 --- a/uds/database/data_record/raw_data_record.py +++ b/uds/database/data_record/raw_data_record.py @@ -1,6 +1,6 @@ -""" -Definition of RawDataRecord. -""" +"""Definition of RawDataRecord.""" + +__all__ = ["RawDataRecord"] from typing import Optional, Tuple @@ -8,11 +8,11 @@ class RawDataRecord(AbstractDataRecord): - """Common implementation and interface for Raw Data Record.""" + """Implementation and interface for Raw Data Record.""" def __init__(self, name: str, length: int) -> None: """ - Initialization of Raw Data Record. + Initialize Raw Data Record. :param name: Name to assign to this Data Record. :param length: Number of bits that this Raw Data Record is stored over. @@ -44,7 +44,7 @@ def length(self, value: int) -> None: raise ValueError("Length must be a positive integer.") self.__length = value - @property + @property # noqa: F841 def is_reoccurring(self) -> bool: """ Whether this Data Record might occur multiple times. @@ -55,7 +55,7 @@ def is_reoccurring(self) -> bool: """ return False - @property + @property # noqa: F841 def min_occurrences(self) -> int: """ Minimal number of this Data Record occurrences. @@ -65,7 +65,7 @@ def min_occurrences(self) -> int: """ return 1 - @property + @property # noqa: F841 def max_occurrences(self) -> Optional[int]: """ Maximal number of this Data Record occurrences. @@ -76,7 +76,7 @@ def max_occurrences(self) -> Optional[int]: """ return 1 - @property + @property # noqa: F841 def contains(self) -> Tuple[AbstractDataRecord, ...]: """Get Data Records contained by this Data Record.""" return () From 8f00c7e9672f5f79550bf3ad7fe36e6028db3635 Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Sat, 23 Nov 2024 16:25:52 +0100 Subject: [PATCH 6/9] addressing review comments --- .../data_record/test_abstract_data_record.py | 4 ---- uds/database/__init__.py | 2 -- uds/database/data_record/__init__.py | 2 -- .../data_record/abstract_data_record.py | 5 ----- uds/database/data_record/raw_data_record.py | 19 +++++++++++++++++++ 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tests/software_tests/database/data_record/test_abstract_data_record.py b/tests/software_tests/database/data_record/test_abstract_data_record.py index 46339e22..07c8a3ee 100644 --- a/tests/software_tests/database/data_record/test_abstract_data_record.py +++ b/tests/software_tests/database/data_record/test_abstract_data_record.py @@ -35,7 +35,3 @@ def test_init__valid(self, mock_isinstance): def test_name(self): self.mock_data_record._AbstractDataRecord__name = Mock() assert AbstractDataRecord.name.fget(self.mock_data_record) == self.mock_data_record._AbstractDataRecord__name - - def test_data_record_type(self): - self.mock_data_record.__class__.__name__ = "MockDataRecord" - assert AbstractDataRecord.data_record_type.fget(self.mock_data_record) == "MockDataRecord" diff --git a/uds/database/__init__.py b/uds/database/__init__.py index 020ccb3c..c5b21092 100644 --- a/uds/database/__init__.py +++ b/uds/database/__init__.py @@ -4,7 +4,5 @@ Tools for decoding and encoding information from/to diagnostic messages. """ -__all__ = ["AbstractDataRecord", "AbstractService", "DecodedDataRecord", "RawDataRecord"] - from .data_record import AbstractDataRecord, DecodedDataRecord, RawDataRecord from .services import AbstractService diff --git a/uds/database/data_record/__init__.py b/uds/database/data_record/__init__.py index 4ffa158f..3e5c39b5 100644 --- a/uds/database/data_record/__init__.py +++ b/uds/database/data_record/__init__.py @@ -5,7 +5,5 @@ meaningful information (e.g. physical value, text). """ -__all__ = ["AbstractDataRecord", "DecodedDataRecord", "RawDataRecord"] - from .abstract_data_record import AbstractDataRecord, DecodedDataRecord from .raw_data_record import RawDataRecord diff --git a/uds/database/data_record/abstract_data_record.py b/uds/database/data_record/abstract_data_record.py index 8a7a7a52..33d5a876 100644 --- a/uds/database/data_record/abstract_data_record.py +++ b/uds/database/data_record/abstract_data_record.py @@ -37,11 +37,6 @@ def name(self) -> str: """Name of this Data Record.""" return self.__name - @property # noqa: F841 - def data_record_type(self) -> str: - """Type of this Data Record.""" - return self.__class__.__name__ - @property # noqa: F841 @abstractmethod def length(self) -> int: diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py index ac1851e2..56a47f58 100644 --- a/uds/database/data_record/raw_data_record.py +++ b/uds/database/data_record/raw_data_record.py @@ -44,6 +44,15 @@ def length(self, value: int) -> None: raise ValueError("Length must be a positive integer.") self.__length = value + @property + def max_raw_value(self): + """ + Maximum raw (bit) value for this Data Record. + + :return: Maximum value that can be represented by `length` bits. + """ + return (1 << self.length) - 1 + @property # noqa: F841 def is_reoccurring(self) -> bool: """ @@ -88,7 +97,17 @@ def decode(self, raw_value: int) -> DecodedDataRecord: :param raw_value: Raw (bit) value of Data Record. :return: Dictionary with physical value for this Data Record. + + :raises TypeError: Provided `raw_value` is not int type. + :raises ValueError: Provided `raw_value` is out of range (0 <= raw_value <= max_raw_value). """ + if not isinstance(raw_value, int): + raise TypeError(f"Expected raw_value to be an int type, got '{type(raw_value).__name__}' instead.") + + if not (0 <= raw_value <= self.max_raw_value): + raise ValueError( + f"Provided value of raw_value is out of range: must be between 0 and {self.max_raw_value}, got {raw_value}." + ) return DecodedDataRecord(name=self.name, raw_value=raw_value, physical_value=raw_value) def encode(self, physical_value: DataRecordPhysicalValueAlias) -> int: From eb47239a32db5e5228b259586ace64213a27a36f Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Mon, 25 Nov 2024 20:40:25 +0100 Subject: [PATCH 7/9] unittests and modification of encode method in RawDataRecord --- .../data_record/test_raw_data_record.py | 161 +++++++++++++----- uds/database/data_record/raw_data_record.py | 7 + 2 files changed, 121 insertions(+), 47 deletions(-) diff --git a/tests/software_tests/database/data_record/test_raw_data_record.py b/tests/software_tests/database/data_record/test_raw_data_record.py index 9b4c8038..823e92a3 100644 --- a/tests/software_tests/database/data_record/test_raw_data_record.py +++ b/tests/software_tests/database/data_record/test_raw_data_record.py @@ -1,5 +1,5 @@ import pytest -from mock import patch +from mock import Mock, patch from uds.database.data_record.raw_data_record import RawDataRecord @@ -9,80 +9,147 @@ class TestRawDataRecord: def setup_method(self): - self.name = "TestRawDataRecord" - self.length = 8 - self.raw_data_record = RawDataRecord(self.name, self.length) + self.mock_data_record = Mock(spec=RawDataRecord) # __init__ - def test_init_valid(self): - assert self.raw_data_record.name == self.name - assert self.raw_data_record.length == self.length - - def test_init_type_error_for_name(self): - with pytest.raises(TypeError): - RawDataRecord(123, self.length) - - def test_init_value_error_for_length(self): - with pytest.raises(ValueError): - RawDataRecord(self.name, -5) - - def test_init_type_error_for_length(self): - with pytest.raises(TypeError): - RawDataRecord(self.name, "eight") + @pytest.mark.parametrize( + "name, length", [ + ("TestRawDataRecord", 8), + (Mock(), Mock()), + ] + ) + @patch(f"{SCRIPT_LOCATION}.AbstractDataRecord.__init__") + def test_init__valid(self, mock_abstract, name, length): + assert RawDataRecord.__init__(self.mock_data_record, name, length) is None + mock_abstract.assert_called_once_with(name) + assert self.mock_data_record.length == length # length def test_length_getter(self): - assert self.raw_data_record.length == self.length - - def test_length_setter_valid(self): - self.raw_data_record.length = 16 - assert self.raw_data_record.length == 16 - - def test_length_setter_type_error(self): + self.mock_data_record._RawDataRecord__length = Mock() + assert RawDataRecord.length.fget(self.mock_data_record) == self.mock_data_record._RawDataRecord__length + + @pytest.mark.parametrize( + "value", [1, 8] + ) + def test_length_setter_valid(self, value): + RawDataRecord.length.fset(self.mock_data_record, value) + assert self.mock_data_record._RawDataRecord__length == value + + @pytest.mark.parametrize( + "value", ["test", None] + ) + @patch(f"{SCRIPT_LOCATION}.isinstance") + def test_length_setter_type_error(self, mock_isinstance, value): + mock_isinstance.return_value = False with pytest.raises(TypeError): - self.raw_data_record.length = "sixteen" + RawDataRecord.length.fset(self.mock_data_record, value) + mock_isinstance.assert_called_once_with(value, int) - def test_length_setter_value_error(self): + @pytest.mark.parametrize( + "value", [0, -1] + ) + def test_length_setter_value_error(self, value): with pytest.raises(ValueError): - self.raw_data_record.length = -10 + RawDataRecord.length.fset(self.mock_data_record, value) + + # max_raw_value + + @pytest.mark.parametrize( + "length, value", [ + (2, 3), + (5, 31), + (8, 255), + ] + ) + def test_max_raw_value_getter(self, length, value): + raw_data_record = RawDataRecord("TestRawDataRecord", length) + assert raw_data_record.max_raw_value == value # is_reoccurring - def test_is_reoccurring(self): - assert self.raw_data_record.is_reoccurring is False + def test_is_reoccurring_getter(self): + assert RawDataRecord.is_reoccurring.fget(self.mock_data_record) is False # min_occurrences - def test_min_occurrences(self): - assert self.raw_data_record.min_occurrences == 1 + def test_min_occurrences_getter(self): + assert RawDataRecord.min_occurrences.fget(self.mock_data_record) == 1 # max_occurrences - def test_max_occurrences(self): - assert self.raw_data_record.max_occurrences == 1 + def test_max_occurrences_getter(self): + assert RawDataRecord.max_occurrences.fget(self.mock_data_record) == 1 # contains - def test_contains(self): - assert self.raw_data_record.contains == () + def test_contains_getter(self): + assert RawDataRecord.contains.fget(self.mock_data_record) == () # decode + @pytest.mark.parametrize( + "value", [1, 4] + ) @patch(f"{SCRIPT_LOCATION}.DecodedDataRecord") - def test_decode(self, mock_decoded_data_record): - raw_value = 42 - result = self.raw_data_record.decode(raw_value) + def test_decode(self, mock_decoded_data_record, value): + self.mock_data_record.max_raw_value = 8 + assert RawDataRecord.decode(self.mock_data_record, value) == mock_decoded_data_record.return_value mock_decoded_data_record.assert_called_once_with( - name=self.name, - raw_value=raw_value, - physical_value=raw_value + name=self.mock_data_record.name, + raw_value=value, + physical_value=value ) - assert result == mock_decoded_data_record.return_value + + @pytest.mark.parametrize( + "value", ["test", None] + ) + @patch(f"{SCRIPT_LOCATION}.isinstance") + def test_decode_type_error(self, mock_isinstance, value): + with pytest.raises(TypeError): + RawDataRecord.decode(self.mock_data_record, value) + mock_isinstance.assert_called_once_with(value, int) + + @pytest.mark.parametrize( + "value, max_raw_value", [ + (-1, 2), + (3, 2), + (16, 6), + ] + ) + def test_decode_value_error(self, value, max_raw_value): + self.mock_data_record.max_raw_value = max_raw_value + with pytest.raises(ValueError): + RawDataRecord.decode(self.mock_data_record, value) # encode - def test_encode(self): - physical_value = 42 - assert self.raw_data_record.encode(physical_value) == physical_value + @pytest.mark.parametrize( + "value", [1, 4] + ) + def test_encode(self, value): + self.mock_data_record.max_raw_value = 8 + assert RawDataRecord.encode(self.mock_data_record, value) == value + + @pytest.mark.parametrize( + "value", ["test", None] + ) + @patch(f"{SCRIPT_LOCATION}.isinstance") + def test_encode_type_error(self, mock_isinstance, value): + with pytest.raises(TypeError): + RawDataRecord.encode(self.mock_data_record, value) + mock_isinstance.assert_called_once_with(value, int) + + @pytest.mark.parametrize( + "value, max_raw_value", [ + (-1, 2), + (3, 2), + (16, 6), + ] + ) + def test_encode_value_error(self, value, max_raw_value): + self.mock_data_record.max_raw_value = max_raw_value + with pytest.raises(ValueError): + RawDataRecord.encode(self.mock_data_record, value) diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py index 56a47f58..ccfb27a4 100644 --- a/uds/database/data_record/raw_data_record.py +++ b/uds/database/data_record/raw_data_record.py @@ -118,4 +118,11 @@ def encode(self, physical_value: DataRecordPhysicalValueAlias) -> int: :return: Raw Value of this Data Record. """ + if not isinstance(physical_value, int): + raise TypeError(f"Expected physical_value to be an int type, got '{type(physical_value).__name__}' instead.") + + if not (0 <= physical_value <= self.max_raw_value): + raise ValueError( + f"Provided value of physical_value is out of range: must be between 0 and {self.max_raw_value}, got {physical_value}." + ) return physical_value # type: ignore From d71291e27ece586d341d0e35bd833e9c484a7edb Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Mon, 25 Nov 2024 21:01:01 +0100 Subject: [PATCH 8/9] pylint fixes --- uds/database/data_record/raw_data_record.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py index ccfb27a4..462dd147 100644 --- a/uds/database/data_record/raw_data_record.py +++ b/uds/database/data_record/raw_data_record.py @@ -104,9 +104,10 @@ def decode(self, raw_value: int) -> DecodedDataRecord: if not isinstance(raw_value, int): raise TypeError(f"Expected raw_value to be an int type, got '{type(raw_value).__name__}' instead.") - if not (0 <= raw_value <= self.max_raw_value): + if not 0 <= raw_value <= self.max_raw_value: raise ValueError( - f"Provided value of raw_value is out of range: must be between 0 and {self.max_raw_value}, got {raw_value}." + "Provided value of raw_value is out of range: " + f"must be between 0 and {self.max_raw_value}, got {raw_value}." ) return DecodedDataRecord(name=self.name, raw_value=raw_value, physical_value=raw_value) @@ -119,10 +120,13 @@ def encode(self, physical_value: DataRecordPhysicalValueAlias) -> int: :return: Raw Value of this Data Record. """ if not isinstance(physical_value, int): - raise TypeError(f"Expected physical_value to be an int type, got '{type(physical_value).__name__}' instead.") + raise TypeError( + f"Expected physical_value to be an int type, got '{type(physical_value).__name__}' instead." + ) - if not (0 <= physical_value <= self.max_raw_value): + if not 0 <= physical_value <= self.max_raw_value: raise ValueError( - f"Provided value of physical_value is out of range: must be between 0 and {self.max_raw_value}, got {physical_value}." + "Provided value of physical_value is out of range: " + f"must be between 0 and {self.max_raw_value}, got {physical_value}." ) return physical_value # type: ignore From 91aa3d1b22f9dd80e4ca27183ded60930b86d5a0 Mon Sep 17 00:00:00 2001 From: Przemyslaw Niescior Date: Wed, 27 Nov 2024 21:46:29 +0100 Subject: [PATCH 9/9] addressing comments --- .../data_record/test_abstract_data_record.py | 13 +++++++++++++ .../database/data_record/test_raw_data_record.py | 12 +++++++++--- uds/database/data_record/abstract_data_record.py | 9 +++++++++ uds/database/data_record/raw_data_record.py | 9 --------- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/tests/software_tests/database/data_record/test_abstract_data_record.py b/tests/software_tests/database/data_record/test_abstract_data_record.py index 07c8a3ee..230b0251 100644 --- a/tests/software_tests/database/data_record/test_abstract_data_record.py +++ b/tests/software_tests/database/data_record/test_abstract_data_record.py @@ -35,3 +35,16 @@ def test_init__valid(self, mock_isinstance): def test_name(self): self.mock_data_record._AbstractDataRecord__name = Mock() assert AbstractDataRecord.name.fget(self.mock_data_record) == self.mock_data_record._AbstractDataRecord__name + + # max_raw_value + + @pytest.mark.parametrize( + "length, value", [ + (2, 3), + (5, 31), + (8, 255), + ] + ) + def test_max_raw_value_getter(self, length, value): + self.mock_data_record.length = length + assert AbstractDataRecord.max_raw_value.fget(self.mock_data_record) == value diff --git a/tests/software_tests/database/data_record/test_raw_data_record.py b/tests/software_tests/database/data_record/test_raw_data_record.py index 823e92a3..d5222b3a 100644 --- a/tests/software_tests/database/data_record/test_raw_data_record.py +++ b/tests/software_tests/database/data_record/test_raw_data_record.py @@ -108,6 +108,7 @@ def test_decode(self, mock_decoded_data_record, value): ) @patch(f"{SCRIPT_LOCATION}.isinstance") def test_decode_type_error(self, mock_isinstance, value): + mock_isinstance.return_value = False with pytest.raises(TypeError): RawDataRecord.decode(self.mock_data_record, value) mock_isinstance.assert_called_once_with(value, int) @@ -127,10 +128,14 @@ def test_decode_value_error(self, value, max_raw_value): # encode @pytest.mark.parametrize( - "value", [1, 4] + "value, max_raw_value", [ + (0, 2), + (3, 3), + (16, 16), + ] ) - def test_encode(self, value): - self.mock_data_record.max_raw_value = 8 + def test_encode(self, value, max_raw_value): + self.mock_data_record.max_raw_value = max_raw_value assert RawDataRecord.encode(self.mock_data_record, value) == value @pytest.mark.parametrize( @@ -138,6 +143,7 @@ def test_encode(self, value): ) @patch(f"{SCRIPT_LOCATION}.isinstance") def test_encode_type_error(self, mock_isinstance, value): + mock_isinstance.return_value = False with pytest.raises(TypeError): RawDataRecord.encode(self.mock_data_record, value) mock_isinstance.assert_called_once_with(value, int) diff --git a/uds/database/data_record/abstract_data_record.py b/uds/database/data_record/abstract_data_record.py index 33d5a876..fe4afd0a 100644 --- a/uds/database/data_record/abstract_data_record.py +++ b/uds/database/data_record/abstract_data_record.py @@ -42,6 +42,15 @@ def name(self) -> str: def length(self) -> int: """Get number of bits that this Data Record is stored over.""" + @property + def max_raw_value(self): + """ + Maximum raw (bit) value for this Data Record. + + :return: Maximum value that can be represented by `length` bits. + """ + return (1 << self.length) - 1 + @property # noqa: F841 @abstractmethod def is_reoccurring(self) -> bool: diff --git a/uds/database/data_record/raw_data_record.py b/uds/database/data_record/raw_data_record.py index 462dd147..89de1d1c 100644 --- a/uds/database/data_record/raw_data_record.py +++ b/uds/database/data_record/raw_data_record.py @@ -44,15 +44,6 @@ def length(self, value: int) -> None: raise ValueError("Length must be a positive integer.") self.__length = value - @property - def max_raw_value(self): - """ - Maximum raw (bit) value for this Data Record. - - :return: Maximum value that can be represented by `length` bits. - """ - return (1 << self.length) - 1 - @property # noqa: F841 def is_reoccurring(self) -> bool: """