Skip to content

Make the response size sent through coap.sendResponse function dyanmic.#47

Open
trishantpahwa wants to merge 1 commit intohirotakaster:masterfrom
trishantpahwa:master
Open

Make the response size sent through coap.sendResponse function dyanmic.#47
trishantpahwa wants to merge 1 commit intohirotakaster:masterfrom
trishantpahwa:master

Conversation

@trishantpahwa
Copy link
Copy Markdown
Contributor

This pull request introduces support for a separate, dynamically-sized response buffer in the CoAP implementation, allowing for more flexible handling of large response payloads. The changes include modifications to both the core CoAP library and an example usage to demonstrate dynamic buffer sizing.

Response Buffer Management

  • Added a dedicated resp_buffer and associated resp_buf_size member to the Coap class, allowing response packets to use a separate buffer from the transmit buffer. The constructor now accepts an optional response buffer size parameter, and memory management for the new buffer is handled in the destructor. (coap-simple.cpp, coap-simple.h)
  • Implemented the setResponseBufferSize(int size) method to allow dynamic resizing of the response buffer at runtime. (coap-simple.cpp, coap-simple.h)

Packet Sending Logic

  • Refactored sendPacket methods to accept a buffer and buffer size, ensuring both requests and responses can use different buffers and sizes. All internal buffer size checks now use the provided buffer size instead of a fixed value. (coap-simple.cpp, coap-simple.h)
  • Updated sendResponse to use the new response buffer and size. (coap-simple.cpp)

Example Usage

  • Demonstrated dynamic response buffer sizing in the ESP8266 example by setting the response buffer size to 4096 bytes before sending a response. (examples/esp8266/esp8266.ino)
  • Minor code style/clarification fixes in comments and whitespace. (coap-simple.cpp, examples/esp8266/esp8266.ino)

@hirotakaster
Copy link
Copy Markdown
Owner

Hi @trishantpahwa

Thanks for your contribution.

  • Regression / Not Found handling

    • In loop(), when the URI is not found, it responds with sendResponse(..., COAP_NOT_FOUNT, ...). This behavior (and the COAP_NOT_FOUNT typo to FOUND) was a known issue and has already been fixed in v1.3.29.

    • If this PR was created from an v.1.3.28, a rebase/merge may resolve it—could you please confirm it is consistent with the current behavior?

  • UDP Packet Fragmentation and feasibility of RFC 7959 (Block-Wise Transfer)

    • The example setResponseBufferSize(4096) may encourage sending a large CoAP response as a single UDP datagram. With large UDP payloads, this can exceed the link MTU and therefore rely on IP fragmentation. Fragmented UDP packets may be dropped depending on the network path (NAT, firewalls, Wi‑Fi quality, etc.), which can make responses unreliable.

    • As a rough guideline: with Ethernet MTU 1500 and IPv4, the UDP payload budget is about 1500 - 20 (IPv4 header) - 8 (UDP header) = 1472 bytes. CoAP headers/options reduce this further.

    • If we truly need to support large payloads in CoAP over UDP, RFC 7959 (Block-Wise Transfer, especially Block2) is typically required, and that also means the client must support RFC 7959.

    • Since this library is intended as a “basic” CoAP implementation, I’m leaning toward treating IP-fragmentation handling / RFC 7959 support as out of scope.

    • Given these constraints, it may also be safer to define a reasonable maximum resp_buffer size (or at least provide a configurable compile-time cap). This cap would represent a trade-off between avoiding oversized UDP datagrams (which may rely on IP fragmentation) and keeping RAM usage reasonable on constrained IoT devices. It would be to keep the default response buffer small (e.g., on the order of 128 bytes), and allow it to be increased up to a conservative upper bound (e.g., ~1200 bytes) via an explicit opt-in mechanism.

  • Implementation safety (memory)

    • setResponseBufferSize() currently does delete[] → new[], which may lose resp_buffer on allocation failure. Also, reallocating during request handling can contribute to heap fragmentation and latency.

    • Since this PR already allows specifying resp_buf_size in the constructor, it would be safer to encourage “allocate once at startup” usage (e.g., Coap(udp, coap_buf_size, resp_buf_size)). If the setter remains, it would be safer to allocate a new buffer first and swap only on success.

@trishantpahwa
Copy link
Copy Markdown
Contributor Author

If you check the file changes, it only refers to coap-simple.cpp, coap-simple.h and examples/esp8266/esp8266.ino. So I think the resolution has no conflict with the latest version 1.3.29. Also, it contains the updated changes as well.

The default size of buffer that is used to send as a response is 128 bytes, but the function setResponseBufferSize just helps in adjusting, when it can be used to send large content, as per need.

@hirotakaster
Copy link
Copy Markdown
Owner

Yes, I understand the increasing the response buffer via setResponseBufferSize() (e.g., to support larger responses when needed). However, once the buffer is increased, it also becomes easy to generate multi‑KB CoAP responses as a single UDP datagram, which may exceed the path MTU and rely on IP fragmentation—this can be unreliable depending on the network.

If large payload support is a real requirement, RFC 7959 (Block‑Wise, e.g. Block2) is typically needed, and that also requires CoAP server/client implementation support have to need.

@trishantpahwa
Copy link
Copy Markdown
Contributor Author

Got it. How about setting a warning if the response size exceeds the maximum limit? Currently, it fails silently.

@hirotakaster
Copy link
Copy Markdown
Owner

I think it would be better to fail explicitly (e.g., return an error) when the response exceeds the configured maximum.
If it is already known that the packet is oversized and likely to rely on IP fragmentation, sending it anyway means knowingly sending data that may be unreliable.

@hirotakaster
Copy link
Copy Markdown
Owner

@trishantpahwa

@trishantpahwa
Copy link
Copy Markdown
Contributor Author

Sure. I am unable to resolve the conflicts. Can you do the same please.

Thanks.

@hirotakaster
Copy link
Copy Markdown
Owner

@trishantpahwa
Hi! I've rebased your changes onto the latest master and resolved the conflicts.
You can find them in the https://github.com/hirotakaster/CoAP-simple-library/tree/pr-47-rebased branch of this repository.
Please take a look!

@trishantpahwa
Copy link
Copy Markdown
Contributor Author

Hi Please merge the pull request after verifying the proper functionality of the response being sent, as I have resolved the conflicts.

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

Successfully merging this pull request may close these issues.

2 participants