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

LwM2M block-wise transfer: add callback to notify upper layer that a transfer has restarted #71269

Closed
marco85mars20 opened this issue Apr 9, 2024 · 2 comments · Fixed by #72590
Assignees
Labels
area: LWM2M Enhancement Changes/Updates/Additions to existing features

Comments

@marco85mars20
Copy link
Collaborator

marco85mars20 commented Apr 9, 2024

When a Zephyr device receives data via Block-Transfer, each block is passed to the upper layers via a post-write callback. The receiving function knows:

  • the size of the current block
  • the total size of the data being transferred
  • if the current block is the last one

However, if the transfer fails, depending on the CoAP configuration of the client (sending the data), the transfer might be restarted from scratch, ie. from block #0. Zephyr detects the reception of the first block for an already existing transfer, removes the old context for the block transfer and creates a new one. However, the application is not notified about it and keeps appending the incoming blocks for the new transfers to those from the old one.

Describe the solution you'd like

  • A possible solution could be to add a parameter to the post-write callback (eg. first_block) set to true when receiving block #0
  • Another solution could be to modify the post-write callback definition and replace the parameter last_block with offset of the data received. The callback function can easily detect whether it's the first or last block.
  • probably the least invasive solution would be to have a dummy call to the post-write callback, with:
    • data_len = 0
    • data[0] = 0x0
    • last_block = true
    • total_size = <data size>
      This would allow the post-write callback to detect the error condition and reset its own counters. After the dummy call, the new block 0 can be sent to the post-write callback.
@SeppoTakalo
Copy link
Collaborator

As you have found a real issue here, my proposal would be even a third option: refactor the post-write callback

  1. Remove any Block-Wise parameters from post-write. It should only receive a full payload, if that is available.
  2. Define a new callback and resource type that can handle block-wise transfers. Even without having full memory for it.

This new resource type should allow both read&write using block-wise.

@rlubos
Copy link
Contributor

rlubos commented Apr 11, 2024

I'm not sure if I'm convinced about introducing yet another callback type (although I'm of course open for discussion), wouldn't that be confusing to have two different types of write callbacks, depending on whether you expect to receive resource data via block transfer or not? Especially, that post-write wouldn't be reliably called then, for example during FW updates.

For me the most appealing option would be to simply add the offset parameter to the callback. It would not only allow to detect when the transfer starts from scratch, but also detect data stream inconsistencies (although I believe that the LwM2M engine already guarantees that out-of-order blocks are not forwarded to the application).

I don't think though that we could drop last_block parameter - total_size is not always available, i.e. if the CoAP peer does not send the Size1/2 option, we have no information about the total resource size. Maybe we should group block parameters given to the callback function into some structure, and provide a pointer to the structure in the callback instead - that way apps could easily determine whether block transfer is used or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: LWM2M Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants