Check writeability for SDO download on LocalNode

[semiversus]

SDO download on LocalNode is not checked for writeability. This pull request fixes it.

[semiversus]

I'm not sure if this merge request is a good idea. It looks easy, but...

When I'm doing something like this, it will not work, if 0x1800 is read only

node.sdo[0x1800][1].raw = 0x40000180 + node_id  # set COB-ID for TPDO1

Naive me thought, this should directly set the raw value of the variable, but the library is going down the rabbit hole and is doing a download via the sdo server.

  File "canopen/variable.py", line 88, in raw
    self.data = self.od.encode_raw(value)
  File "canopen/variable.py", line 40, in data
    self.set_data(data)
  File "canopen/sdo/base.py", line 112, in set_data
    self.sdo_node.download(self.od.index, self.od.subindex, data, force_segment)
  File "canopen/sdo/server.py", line 213, in download
    return self._node.set_data(index, subindex, data, check_writable=True)
  File "canopen/node/local.py", line 89, in set_data
    raise SdoAbortedError(0x06010002)
canopen.sdo.exceptions.SdoAbortedError: Code 0x06010002, Attempt to write a read only object

I liked the idea to have RemoteNode and LocalNode on the same network. But in the idea things get complicated. As my main target is a LocalNode I'm not sure if it's the right path. Cool library anyway - keep on!

Please close this merge request, if you're afraid it might break existing code.

[christiansandberg]

I don't remember why I did not implement write check. There is a TODO for it in the file. Maybe it would work if you replace the download call on line 139 with set_data instead like it's done for segmented download.

[semiversus]

This solves the issue.

Another thing I just saw in the code: Concurrent segmented transfers wouldn't work, because ._index, ._subindex, ._buffer, and ._toggle are single instances. The result of concurrent segmented transfers would result in a strange behavior.

[christiansandberg]

Now I remember what this was for. Preferably we should check if it is writable before accepting segmented data. Right now a segmented download won't fail until the data has been fully received. So you can just leave the TODO for now.

[semiversus]

re-added the comment

[christiansandberg]

In my understanding CANopen is not designed for concurrent SDO requests. It is usually accomplished using multiple SDO channels with different CAN IDs.

[semiversus]

Regarding concurrent SDO segmented requests: You're probably right. Imagine two clients try to segment download to the same object will never work within one channel.

Do you do hackathons? If so, I would join you doing some work on this library.

[christiansandberg]

Unfortunately I don't have much spare time anymore for programming. But I hope someone would be able to fix things regarding the slave functionality since that part has been more of an afterthought.