Skip to content
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.

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

SDO communication after "SDO block upload end" is dropped #39

Closed
martinwag opened this issue May 23, 2017 · 7 comments
Closed

SDO communication after "SDO block upload end" is dropped #39

martinwag opened this issue May 23, 2017 · 7 comments

Comments

@martinwag
Copy link
Collaborator

When doing a SDO block upload and immediately after that starting another SDO request, this request is dropped.
This happens e.g. in the CANopen conformance test tool when checking SDO test 19 followed by 21. This is equivalent to the following linux commands

#do SDO upload block
cansend can0 67f#a0.00.10.00.14.00.00.00
sleep 0.1
cansend can0 67f#a3.00.00.00.00.00.00.00
sleep 0.1
cansend can0 67f#a2.01.14.00.00.00.00.00
sleep 0.1
#SDO block upload end
cansend can0 67f#a1.00.00.00.00.00.00.00
#immediately start next SDO communication
cansend can0 67f#C2.17.10.00.02.00.00.00

I've narrowed this down to the CO_SDO_receive() function. This receives the new request before the "SDO block upload end" message is processed, and therefore just drops it.
When looking at the spec, I can't find anything that would speak against having this sequence. I also think this is a realistic scenario for the client to just start the next request after one has finished.

From looking at the code, I have no idea how to fix this easily, at least for upstream...

I've also had a look at the other SDO transfer methods. They are not affected as the end message is always from server to client.

reference: CiA301, 7.2.4.3.12 Protocol SDO block upload

@martinwag
Copy link
Collaborator Author

This dirty hack makes it work when using FreeRTOS Threads for receiving and processing.

static void CO_SDO_receive(void *object, const CO_CANrxMsg_t *msg){
    CO_SDO_t *SDO;

    SDO = (CO_SDO_t*)object;   /* this is the correct pointer type of the first argument */

    extern void vTaskDelay( const uint32_t xTicksToDelay );
    while (SDO->CANrxNew) {
      vTaskDelay(1);
    }
...

CANopenNode added a commit that referenced this issue Jul 4, 2017
@CANopenNode
Copy link
Owner

Block transfer is quite complicated and large part of code and it needs fast response. SDO->pFunctSignal(); should be used with RTOS.
However, problem you describe still exists and one solution is, as you describe.

I think, I should make SDO block transfer as optional feature in CANopenNode. It should be possible to not include it's code. I think, it is not very widely used and it may add unnecessary memory consumption.

Another thing to consider is block transfer in future editions of the standard. I checked the latest 301 proposal, which includes also CAN FD messages (64 bytes long). SDO transfer is quite different there and block transfer is not used.

For now, I think, careful integration with RTOS is best practice. I added a note into the stack.

@martinwag
Copy link
Collaborator Author

martinwag commented Jul 5, 2017

I think, I found another issue with the used message handling (#48).
For my case, I'm actually using block transfer mode.

@CANopenNode
Copy link
Owner

The only problem is SDO block upload immediately followed by another SDO request. This is the situation, where two client requests (SDO block upload end and next SDO initiate) are transfered without server confirmation between (except block download segment, which is correctly handled by the stack).

One clean solution would be with double SDO->CANrxData buffer inside CO_SDO_receive (or short FIFO). Something similar is used for synchronous RPDOs inside CO_PDO_receive.

@lukegluke
Copy link
Contributor

Hi. Faced the same issue in master branch.
This issue was post long ago, maybe @martinwag you had fix it more accurate than with delay?

By the way, @CANopenNode block transfers work quite well, thank you for your work!

@martinwag
Copy link
Collaborator Author

@lukegluke, no I didn't fix this as block upload is only used once (as last command) in my application. Proper way is using a queue for data exchange.

lukegluke added a commit to lukegluke/CANopenNode that referenced this issue Apr 15, 2020
…secutive block transfers

previously when doing a SDO block upload and immediately after that starting another SDO request, this request was dropped. Especially if processing function has slow response.

fifo queue size is 2 by default
you can redifine CO_SDO_RX_DATA_SIZE to 1 in CO_driver_target.h, if you do not use block transfers to increase performance a little

issue CANopenNode#39
lukegluke added a commit to lukegluke/CANopenNode that referenced this issue Jun 30, 2020
…g it

This is critical fix for PR CANopenNode#174

Before this patch SDO queue process pointer could overrun receive pointer on receiving NMT stop command during active SDO communication.

issue CANopenNode#39
lukegluke added a commit to lukegluke/CANopenNode that referenced this issue Jun 30, 2020
This is critical fix for PR CANopenNode#174

Before this patch SDO queue process pointer could overrun receive pointer on receiving NMT stop command during active SDO communication.

issue CANopenNode#39
CANopenNode pushed a commit that referenced this issue Jul 1, 2020
This is critical fix for PR #174

Before this patch SDO queue process pointer could overrun receive pointer on receiving NMT stop command during active SDO communication.

issue #39
lukegluke added a commit to lukegluke/CANopenNode that referenced this issue Jul 13, 2020
Fix wrong double calling CO_SDO_receive_done for last segment in subblock/block.

This is critical fix for PR CANopenNode#174

issue CANopenNode#39
lukegluke added a commit to lukegluke/CANopenNode that referenced this issue Jul 13, 2020
Fix wrong double calling CO_SDO_receive_done for last segment in subblock/block.

This is critical fix for PR CANopenNode#174

issue CANopenNode#39
CANopenNode pushed a commit that referenced this issue Jul 13, 2020
Fix wrong double calling CO_SDO_receive_done for last segment in subblock/block.

This is critical fix for PR #174

issue #39
@CANopenNode
Copy link
Owner

I'm not quite sure, how is with this issue now. Currently we have:

  • new Object Dictionary approach, rewritten and tested SDO server in CANopenNode version 4.0
  • rewritten and tested SDO client in CANopenNode version 2.0 and above
  • no additional information for old Object Dictionary and old SDO server.
    I'm closing this issue for now. If there will be problems with old SDO server, new issue should be opened.

henrikbrixandersen pushed a commit to vestas-wind-systems/CANopenNode that referenced this issue Oct 19, 2021
…secutive block transfers

previously when doing a SDO block upload and immediately after that starting another SDO request, this request was dropped. Especially if processing function has slow response.

fifo queue size is 2 by default
you can redifine CO_SDO_RX_DATA_SIZE to 1 in CO_driver_target.h, if you do not use block transfers to increase performance a little

issue CANopenNode#39
henrikbrixandersen pushed a commit to vestas-wind-systems/CANopenNode that referenced this issue Oct 19, 2021
…nNode#204)

This is critical fix for PR CANopenNode#174

Before this patch SDO queue process pointer could overrun receive pointer on receiving NMT stop command during active SDO communication.

issue CANopenNode#39
henrikbrixandersen pushed a commit to vestas-wind-systems/CANopenNode that referenced this issue Oct 19, 2021
Fix wrong double calling CO_SDO_receive_done for last segment in subblock/block.

This is critical fix for PR CANopenNode#174

issue CANopenNode#39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants