Skip to content

Fixed HighLimit and LowLimit for SIGNED values in EDS #345

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

Merged
merged 3 commits into from
Feb 24, 2023
Merged

Fixed HighLimit and LowLimit for SIGNED values in EDS #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
982d58b
Fixed incorrect min (LowLImit) and max (HighLimit) values when using …
friederschueler Feb 17, 2023
4cf240b
Fixed min/max for non-signed datatypes and added tests
friederschueler Feb 17, 2023
544a07d
Removed type hints
friederschueler Feb 22, 2023
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
42 changes: 37 additions & 5 deletions canopen/objectdictionary/eds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import re
import io
import logging
import copy
import logging
import re

from canopen.objectdictionary import datatypes

try:
from configparser import RawConfigParser, NoOptionError, NoSectionError
except ImportError:
Expand Down Expand Up @@ -190,6 +192,28 @@ def import_from_node(node_id, 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
else:
raise ValueError(f"Invalid data_type '{data_type}', expecting a signed integer 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
else:
return number


def _convert_variable(node_id, var_type, value):
if var_type in (objectdictionary.OCTET_STRING, objectdictionary.DOMAIN):
return bytes.fromhex(value)
Expand Down Expand Up @@ -251,12 +275,20 @@ def build_variable(eds, section, node_id, index, subindex=0):

if eds.has_option(section, "LowLimit"):
try:
var.min = int(eds.get(section, "LowLimit"), 0)
min_string = eds.get(section, "LowLimit")
if var.data_type in objectdictionary.SIGNED_TYPES:
var.min = _signed_int_from_hex(min_string, _calc_bit_length(var.data_type))
else:
var.min = int(min_string, 0)
except ValueError:
pass
if eds.has_option(section, "HighLimit"):
try:
var.max = int(eds.get(section, "HighLimit"), 0)
max_string = eds.get(section, "HighLimit")
if var.data_type in objectdictionary.SIGNED_TYPES:
var.max = _signed_int_from_hex(max_string, _calc_bit_length(var.data_type))
else:
var.max = int(max_string, 0)
except ValueError:
pass
if eds.has_option(section, "DefaultValue"):
Expand Down
36 changes: 36 additions & 0 deletions test/sample.eds
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,3 +902,39 @@ DataType=0x0008
AccessType=ro
DefaultValue=0
PDOMapping=1

[3020]
ParameterName=INTEGER8 only positive values
ObjectType=0x7
DataType=0x02
AccessType=rw
HighLimit=0x7F
LowLimit=0x00
PDOMapping=0

[3021]
ParameterName=UNSIGNED8 value range +2 to +10
ObjectType=0x7
DataType=0x05
AccessType=rw
HighLimit=0x0A
LowLimit=0x02
PDOMapping=0

[3030]
ParameterName=INTEGER32 only negative values
ObjectType=0x7
DataType=0x04
AccessType=rw
HighLimit=0x00000000
LowLimit=0xFFFFFFFF
PDOMapping=0

[3040]
ParameterName=INTEGER64 value range -10 to +10
ObjectType=0x7
DataType=0x15
AccessType=rw
HighLimit=0x000000000000000A
LowLimit=0x8000000000000009
PDOMapping=0
92 changes: 55 additions & 37 deletions test/test_eds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

EDS_PATH = os.path.join(os.path.dirname(__file__), 'sample.eds')


class TestEDS(unittest.TestCase):

def setUp(self):
Expand Down Expand Up @@ -47,6 +48,20 @@ def test_record(self):
self.assertEqual(var.data_type, canopen.objectdictionary.UNSIGNED32)
self.assertEqual(var.access_type, 'ro')

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)
Copy link

@lamuguo lamuguo Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

int32.min should be -1 in my understanding.

Copy link
Collaborator Author

@friederschueler friederschueler Sep 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You are correct. I don't know why I didn't see it. This should be the correct function:

def _signed_int_from_hex(hex_str, bit_length):
    number = int(hex_str, 0)
    max_value = (1 << (bit_length - 1)) - 1
    
    if number > max_value:
        return number - (1 << bit_length)
    else:
        return number

I will commit a full fix with correct tests in the evening.

self.assertEqual(int32.max, 0)
int64 = self.od[0x3040]
self.assertEqual(int64.min, -10)
self.assertEqual(int64.max, +10)

def test_array_compact_subobj(self):
array = self.od[0x1003]
self.assertIsInstance(array, canopen.objectdictionary.Array)
Expand Down Expand Up @@ -98,73 +113,76 @@ def test_dummy_variable_undefined(self):

def test_comments(self):
self.assertEqual(self.od.comments,
"""
"""
|-------------|
| Don't panic |
|-------------|
""".strip()
)

""".strip())

def test_export_eds(self):
import tempfile
for doctype in {"eds", "dcf"}:
with tempfile.NamedTemporaryFile(suffix="."+doctype, mode="w+") as tempeds:
with tempfile.NamedTemporaryFile(suffix="." + doctype, mode="w+") as tempeds:
print("exporting %s to " % doctype + tempeds.name)
canopen.export_od(self.od, tempeds, doc_type=doctype)
tempeds.flush()
exported_od = canopen.import_od(tempeds.name)

for index in exported_od:
self.assertIn(exported_od[index].name, self.od)
self.assertIn(index , self.od)
self.assertIn(index, self.od)

for index in self.od:
if index < 0x0008:
# ignore dummies
continue
self.assertIn(self.od[index].name, exported_od)
self.assertIn(index , exported_od)
self.assertIn(index, exported_od)

actual_object = exported_od[index]
expected_object = self.od[index]
actual_object = exported_od[index]
expected_object = self.od[index]
self.assertEqual(type(actual_object), type(expected_object))
self.assertEqual(actual_object.name, expected_object.name)

if type(actual_object) is canopen.objectdictionary.Variable:
expected_vars = [expected_object]
actual_vars = [actual_object ]
else :
actual_vars = [actual_object]
else:
expected_vars = [expected_object[idx] for idx in expected_object]
actual_vars = [actual_object [idx] for idx in actual_object]
actual_vars = [actual_object[idx] for idx in actual_object]

for prop in [
"allowed_baudrates",
"vendor_name",
"vendor_number",
"product_name",
"product_number",
"revision_number",
"order_code",
"simple_boot_up_master",
"simple_boot_up_slave",
"granularity",
"dynamic_channels_supported",
"group_messaging",
"nr_of_RXPDO",
"nr_of_TXPDO",
"LSS_supported",
"allowed_baudrates",
"vendor_name",
"vendor_number",
"product_name",
"product_number",
"revision_number",
"order_code",
"simple_boot_up_master",
"simple_boot_up_slave",
"granularity",
"dynamic_channels_supported",
"group_messaging",
"nr_of_RXPDO",
"nr_of_TXPDO",
"LSS_supported",
]:
self.assertEqual(getattr(self.od.device_information, prop), getattr(exported_od.device_information, prop), f"prop {prop!r} mismatch on DeviceInfo")


for evar,avar in zip(expected_vars,actual_vars):
self. assertEqual(getattr(avar, "data_type" , None) , getattr(evar,"data_type" ,None) , " mismatch on %04X:%X"%(evar.index, evar.subindex))
self. assertEqual(getattr(avar, "default_raw", None) , getattr(evar,"default_raw",None) , " mismatch on %04X:%X"%(evar.index, evar.subindex))
self. assertEqual(getattr(avar, "min" , None) , getattr(evar,"min" ,None) , " mismatch on %04X:%X"%(evar.index, evar.subindex))
self. assertEqual(getattr(avar, "max" , None) , getattr(evar,"max" ,None) , " mismatch on %04X:%X"%(evar.index, evar.subindex))
self.assertEqual(getattr(self.od.device_information, prop),
getattr(exported_od.device_information, prop),
f"prop {prop!r} mismatch on DeviceInfo")

for evar, avar in zip(expected_vars, actual_vars):
self.assertEqual(getattr(avar, "data_type", None), getattr(evar, "data_type", None),
" mismatch on %04X:%X" % (evar.index, evar.subindex))
self.assertEqual(getattr(avar, "default_raw", None), getattr(evar, "default_raw", None),
" mismatch on %04X:%X" % (evar.index, evar.subindex))
self.assertEqual(getattr(avar, "min", None), getattr(evar, "min", None),
" mismatch on %04X:%X" % (evar.index, evar.subindex))
self.assertEqual(getattr(avar, "max", None), getattr(evar, "max", None),
" mismatch on %04X:%X" % (evar.index, evar.subindex))
if doctype == "dcf":
self.assertEqual(getattr(avar, "value" , None) , getattr(evar,"value" ,None) , " mismatch on %04X:%X"%(evar.index, evar.subindex))
self.assertEqual(getattr(avar, "value", None), getattr(evar, "value", None),
" mismatch on %04X:%X" % (evar.index, evar.subindex))

self.assertEqual(self.od.comments, exported_od.comments)