From 2133067bf03691cefc6b9cc41a90218424edd70c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frieder=20Sch=C3=BCler?= Date: Wed, 29 Apr 2026 13:59:25 +0200 Subject: [PATCH 1/3] Fix encode_phys precision loss for int64 with factor=1 Skip the float division in encode_phys when factor is 1 (the default). Previously, `value /= self.factor` always performed float division, causing up to 10 bits of precision loss for large int64 values (e.g. 0x55554444AAAABBBB became 0x55554444AAAABC00). Use `!= 1` for the comparison since we are comparing values, not identity. Also clean up the redundant int(round(...)) to just round(), since round() already returns int when ndigits is omitted. Add unit tests covering: - int64 roundtrip with factor=1 (the original bug) - int type preservation with factor=1 - rounding with integer factor != 1 (existing behavior preserved) - float factor (existing behavior preserved) Based on the discussion in #611. --- canopen/objectdictionary/__init__.py | 4 ++-- test/test_od.py | 29 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/canopen/objectdictionary/__init__.py b/canopen/objectdictionary/__init__.py index fa694c56..a44a963e 100644 --- a/canopen/objectdictionary/__init__.py +++ b/canopen/objectdictionary/__init__.py @@ -484,8 +484,8 @@ def decode_phys(self, value: int) -> Union[int, bool, float, str, bytes]: def encode_phys(self, value: Union[int, bool, float, str, bytes]) -> int: if self.data_type in INTEGER_TYPES: - value /= self.factor - value = int(round(value)) + if self.factor != 1: + value = round(value / self.factor) return value def decode_desc(self, value: int) -> str: diff --git a/test/test_od.py b/test/test_od.py index 9ab0e187..28d7637b 100644 --- a/test/test_od.py +++ b/test/test_od.py @@ -182,6 +182,35 @@ def test_phys(self): self.assertAlmostEqual(var.decode_phys(128), 12.8) self.assertEqual(var.encode_phys(-0.1), -1) + def test_phys_factor_1_int64_roundtrip(self): + """int64 values must survive encode_phys when factor is 1.""" + var = od.ODVariable("Test UNSIGNED64", 0x1000) + var.data_type = od.UNSIGNED64 + value = 0x55554444AAAABBBB + self.assertEqual(var.encode_phys(value), value) + + def test_phys_factor_1_preserves_int(self): + """encode_phys with factor=1 must not convert int to float.""" + var = od.ODVariable("Test INTEGER32", 0x1000) + var.data_type = od.INTEGER32 + self.assertIsInstance(var.encode_phys(42), int) + + def test_phys_factor_1000_rounds(self): + """Existing rounding behaviour with integer factor != 1 is preserved.""" + var = od.ODVariable("Test INTEGER32", 0x1000) + var.data_type = od.INTEGER32 + var.factor = 1000 + # 5555 / 1000 = 5.555 → round → 6 + self.assertEqual(var.encode_phys(5555), 6) + + def test_phys_float_factor(self): + """Float factor still works via float division + round.""" + var = od.ODVariable("Test INTEGER16", 0x1000) + var.data_type = od.INTEGER16 + var.factor = 0.5 + # 10 / 0.5 = 20 + self.assertEqual(var.encode_phys(10), 20) + def test_desc(self): var = od.ODVariable("Test UNSIGNED8", 0x1000) var.data_type = od.UNSIGNED8 From 43ae8926e48528ab3b44a66bb339d7a717514dd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Wed, 6 May 2026 11:21:59 +0200 Subject: [PATCH 2/3] Reword docstrings to state facts, not changes. --- test/test_od.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_od.py b/test/test_od.py index 28d7637b..0b63a243 100644 --- a/test/test_od.py +++ b/test/test_od.py @@ -196,7 +196,7 @@ def test_phys_factor_1_preserves_int(self): self.assertIsInstance(var.encode_phys(42), int) def test_phys_factor_1000_rounds(self): - """Existing rounding behaviour with integer factor != 1 is preserved.""" + """Integer factor > 1 uses float rounding behaviour, not truncating division.""" var = od.ODVariable("Test INTEGER32", 0x1000) var.data_type = od.INTEGER32 var.factor = 1000 @@ -204,7 +204,7 @@ def test_phys_factor_1000_rounds(self): self.assertEqual(var.encode_phys(5555), 6) def test_phys_float_factor(self): - """Float factor still works via float division + round.""" + """Float factor uses float division + round.""" var = od.ODVariable("Test INTEGER16", 0x1000) var.data_type = od.INTEGER16 var.factor = 0.5 From 1eba930eb9ba9b49d4aa5bfc8bf6b5b699a9aaf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Wed, 6 May 2026 11:26:39 +0200 Subject: [PATCH 3/3] Add test for float 1.0 factor promoting to float type. --- test/test_od.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/test_od.py b/test/test_od.py index 0b63a243..cca8a234 100644 --- a/test/test_od.py +++ b/test/test_od.py @@ -211,6 +211,13 @@ def test_phys_float_factor(self): # 10 / 0.5 = 20 self.assertEqual(var.encode_phys(10), 20) + def test_phys_float_factor_decodes_to_float(self): + """decode_phys with float factor ensures a float result.""" + var = od.ODVariable("Test INTEGER32", 0x1000) + var.data_type = od.INTEGER32 + var.factor = 1.0 + self.assertIsInstance(var.decode_phys(42), float) + def test_desc(self): var = od.ODVariable("Test UNSIGNED8", 0x1000) var.data_type = od.UNSIGNED8