Skip to content

eds: fix signed integer limit parsing and export for all SIGNED_TYPES #658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
friederschueler wants to merge 6 commits into
base: master
Choose a base branch
from
+144 −41
Open

eds: fix signed integer limit parsing and export for all SIGNED_TYPES #658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bf972c1
eds: fix LowLimit/HighLimit parsing for all signed integer types
friederschueler May 7, 2026
f65a1fb
eds: revert LowLimit/HighLimit parse error to silent pass, remove war…
friederschueler May 7, 2026
4ed45dd
add back and add more exception handling via warning instead of silen…
friederschueler May 11, 2026
4319d4a
eds: use eds.get() in warning for DefaultValue/ParameterValue (mypy fix)
friederschueler May 11, 2026
2382b96
eds: support decimal and hex literals for signed LowLimit/HighLimit
friederschueler May 11, 2026
28c7d26
Consistent logic with early return / raise.
acolomb May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 61 additions & 29 deletions canopen/objectdictionary/eds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,12 @@ def import_from_node(node_id: int, network: canopen.network.Network):
return od


def _calc_bit_length(data_type):
if data_type == datatypes.INTEGER8:
return 8
elif data_type == datatypes.INTEGER16:
return 16
elif data_type == datatypes.INTEGER32:
return 32
elif data_type == datatypes.INTEGER64:
return 64
def _calc_bit_length(data_type: int) -> int:
if data_type in datatypes.SIGNED_TYPES:
st = ODVariable.STRUCT_TYPES[data_type]
if isinstance(st, datatypes.IntegerN):
return st.width
return st.size * 8
else:
raise ValueError(
f"Invalid data_type '{data_type}', expecting a signed integer data_type."
Comment on lines +208 to 215
Copy link
Copy Markdown
Member

@acolomb acolomb May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works equally well for unsigned integers (and could be used for floats if needed):

Suggested change
if data_type in datatypes.SIGNED_TYPES:
st = ODVariable.STRUCT_TYPES[data_type]
if isinstance(st, datatypes.IntegerN):
return st.width
return st.size * 8
else:
raise ValueError(
f"Invalid data_type '{data_type}', expecting a signed integer data_type."
if data_type in datatypes.INTEGER_TYPES:
st = ODVariable.STRUCT_TYPES[data_type]
if isinstance(st, datatypes.IntegerN):
return st.width
return st.size * 8
else:
raise ValueError(
f"Invalid data_type 0x{data_type:04X}, expecting an integer data_type."

Expand All @@ -221,13 +218,27 @@ def _calc_bit_length(data_type):

def _signed_int_from_hex(hex_str, bit_length):
number = int(hex_str, 0)
max_value = (1 << (bit_length - 1)) - 1
min_signed = -(1 << (bit_length - 1))
max_signed = (1 << (bit_length - 1)) - 1
max_unsigned = (1 << bit_length) - 1

if number > max_value:
return number - (1 << bit_length)
else:
if number < min_signed:
raise ValueError(
f"Value {hex_str!r} is out of range for a {bit_length}-bit signed integer"
)
if number < 0:
# Negative literal (e.g. LowLimit=-32768 or -0x8000)
return number

if number > max_unsigned:
raise ValueError(
f"Value {hex_str!r} is out of range for a {bit_length}-bit signed integer"
)
if number > max_signed:
# Unsigned hex literal, two's-complement (e.g. LowLimit=0xFFFF → -1 for INTEGER16)
return number - (1 << bit_length)
return number


def _convert_variable(node_id, var_type, value):
if var_type in (datatypes.OCTET_STRING, datatypes.DOMAIN):
Expand All @@ -245,6 +256,18 @@ def _convert_variable(node_id, var_type, value):
return int(value, 0)


def _int_to_hex(data_type: int, value: int) -> str:
"""Format an integer as EDS hex string.

Signed types with a negative value are written as two's-complement hex
(e.g. INTEGER8 -1 → 0xFF) so the output is a valid EDS literal.
Copy link
Copy Markdown
Member

@acolomb acolomb May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to CiA 306:

Integer numbers shall be written as decimal numbers, hexadecimal numbers or octal numbers.

I don't see any need to restrict ourselves to writing hex or two's complement at all. We may still accept it, though, as the data type rules are very clear on the possible interpretation. But the spec doesn't mention this at all.

It's also completely unclear how to handle floating point numbers in EDS files. I guess if we can parse them in Python, we can accept them. But I wouldn't assume any particular conversion to apply for a literal in hex notation: It's fine to write -0xFFFF.FF as a theoretical floating-point number noted in hex, just that Python only parses decimal floats.

>>> float(int("-0xFFFF", 16))  # this works fine, but not directly
-65535.0

Just because CiA 301 defines a wire format for REAL types, doesn't mean we should try parsing hexadecimal byte sequences as IEEE floats. When exporting, the best approach I can see is to simply use Python's float to string conversion. This function gets in the way though, prohibiting us from writing anything but integer limits at all. Why not simply use _revert_variable() for the limits as well? The fall-back (integer) case in there can still do some limit checking, but no hex conversion.

"""
if data_type in datatypes.SIGNED_TYPES and value < 0:
bit_length = _calc_bit_length(data_type)
return f"0x{value + (1 << bit_length):0{bit_length // 4}X}"
return f"0x{value:02X}"


def _revert_variable(var_type, value):
if value is None:
return None
Expand All @@ -255,7 +278,7 @@ def _revert_variable(var_type, value):
elif var_type in datatypes.FLOAT_TYPES:
return value
else:
return f"0x{value:02X}"
return _int_to_hex(var_type, value)


def build_variable(
Expand Down Expand Up @@ -309,7 +332,10 @@ def build_variable(
else:
var.min = int(min_string, 0)
except ValueError:
pass
logger.warning(
"Invalid LowLimit %r for %s (0x%X), ignoring",
eds.get(section, "LowLimit"), var.name, var.index,
)
if eds.has_option(section, "HighLimit"):
try:
max_string = eds.get(section, "HighLimit")
Expand All @@ -318,38 +344,44 @@ def build_variable(
else:
var.max = int(max_string, 0)
except ValueError:
pass
logger.warning(
"Invalid HighLimit %r for %s (0x%X), ignoring",
eds.get(section, "HighLimit"), var.name, var.index,
)
if eds.has_option(section, "DefaultValue"):
try:
var.default_raw = eds.get(section, "DefaultValue")
if '$NODEID' in var.default_raw:
var.relative = True
var.default = _convert_variable(node_id, var.data_type, var.default_raw)
except ValueError:
pass
logger.warning(
"Invalid DefaultValue %r for %s (0x%X), ignoring",
eds.get(section, "DefaultValue"), var.name, var.index,
)
if eds.has_option(section, "ParameterValue"):
try:
var.value_raw = eds.get(section, "ParameterValue")
var.value = _convert_variable(node_id, var.data_type, var.value_raw)
except ValueError:
pass
logger.warning(
"Invalid ParameterValue %r for %s (0x%X), ignoring",
eds.get(section, "ParameterValue"), var.name, var.index,
)
# Factor, Description and Unit are not standard according to the CANopen specifications, but
# they are implemented in the python canopen package, so we can at least try to use them
if eds.has_option(section, "Factor"):
try:
var.factor = float(eds.get(section, "Factor"))
except ValueError:
pass
logger.warning(
"Invalid Factor %r for %s (0x%X), ignoring",
eds.get(section, "Factor"), var.name, var.index,
)
if eds.has_option(section, "Description"):
try:
var.description = eds.get(section, "Description")
except ValueError:
pass
var.description = eds.get(section, "Description")
if eds.has_option(section, "Unit"):
try:
var.unit = eds.get(section, "Unit")
except ValueError:
pass
var.unit = eds.get(section, "Unit")
return var


Expand Down Expand Up @@ -414,9 +446,9 @@ def export_variable(var, eds):
eds.set(section, "PDOMapping", hex(var.pdo_mappable))

if getattr(var, 'min', None) is not None:
eds.set(section, "LowLimit", var.min)
eds.set(section, "LowLimit", _int_to_hex(var.data_type, var.min))
if getattr(var, 'max', None) is not None:
eds.set(section, "HighLimit", var.max)
eds.set(section, "HighLimit", _int_to_hex(var.data_type, var.max))

if getattr(var, 'description', '') != '':
eds.set(section, "Description", var.description)
Expand Down
54 changes: 54 additions & 0 deletions test/sample.eds
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,24 @@ HighLimit=0x0A
LowLimit=0x02
PDOMapping=0

[3022]
ParameterName=UNSIGNED16 decimal limits
ObjectType=0x7
DataType=0x06
AccessType=rw
LowLimit=100
HighLimit=1000
PDOMapping=0

[3023]
ParameterName=INTEGER16 decimal limits
ObjectType=0x7
DataType=0x03
AccessType=rw
LowLimit=-100
HighLimit=100
PDOMapping=0

[3030]
ParameterName=INTEGER32 only negative values
ObjectType=0x7
Expand All @@ -976,6 +994,42 @@ HighLimit=0xFFFFFFFF
LowLimit=0x80000000
PDOMapping=0

[3031]
ParameterName=INTEGER24 value range -1 to 0
ObjectType=0x7
DataType=0x10
AccessType=rw
HighLimit=0x000000
LowLimit=0xFFFFFF
PDOMapping=0

[3032]
ParameterName=INTEGER40 value range -1 to 0
ObjectType=0x7
DataType=0x12
AccessType=rw
HighLimit=0x0000000000
LowLimit=0xFFFFFFFFFF
PDOMapping=0

[3033]
ParameterName=INTEGER48 value range -1 to 0
ObjectType=0x7
DataType=0x13
AccessType=rw
HighLimit=0x000000000000
LowLimit=0xFFFFFFFFFFFF
PDOMapping=0

[3034]
ParameterName=INTEGER56 value range -1 to 0
ObjectType=0x7
DataType=0x14
AccessType=rw
HighLimit=0x00000000000000
LowLimit=0xFFFFFFFFFFFFFF
PDOMapping=0

[3040]
ParameterName=INTEGER64 value range -10 to +10
ObjectType=0x7
Expand Down
41 changes: 29 additions & 12 deletions test/test_eds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,23 @@ def test_record(self):
self.assertFalse(var.is_domain)

def test_record_with_limits(self):
int8 = self.od[0x3020]
self.assertEqual(int8.min, 0)
self.assertEqual(int8.max, 127)
uint8 = self.od[0x3021]
self.assertEqual(uint8.min, 2)
self.assertEqual(uint8.max, 10)
int32 = self.od[0x3030]
self.assertEqual(int32.min, -2147483648)
self.assertEqual(int32.max, -1)
int64 = self.od[0x3040]
self.assertEqual(int64.min, -10)
self.assertEqual(int64.max, +10)
cases = [
(0x3020, 0, 127), # INTEGER8 hex limits
(0x3021, 2, 10), # UNSIGNED8 hex limits
(0x3022, 100, 1000), # UNSIGNED16 decimal limits
(0x3023, -100, 100), # INTEGER16 decimal limits
(0x3030, -2147483648, -1), # INTEGER32 hex limits
(0x3031, -1, 0), # INTEGER24 hex limits
(0x3032, -1, 0), # INTEGER40 hex limits
(0x3033, -1, 0), # INTEGER48 hex limits
(0x3034, -1, 0), # INTEGER56 hex limits
(0x3040, -10, +10), # INTEGER64 hex limits
]
for index, expected_min, expected_max in cases:
with self.subTest(index=f"0x{index:04X}"):
var = self.od[index]
self.assertEqual(var.min, expected_min)
self.assertEqual(var.max, expected_max)

def test_signed_int_from_hex(self):
for data_type, test_cases in self.test_data.items():
Expand All @@ -156,6 +161,18 @@ def test_signed_int_from_hex(self):
result = _signed_int_from_hex('0x' + test_case["hex_str"], test_case["bit_length"])
self.assertEqual(result, test_case["expected"])

def test_signed_int_from_hex_accepts_decimal(self):
# Negative decimal values are valid EDS literals (CiA 306 allows both formats).
self.assertEqual(_signed_int_from_hex("-1", 8), -1)
self.assertEqual(_signed_int_from_hex("-128", 8), -128)
self.assertEqual(_signed_int_from_hex("-2147483648", 32), -2147483648)

def test_signed_int_from_hex_rejects_out_of_range(self):
with self.assertRaises(ValueError):
_signed_int_from_hex("0xFFFF", 8) # 16-bit value into 8-bit field
with self.assertRaises(ValueError):
_signed_int_from_hex("-129", 8) # below minimum for 8-bit signed

def test_array_compact_subobj(self):
array = self.od[0x1003]
self.assertIsInstance(array, canopen.objectdictionary.ODArray)
Expand Down
Loading