Skip to content

Commit bf972c1

Browse files
eds: fix LowLimit/HighLimit parsing for all signed integer types
- Replace _calc_bit_length() (only handled INTEGER8/16/32/64) with a lookup dict _SIGNED_BIT_LENGTHS covering all 8 SIGNED_TYPES (INTEGER8/16/24/32/40/48/56/64) - Log a warning instead of silently ignoring invalid limit values (except ValueError: pass) - Add EDS test entries in sample.eds for INTEGER24/40/48/56 with hex-encoded negative limits - Extend test_record_with_limits and add test_invalid_limit_logs_warning Closes part of #352.
1 parent a4e61d7 commit bf972c1

3 files changed

Lines changed: 72 additions & 17 deletions

File tree

canopen/objectdictionary/eds.py

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -204,19 +204,16 @@ def import_from_node(node_id: int, network: canopen.network.Network):
204204
return od
205205

206206

207-
def _calc_bit_length(data_type):
208-
if data_type == datatypes.INTEGER8:
209-
return 8
210-
elif data_type == datatypes.INTEGER16:
211-
return 16
212-
elif data_type == datatypes.INTEGER32:
213-
return 32
214-
elif data_type == datatypes.INTEGER64:
215-
return 64
216-
else:
217-
raise ValueError(
218-
f"Invalid data_type '{data_type}', expecting a signed integer data_type."
219-
)
207+
_SIGNED_BIT_LENGTHS = {
208+
datatypes.INTEGER8: 8,
209+
datatypes.INTEGER16: 16,
210+
datatypes.INTEGER24: 24,
211+
datatypes.INTEGER32: 32,
212+
datatypes.INTEGER40: 40,
213+
datatypes.INTEGER48: 48,
214+
datatypes.INTEGER56: 56,
215+
datatypes.INTEGER64: 64,
216+
}
220217

221218

222219
def _signed_int_from_hex(hex_str, bit_length):
@@ -305,20 +302,20 @@ def build_variable(
305302
try:
306303
min_string = eds.get(section, "LowLimit")
307304
if var.data_type in datatypes.SIGNED_TYPES:
308-
var.min = _signed_int_from_hex(min_string, _calc_bit_length(var.data_type))
305+
var.min = _signed_int_from_hex(min_string, _SIGNED_BIT_LENGTHS[var.data_type])
309306
else:
310307
var.min = int(min_string, 0)
311308
except ValueError:
312-
pass
309+
logger.warning("Failed to parse LowLimit for %s: %r", name, min_string)
313310
if eds.has_option(section, "HighLimit"):
314311
try:
315312
max_string = eds.get(section, "HighLimit")
316313
if var.data_type in datatypes.SIGNED_TYPES:
317-
var.max = _signed_int_from_hex(max_string, _calc_bit_length(var.data_type))
314+
var.max = _signed_int_from_hex(max_string, _SIGNED_BIT_LENGTHS[var.data_type])
318315
else:
319316
var.max = int(max_string, 0)
320317
except ValueError:
321-
pass
318+
logger.warning("Failed to parse HighLimit for %s: %r", name, max_string)
322319
if eds.has_option(section, "DefaultValue"):
323320
try:
324321
var.default_raw = eds.get(section, "DefaultValue")

test/sample.eds

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,42 @@ HighLimit=0xFFFFFFFF
976976
LowLimit=0x80000000
977977
PDOMapping=0
978978

979+
[3031]
980+
ParameterName=INTEGER24 value range -1 to 0
981+
ObjectType=0x7
982+
DataType=0x10
983+
AccessType=rw
984+
HighLimit=0x000000
985+
LowLimit=0xFFFFFF
986+
PDOMapping=0
987+
988+
[3032]
989+
ParameterName=INTEGER40 value range -1 to 0
990+
ObjectType=0x7
991+
DataType=0x12
992+
AccessType=rw
993+
HighLimit=0x0000000000
994+
LowLimit=0xFFFFFFFFFF
995+
PDOMapping=0
996+
997+
[3033]
998+
ParameterName=INTEGER48 value range -1 to 0
999+
ObjectType=0x7
1000+
DataType=0x13
1001+
AccessType=rw
1002+
HighLimit=0x000000000000
1003+
LowLimit=0xFFFFFFFFFFFF
1004+
PDOMapping=0
1005+
1006+
[3034]
1007+
ParameterName=INTEGER56 value range -1 to 0
1008+
ObjectType=0x7
1009+
DataType=0x14
1010+
AccessType=rw
1011+
HighLimit=0x00000000000000
1012+
LowLimit=0xFFFFFFFFFFFFFF
1013+
PDOMapping=0
1014+
9791015
[3040]
9801016
ParameterName=INTEGER64 value range -10 to +10
9811017
ObjectType=0x7

test/test_eds.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,28 @@ def test_record_with_limits(self):
148148
int64 = self.od[0x3040]
149149
self.assertEqual(int64.min, -10)
150150
self.assertEqual(int64.max, +10)
151+
# Verify all remaining SIGNED_TYPES are handled (INTEGER24/40/48/56)
152+
for index in (0x3031, 0x3032, 0x3033, 0x3034):
153+
var = self.od[index]
154+
self.assertEqual(var.min, -1, f"min mismatch at 0x{index:04X}")
155+
self.assertEqual(var.max, 0, f"max mismatch at 0x{index:04X}")
156+
157+
def test_invalid_limit_logs_warning(self):
158+
import io
159+
import logging
160+
161+
with open(SAMPLE_EDS) as f:
162+
content = f.read()
163+
invalid_eds = content.replace(
164+
"LowLimit=0x02\nPDOMapping=0\n\n[3030]",
165+
"LowLimit=INVALID\nPDOMapping=0\n\n[3030]",
166+
)
167+
with io.StringIO(invalid_eds) as buf:
168+
buf.name = "mock.eds"
169+
with self.assertLogs("canopen.objectdictionary.eds", level=logging.WARNING) as cm:
170+
od = canopen.import_od(buf)
171+
self.assertIsNone(od[0x3021].min)
172+
self.assertTrue(any("LowLimit" in msg for msg in cm.output))
151173

152174
def test_signed_int_from_hex(self):
153175
for data_type, test_cases in self.test_data.items():

0 commit comments

Comments
 (0)