Repository: christiansandberg/canopen (language: python)

Issue:
eds LowLimit/HighLimit HEX signed/unsigned (CONTINUE)
In my understanding, 0xFF should be -1(int8), and 0xFFFFFFFF => -1(int32), too.

Would you double check the check here? https://github.com/christiansandberg/canopen/blob/master/test/test_eds.py#L60 

If my understanding is correct, maybe we can modify the function: https://github.com/christiansandberg/canopen/blob/master/canopen/objectdictionary/eds.py#L214

A related issue here: https://github.com/christiansandberg/canopen/pull/345




Discussion / hints:

Fix patch:
diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py
index 187b5135..16ed22c1 100644
--- a/canopen/objectdictionary/eds.py
+++ b/canopen/objectdictionary/eds.py
@@ -211,9 +211,10 @@ def _calc_bit_length(data_type):
 
 def _signed_int_from_hex(hex_str, bit_length):
     number = int(hex_str, 0)
-    limit = ((1 << bit_length - 1) - 1)
-    if number > limit:
-        return limit - number
+    max_value = (1 << (bit_length - 1)) - 1
+    
+    if number > max_value:
+        return number - (1 << bit_length)
     else:
         return number
 


Test patch:
diff --git a/test/sample.eds b/test/sample.eds
index 2267ff55..b88ff75b 100644
--- a/test/sample.eds
+++ b/test/sample.eds
@@ -926,8 +926,8 @@ ParameterName=INTEGER32 only negative values
 ObjectType=0x7
 DataType=0x04
 AccessType=rw
-HighLimit=0x00000000
-LowLimit=0xFFFFFFFF
+HighLimit=0xFFFFFFFF
+LowLimit=0x80000000
 PDOMapping=0
 
 [3040]
@@ -936,10 +936,9 @@ ObjectType=0x7
 DataType=0x15
 AccessType=rw
 HighLimit=0x000000000000000A
-LowLimit=0x8000000000000009
+LowLimit=0xFFFFFFFFFFFFFFF6
 PDOMapping=0
 
-
 [3050]
 ParameterName=EDS file extensions
 SubNumber=0x7
diff --git a/test/test_eds.py b/test/test_eds.py
index 17674d45..50629234 100644
--- a/test/test_eds.py
+++ b/test/test_eds.py
@@ -1,12 +1,44 @@
 import os
 import unittest
 import canopen
+from canopen.objectdictionary.eds import _signed_int_from_hex
 
 EDS_PATH = os.path.join(os.path.dirname(__file__), 'sample.eds')
 
 
 class TestEDS(unittest.TestCase):
 
+    test_data = {
+        "int8": [
+            {"hex_str": "7F", "bit_length": 8, "expected": 127},
+            {"hex_str": "80", "bit_length": 8, "expected": -128},
+            {"hex_str": "FF", "bit_length": 8, "expected": -1},
+            {"hex_str": "00", "bit_length": 8, "expected": 0},
+            {"hex_str": "01", "bit_length": 8, "expected": 1}
+        ],
+        "int16": [
+            {"hex_str": "7FFF", "bit_length": 16, "expected": 32767},
+            {"hex_str": "8000", "bit_length": 16, "expected": -32768},
+            {"hex_str": "FFFF", "bit_length": 16, "expected": -1},
+            {"hex_str": "0000", "bit_length": 16, "expected": 0},
+            {"hex_str": "0001", "bit_length": 16, "expected": 1}
+        ],
+        "int32": [
+            {"hex_str": "7FFFFFFF", "bit_length": 32, "expected": 2147483647},
+            {"hex_str": "80000000", "bit_length": 32, "expected": -2147483648},
+            {"hex_str": "FFFFFFFF", "bit_length": 32, "expected": -1},
+            {"hex_str": "00000000", "bit_length": 32, "expected": 0},
+            {"hex_str": "00000001", "bit_length": 32, "expected": 1}
+        ],
+        "int64": [
+            {"hex_str": "7FFFFFFFFFFFFFFF", "bit_length": 64, "expected": 9223372036854775807},
+            {"hex_str": "8000000000000000", "bit_length": 64, "expected": -9223372036854775808},
+            {"hex_str": "FFFFFFFFFFFFFFFF", "bit_length": 64, "expected": -1},
+            {"hex_str": "0000000000000000", "bit_length": 64, "expected": 0},
+            {"hex_str": "0000000000000001", "bit_length": 64, "expected": 1}
+        ]
+    }
+
     def setUp(self):
         self.od = canopen.import_od(EDS_PATH, 2)
 
@@ -58,11 +90,18 @@ def test_record_with_limits(self):
         self.assertEqual(uint8.max, 10)
         int32 = self.od[0x3030]
         self.assertEqual(int32.min, -2147483648)
-        self.assertEqual(int32.max, 0)
+        self.assertEqual(int32.max, -1)
         int64 = self.od[0x3040]
         self.assertEqual(int64.min, -10)
         self.assertEqual(int64.max, +10)
 
+    def test_signed_int_from_hex(self):
+        for data_type, test_cases in self.test_data.items():
+            for test_case in test_cases:
+                with self.subTest(data_type=data_type, test_case=test_case):
+                    result = _signed_int_from_hex('0x' + test_case["hex_str"], test_case["bit_length"])
+                    self.assertEqual(result, test_case["expected"])
+
     def test_array_compact_subobj(self):
         array = self.od[0x1003]
         self.assertIsInstance(array, canopen.objectdictionary.Array)