consider review comments

thanks goes to [at]kayoub5. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Florian Jost <[email protected]>
mercedes-benz · Feb 15, 2024 · 6b49fbb · 6b49fbb
1 parent c7a2d57
commit 6b49fbb
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 28 deletions.
diff --git a/odxtools/basicstructure.py b/odxtools/basicstructure.py
@@ -262,12 +262,11 @@ def decode(self, message: bytes) -> ParameterValueDict:
         param_values = self.decode_from_pdu(decode_state)
 
         if len(message) != decode_state.cursor_byte_position:
-            warnings.warn(
+            odxraise(
                 f"The message {message.hex()} probably could not be completely parsed:"
                 f" Expected length of {decode_state.cursor_byte_position} but got {len(message)}.",
-                DecodeError,
-                stacklevel=1,
-            )
+                DecodeError)
+            return {}
 
         if not isinstance(param_values, dict):
             odxraise("Decoding structures must result in a dictionary")

diff --git a/odxtools/multiplexer.py b/odxtools/multiplexer.py
@@ -11,7 +11,7 @@
 from .multiplexerdefaultcase import MultiplexerDefaultCase
 from .multiplexerswitchkey import MultiplexerSwitchKey
 from .odxlink import OdxDocFragment, OdxLinkDatabase, OdxLinkId
-from .odxtypes import ParameterValue, odxstr_to_bool
+from .odxtypes import AtomicOdxType, ParameterValue, odxstr_to_bool
 from .utils import dataclass_fields_asdict
 
 if TYPE_CHECKING:
@@ -64,14 +64,13 @@ def from_et(et_element: ElementTree.Element, doc_frags: List[OdxDocFragment]) ->
     def is_visible(self) -> bool:
         return self.is_visible_raw is True
 
-    def _get_case_limits(self, case: MultiplexerCase) -> Tuple[int, int]:
+    def _get_case_limits(self, case: MultiplexerCase) -> Tuple[AtomicOdxType, AtomicOdxType]:
         key_type = self.switch_key.dop.physical_type.base_data_type
         lower_limit = key_type.make_from(case.lower_limit)
         upper_limit = key_type.make_from(case.upper_limit)
-        if not isinstance(lower_limit, int):
-            odxraise("Bounds of limits must be integers")
-        if not isinstance(upper_limit, int):
-            odxraise("Bounds of limits must be integers")
+        if not isinstance(lower_limit, type(upper_limit)) and not isinstance(
+                upper_limit, type(lower_limit)):
+            odxraise("Upper and lower bounds of limits must compareable")
         return lower_limit, upper_limit
 
     def convert_physical_to_bytes(self, physical_value: ParameterValue, encode_state: EncodeState,
@@ -88,15 +87,15 @@ def convert_physical_to_bytes(self, physical_value: ParameterValue, encode_state
         case_name, case_value = next(iter(physical_value.items()))
         case_pos = self.byte_position
 
-        for case in self.cases or []:
-            if case.short_name == case_name:
-                if case._structure:
-                    case_bytes = case._structure.convert_physical_to_bytes(
+        for mux_case in self.cases or []:
+            if mux_case.short_name == case_name:
+                if mux_case._structure:
+                    case_bytes = mux_case._structure.convert_physical_to_bytes(
                         case_value, encode_state, 0)
                 else:
                     case_bytes = b''
 
-                key_value, _ = self._get_case_limits(case)
+                key_value, _ = self._get_case_limits(mux_case)
                 key_bytes = self.switch_key.dop.convert_physical_to_bytes(
                     key_value, encode_state, bit_position=self.switch_key.bit_position or 0)
 
@@ -128,7 +127,7 @@ def decode_from_pdu(self, decode_state: DecodeState) -> ParameterValue:
         case_value: Optional[ParameterValue] = None
         for case in self.cases or []:
             lower, upper = self._get_case_limits(case)
-            if lower <= key_value and key_value <= upper:
+            if lower <= key_value and key_value <= upper:  # type: ignore[operator]
                 if case._structure:
                     case_value = case._structure.decode_from_pdu(decode_state)
                 break

diff --git a/odxtools/parameters/matchingrequestparameter.py b/odxtools/parameters/matchingrequestparameter.py
@@ -3,10 +3,9 @@
 from typing import Optional
 
 from ..decodestate import DecodeState
-from ..diagcodedtype import DiagCodedType
 from ..encodestate import EncodeState
 from ..exceptions import EncodeError
-from ..odxtypes import DataType, ParameterValue
+from ..odxtypes import ParameterValue
 from .parameter import Parameter, ParameterType
 
 
@@ -43,13 +42,11 @@ def decode_from_pdu(self, decode_state: DecodeState) -> ParameterValue:
         if self.byte_position is not None:
             decode_state.cursor_byte_position = decode_state.origin_byte_position + self.byte_position
 
-        result, decode_state.cursor_byte_position = DiagCodedType._extract_internal_value(
-            coded_message=decode_state.coded_message,
-            byte_position=decode_state.cursor_byte_position,
-            bit_position=self.bit_position or 0,
-            bit_length=self.byte_length * 8,
-            base_data_type=DataType.A_UINT32,
-            is_highlow_byte_order=False)
+        byte_position = decode_state.cursor_byte_position
+        bit_position = self.bit_position or 0
+        byte_length = (8 * self.byte_length + bit_position + 7) // 8
+        result = decode_state.coded_message[byte_position:byte_position + byte_length]
+        decode_state.cursor_byte_position += byte_length
 
         decode_state.cursor_byte_position = max(decode_state.cursor_byte_position, orig_cursor)
         decode_state.cursor_bit_position = 0

diff --git a/tests/test_decoding.py b/tests/test_decoding.py
@@ -1242,7 +1242,7 @@ def test_decode_response(self) -> None:
                 coding_object=message,
                 param_dict={
                     "SID": sid,
-                    "matching_req_param": 0xAB
+                    "matching_req_param": bytes([0xAB])
                 },
             )
             decoded_message = diag_layer.decode(coded_message)[0]

diff --git a/tests/test_somersault.py b/tests/test_somersault.py
@@ -259,7 +259,7 @@ def test_code_table_params(self) -> None:
         self.assertEqual(
             decoded_resp_data["last_pos_response"][1]  # type: ignore[index, call-overload]
             ["num_flips_done"],  # type: ignore[index, call-overload]
-            123)
+            bytes([123]))
         self.assertEqual(
             decoded_resp_data["last_pos_response"][1]  # type: ignore[index, call-overload]
             ["sault_time"],  # type: ignore[index, call-overload]
@@ -352,7 +352,11 @@ def test_decode_response(self) -> None:
         m = messages[0]
         self.assertEqual(m.coded_message.hex(), "fa03ff")
         self.assertEqual(m.coding_object, pos_response)
-        self.assertEqual(m.param_dict, {"sid": 0xFA, "num_flips_done": 0x03, "sault_time": 255})
+        self.assertEqual(m.param_dict, {
+            "sid": 0xFA,
+            "num_flips_done": bytearray([0x03]),
+            "sault_time": 255
+        })
 
 
 class TestNavigation(unittest.TestCase):