Abort SDO server upload when the object has zero length

[acolomb]

A data size of zero bytes cannot be encoded in the "number of bytes that do not contain data" semantics, as that would require n=4, which is however limited to two bits (maximum value 3). So the SDO expedited upload protocol simply cannot convey that condition. The current implementation however still tries and thereby corrupts the server command specifier field value.

Avoid that protocol violation by responding with an SDO abort code of 0800 0024h, No data available.

Add a test case for this condition, verifying that the client raises the appropriate SdoAbortedError exception.

Inspired by the discussion in #566.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

[sveinse]

Sorry but I do not agree implementing this as it would exclude a use-case for zero-length data.

While there are some of the SDO protocols that doesn't support zero sized data (e.g. expedited and block), normal (segmented) transfer does support it.

The use case for zero length data is console or serial data, which may or may not be available in the remote at the time of the SDO operation. An example is 7.5.2.29 OS prompt, object 0x1026 subindex 1-3. The standard quote "This object is mappable to an event driven PDO or polled by SDO". This means this must support zero-length data, as there might not be any data available when the subindex is polled by SDO.

For this use case, the remote would unexpectantly receive the SDO abort "No data" from the client, which is not good.

[acolomb]

Why is it "not good" to receive an SDO abort message with appropriate abort code?

Either way, the SDO transaction ends with that message. How is receiving an empty response (beacuse no data is available) any different from receiving an SDO abort message with the reason "no data available"?

Have you seen such behavior on actual devices, as a reference? I mean this is our SDO server component, we can choose freely how to handle that case, as long as we don't violate the protocol. Just because it is possible for some responses to indicate a length of zero, that doesn't mean it is in any way more correct than the alternative.

We can just as well distinguish by request type. The server decides whether a segmented or expedited transfer response is sent. So nothing stops us from sending a response with e=0, s=1 and then encoding zero bytes in the d field. Not following up with a further segment upload seems to be fine (emphasis added):

SDOs are uploaded as a sequence of zero or more SDO upload segment services preceded by an SDO upload initiate service.

So the client might just not follow up with any SDO upload segment requests. So how can we be sure the SDO transfer has ended? The standard allows three cases:

1. Expedited response (e=1). [Cannot encode a zero size.]
2. Upload segment response with c=1. [Needs to be requested first by the client.]
3. Abort (unsuccessful completion). [Proposed solution in this PR.]

4. A new SDO upload initiate request/indication, indicating the unsuccessful completion of the upload sequence and the start of a new sequence.

If the client doesn't continue requesting a segment because we sent a size of zero, that's fine and the next SDO request will implicitly terminate the transaction. It then counts as an unsuccessful transfer, same as if we sent the SDO abort code. Worst case, the client might send an SDO abort indication in response to our size=0 response. If not, it's absoutely equivalent in terms of bus traffic whether we use the SDO abort or the upload initiate response.

[acolomb]

One more thought, which might be closer to your understanding of the use case @sveinse:

We get a request to upload data from some object. The response indicates segmented transfer and a zero size (or even unknown size). The client can then keep requesting an SDO upload segment repeatedly, and gets a zero-length response each time if no data is still available. But that never ends the transaction. The SDO channel stays blocked and bound to the requested object, so each segment request can transfer up to 7 bytes at once, without further initiation. It ends when the server sends a c=1 response, client or server abort the transfer, or client sends a new upload initiate request.

Now which of these cases work with our current SDO upload segment implementation? Especially, does the "streaming" approach with continuously getting back zero bytes, but not ending the transaction actually work? Otherwise, I'd rather start with this quick fix, then we can restructure to make the other solution work.

As for block upload, that's still waiting in PR #559, so no concern currently.

[sveinse]

Why is it "not good" to receive an SDO abort message with appropriate abort code?

Either way, the SDO transaction ends with that message. How is receiving an empty response (beacuse no data is available) any different from receiving an SDO abort message with the reason "no data available"?

I think two main reasons: Its an anti-pattern to communicate non-errors as errors. Errors (SDO abort) are exceptions, i.e. unexpected events. This is a common design principle for all communication systems I've been working on at least. (-- and I feel as strongly about this as you do about little-endian 😊)

The second reason is that the SDO abort is transmitted to the other party. It might not expect to receive a "No data available" just for responding with 0 length data.

Have you seen such behavior on actual devices, as a reference? I mean this is our SDO server component, we can choose freely how to handle that case, as long as we don't violate the protocol. Just because it is possible for some responses to indicate a length of zero, that doesn't mean it is in any way more correct than the alternative.

Yes, I have a unit. Not public, so I have nothing to reference to unfortunately. It implement a serial console using the OS command object with zero-length SDO pollable subindex as described earlier.

To determine if this is a one-off implementation specific detail, I think perhaps we should attempt to reach out to CiA and hear if they have an opinion. It is a bit odd that not all SDO variants support 0-length data, so it might be that this use case is unintended. If it is, I agree that this library does not need to implement 0-length SDO and we can close this discussion.

We can just as well distinguish by request type. The server decides whether a segmented or expedited transfer response is sent. So nothing stops us from sending a response with e=0, s=1 and then encoding zero bytes in the d field. Not following up with a further segment upload seems to be fine (emphasis added):

SDOs are uploaded as a sequence of zero or more SDO upload segment services preceded by an SDO upload initiate service.

The standard is showing in Figure 23 under 7.2.4.3.5 that segments are always present when e=0. This means that one must have at least one segment exchange when e=0. The only way to prevent exchange of segments is when e=1, which we both agree doesn't work with 0-length data.

So the client might just not follow up with any SDO upload segment requests. So how can we be sure the SDO transfer has ended? The standard allows three cases:

1. Expedited response (e=1). [Cannot encode a zero size.]
2. Upload segment response with c=1. [Needs to be requested first by the client.]
3. Abort (unsuccessful completion). [Proposed solution in this PR.]

4. A new SDO upload initiate request/indication, indicating the unsuccessful completion of the upload sequence and the start of a new sequence.

Yes. But the argument here is that 0-length data is covered by case no 2. See below for sequence.

If the client doesn't continue requesting a segment because we sent a size of zero, that's fine and the next SDO request will implicitly terminate the transaction. It then counts as an unsuccessful transfer, same as if we sent the SDO abort code. Worst case, the client might send an SDO abort indication in response to our size=0 response. If not, it's absoutely equivalent in terms of bus traffic whether we use the SDO abort or the upload initiate response.

We get a request to upload data from some object. The response indicates segmented transfer and a zero size (or even unknown size). The client can then keep requesting an SDO upload segment repeatedly, and gets a zero-length response each time if no data is still available. But that never ends the transaction. The SDO channel stays blocked and bound to the requested object, so each segment request can transfer up to 7 bytes at once, without further initiation. It ends when the server sends a c=1 response, client or server abort the transfer, or client sends a new upload initiate request.

Now which of these cases work with our current SDO upload segment implementation? Especially, does the "streaming" approach with continuously getting back zero bytes, but not ending the transaction actually work? Otherwise, I'd rather start with this quick fix, then we can restructure to make the other solution work.

As for block upload, that's still waiting in PR #559, so no concern currently.

The SDO bus is a "single channel" communication scheme, with only one active transmission per SDO server. Even though its technically possible to keep a transmission "alive" by keep responding 0-sized data packages with c=0, it will block all the other SDO communication. So any messages should finish as quickly as possible.

Sequence for 0-length data, client <--> server
--> Initiate upload request <-- Initiate upload response, e=0, s=1, d=0 --> Upload segment request <-- Upload segment response, c=1, n=7, no data
--> Initiate upload request <-- Initiate upload response, e=0, s=0 --> Upload segment request <-- Upload segment response, c=1, n=7, no data
--> Initiate download request, e=0, s=1, d=0 <-- Initiate download response --> Download segment request, n=7, c=1, no data <-- Download segment response
--> Initiate download request, e=0, s=0 <-- Initiate download response --> Download segment request, n=7, c=1, no data <-- Download segment response

[sveinse]

After trying to google this, I think I need to conclude that 0-length data is a very uncommon corner of SDO. I find absolutely no examples of zero-length data or examples. I read the entire SDO chapter in the spec, and it never mentioned zero length data. I think I am sensing between the lines that it's a not intended use-case. Otherwise it would have been mentioned.

I'm also not finding any reference to SDO abort "no data available" being used on zero length data. I tried looking briefly at a couple of open source canopen libraries, but I couldn't find any use of it. -- I think this corner is probably a case no one has considered it seems.

For fun: Google AI sais (which definitely might be wrong):
""No data available" in the context of CANopen SDO (Service Data Object) CANopen for Python refers to a specific error code, 0x08000024, indicating that the requested object or sub-index does not contain any data. This error typically occurs when a CANopen client attempts to read data from an object dictionary on a server, but the server does not have any data to return for that specific object or sub-index."

[acolomb]

Your responses are well thought out and I agree with your assessment.

1. It is a rather uncommon use case to have objects with zero length (even temporarily, such as your OS prompt).
2. I do see merit in facilitating it, in whatever way the specified protocol allows. Especially for DOMAIN objects, there are certainly valid reasons, like some kind of data streaming applications.
3. I understand your reasoning for avoiding the SDO abort code. Although I think it's more of an implementation problem in the (any) client if they do not expect such a response, even though they perfectly anticipated the case of no data being available.
4. This change only concerns our SDO server component, which means we can choose any response behavior which the standard covers as valid.
5. Right now (and especially for this PR) we're only concerned with SDO upload and the server implementation chooses whether a size is even indicated if the available data has zero length. I confirm your other scenarios though, they look correct to me. The download cases should be checked against our client code, possibly resulting in another PR.
6. I accept that issue is only about expedited transfers. For upload, the server MUST NOT use an expedited response if the intention is to signal zero data available. For download, the client simply cannot make such a request while setting the e=1, s=1, as it would convey at least one byte. e=1, s=0 is theoretically possible, which punts the decision to the server how much data to interpret.
7. I'd extend your listed sequences by a variant which does in fact keep the transfer pending, by sending an upload segment response with c=0, n=7, which can be used for quickly polling repeatedly. The advantage is that there needs to be no further initiate request, and 7 bytes can be transferred immediately with each request when data does become available.
8. I disagree with your interpretation of Figure 23. It is just a figure and the text explicitly says that zero is a possible number of segments. The picture just can't convey that easily.

Now to the really important points: Our server currently gives totally bogus responses for an object with zero data to encode.

• It uses expedited transfer, which is the only(?) mode not capable of signaling zero size.
• It specifies a data size of four bytes.
• Most importantly, it thrashes the "x" flag, clearly violating the specification of "always 0".
• It inflates the response size by inserting byte values because [4:4+0] is a zero-length slice. I suspect that might even throw off some backend interfaces when trying to transmit the frame.
• Not specifying the size isn't even considered in our server upload code (always sets s=1).

Whatever we do, we should fix the clear protocol violations. Aborting the transfer as suggested is just a simple reaction to the special case.

If you feel like supporting more of the possible response behaviors, I'm open to consider adding them. The SDO abort is not set in stone, just the most obvious valid choice for me. But feel free to make an alternative PR with a different choice. If it ends up requiring a matching response to the following upload segment request, that should be implemented as well though. The current approach avoids that by ending the transaction (which can easily be retried anyway).

[sveinse]

I have to humbly admit that I had completely overlooked that this PR is discussing the SDO server, not the client. That changes things completely.

Now to the really important points: Our server currently gives totally bogus responses for an object with zero data to encode.

• It uses expedited transfer, which is the only(?) mode not capable of signaling zero size.
• It specifies a data size of four bytes.
• Most importantly, it thrashes the "x" flag, clearly violating the specification of "always 0".
• It inflates the response size by inserting byte values because [4:4+0] is a zero-length slice. I suspect that might even throw off some backend interfaces when trying to transmit the frame.
• Not specifying the size isn't even considered in our server upload code (always sets s=1).

Whatever we do, we should fix the clear protocol violations. Aborting the transfer as suggested is just a simple reaction to the special case.

Agreed. The current implementation is not handling 0 sized data right and must be fixed. On upload its the server's privilege to chose expedited vs segmented, so it makes sense its also the server's privilege (and our choice) to not support zero-length data. This will be compliant with the standard.

If you feel like supporting more of the possible response behaviors, I'm open to consider adding them. The SDO abort is not set in stone, just the most obvious valid choice for me. But feel free to make an alternative PR with a different choice. If it ends up requiring a matching response to the following upload segment request, that should be implemented as well though. The current approach avoids that by ending the transaction (which can easily be retried anyway).

Again, I'm sorry for this apparent 180 degrees turn - I missed a crucial piece of information. I should have read the PR better before typing off.

[sveinse]

I think we can say this PR has been thoroughly discussed. Approved.