Reading OD entries that are not 32 bits results in unpack error

[igutekunst]

I've just started testing out this library for a consulting project. It looks really nice! I've found what I'm pretty sure is a bug.

Problem
I have a CANOpen device with an OD Entry of type I8. When I try to read it from python, I get an unpack error.

Analysis of Problem
It appears that SDO_STRUCT defined on line 14 of sdo.py is hard coded to expect 8 byte SDO responses. I don't have a PR yet, because I wanted to ask about it first.

Proposed Solution
A dictionary of structs for different sized OD entries might be good enough solution. We could then raise an ObjectDictionaryError: Mismatch between expected and actual data size, as currently done when the device sends a 8 byte response, but the python side expects a smaller response.

Traceback (most recent call last):
  File "C:\Users\ig\canopen-science.py", line 35, in <module>
    print "Mode: {0}".format(device_mode.raw)
  File "C:\Python27\lib\site-packages\canopen\common.py", line 64, in raw
    value = self.od.decode_raw(self.data)
  File "C:\Python27\lib\site-packages\canopen\common.py", line 24, in data
    return self.get_data()
  File "C:\Python27\lib\site-packages\canopen\sdo.py", line 242, in get_data
    return self.sdo_node.upload(self.od.index, self.od.subindex)
  File "C:\Python27\lib\site-packages\canopen\sdo.py", line 89, in upload
    response)
error: unpack requires a string argument of length 8

[christiansandberg]

I think the standard way for the SDO server to send SDO data is to pad the data part with zeroes so that the CAN message is always 8 bytes. However, it would not hurt to do that when receiving the data:

def send_request(self, sdo_request):
    ...
    return self.response.ljust(8, b"\x00")

[christiansandberg]

Or is it that the server does not specify the data size? In that case we assume it should be 4 bytes, but we could just unpack the right amount of data with what we know. I just don't know if we should pass the fact that the size is specified or not to the decode function...

[igutekunst]

I think the standard way for the SDO server to send SDO data is to pad the data part with zeroes so that the CAN message is always 8 bytes. However, it would not hurt to do that when receiving the data:

I don't have the standard in front of me at the moment, so I can't confirm, but that sounds plausible. Our particular server sends fewer bytes if the parameter is smaller.

I've temporarily added this function to get it to work, but I'm also uncertain about the correct way.

# Un pack a variable length response
def unpack_response(response):
    length = len(response) - 4
    SDO_STRUCT_RX = struct.Struct("<BHB{0}s".format(length))
    data =  SDO_STRUCT_RX.unpack(response)
    return data

This does correctly break (with the size mismatch exception) when the parameter size in the remote OD is different than the one specified with variable.data_type.

[christiansandberg]

I'm thinking maybe to unpack only the first four bytes and pass the remaining bytes as they are instead.

[igutekunst]

That seems like the way to go. I don't know I dreamed up such a convoluted solution :)

[christiansandberg]

Have you been able to test if the current master works on your device? In that case would like to close this issue.

[igutekunst]

I will try when I get into work today. Thanks for looking into this.

[christiansandberg]

I'm closing this issue now, but you are welcome to re-open it if it still doesn't work.