Skip to content

Conversation

@SeppoTakalo
Copy link
Contributor

After the PR #85000 the calculation of NUM field of Block 1 option on CoAP Ack packet started to advance to next packet block.

We should not update the ctx->current field because it is used for calculating the NUM field in response packet. It should point to beginning of the payload,
so the response is correct.

Leshan server don't seem to care about this, but Coiote does.

After the PR zephyrproject-rtos#85000 the calculation of NUM field of
Block 1 option on CoAP Ack packet started to advance
to next packet block.

We should not update the ctx->current field because it is
used for calculating the NUM field in response packet.
It should point to beginning of the payload,
so the response is correct.

Leshan server don't seem to care about this, but Coiote does.

Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>
@SeppoTakalo SeppoTakalo requested review from Copilot, kartben and rlubos May 8, 2025 13:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses an issue with the blockwise Ack NUM calculation on CoAP packets by removing an update to the block context pointer.

  • Removed the modification of ctx->current in lwm2m_write_handler_opaque to ensure it points to the payload beginning.
Comments suppressed due to low confidence (1)

subsys/net/lib/lwm2m/lwm2m_message_handling.c:1059

  • The removal of the block context update appears to be the fix per the PR description, but consider adding an inline comment to clarify that this update was removed to correctly calculate the NUM field for CoAP Ack packets (referencing PR #85000 for context).
if (msg->in.block_ctx && !last_pkt_block) {

@kartben kartben merged commit 252f8fe into zephyrproject-rtos:main May 12, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants