Skip to content

Commit 2133067

Browse files
committed
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.
1 parent 4e789fe commit 2133067

2 files changed

Lines changed: 31 additions & 2 deletions

File tree

canopen/objectdictionary/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,8 +484,8 @@ def decode_phys(self, value: int) -> Union[int, bool, float, str, bytes]:
484484

485485
def encode_phys(self, value: Union[int, bool, float, str, bytes]) -> int:
486486
if self.data_type in INTEGER_TYPES:
487-
value /= self.factor
488-
value = int(round(value))
487+
if self.factor != 1:
488+
value = round(value / self.factor)
489489
return value
490490

491491
def decode_desc(self, value: int) -> str:

test/test_od.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,35 @@ def test_phys(self):
182182
self.assertAlmostEqual(var.decode_phys(128), 12.8)
183183
self.assertEqual(var.encode_phys(-0.1), -1)
184184

185+
def test_phys_factor_1_int64_roundtrip(self):
186+
"""int64 values must survive encode_phys when factor is 1."""
187+
var = od.ODVariable("Test UNSIGNED64", 0x1000)
188+
var.data_type = od.UNSIGNED64
189+
value = 0x55554444AAAABBBB
190+
self.assertEqual(var.encode_phys(value), value)
191+
192+
def test_phys_factor_1_preserves_int(self):
193+
"""encode_phys with factor=1 must not convert int to float."""
194+
var = od.ODVariable("Test INTEGER32", 0x1000)
195+
var.data_type = od.INTEGER32
196+
self.assertIsInstance(var.encode_phys(42), int)
197+
198+
def test_phys_factor_1000_rounds(self):
199+
"""Existing rounding behaviour with integer factor != 1 is preserved."""
200+
var = od.ODVariable("Test INTEGER32", 0x1000)
201+
var.data_type = od.INTEGER32
202+
var.factor = 1000
203+
# 5555 / 1000 = 5.555 → round → 6
204+
self.assertEqual(var.encode_phys(5555), 6)
205+
206+
def test_phys_float_factor(self):
207+
"""Float factor still works via float division + round."""
208+
var = od.ODVariable("Test INTEGER16", 0x1000)
209+
var.data_type = od.INTEGER16
210+
var.factor = 0.5
211+
# 10 / 0.5 = 20
212+
self.assertEqual(var.encode_phys(10), 20)
213+
185214
def test_desc(self):
186215
var = od.ODVariable("Test UNSIGNED8", 0x1000)
187216
var.data_type = od.UNSIGNED8

0 commit comments

Comments
 (0)