app: Add CoAP client AT commands#283
Conversation
2c9c80c to
bfc5317
Compare
5cfcffc to
dde17cb
Compare
|
You can find the documentation preview for this PR here. |
dde17cb to
0072bde
Compare
There was a problem hiding this comment.
Pull request overview
Adds three new AT commands (AT#XCOAPCREQ, AT#XCOAPCDATA, AT#XCOAPCCANCEL) that expose Zephyr's coap_client library over the Serial Modem UART, allowing the host to perform asynchronous CoAP requests on an existing UDP/DTLS socket created via AT#XSOCKET/AT#XSSOCKET. Includes auto and manual response delivery modes, optional CoAP options (with hex/text auto-detection), and small/large request payload paths (in-buffer or streamed via payload_cb).
Changes:
- New CoAP client backend in
app/src/sm_at_coap.cplus Kconfig (SM_COAP) and CMake wiring. prj.confupdates to allow multiplecoap_clientinstances; also enablesSM_AUTO_CONNECTand adds commented logging hints.- New documentation page
doc/app/at_coap.rst(linked fromat_commands.rst).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/sm_at_coap.c | Implementation of the three new AT commands and their request lifecycle. |
| app/CMakeLists.txt | Conditionally compiles the new CoAP source file. |
| app/Kconfig | Adds CONFIG_SM_COAP Kconfig entry (default y, selects COAP/COAP_CLIENT). |
| app/prj.conf | Bumps COAP_CLIENT_MAX_INSTANCES; also enables SM_AUTO_CONNECT and adds debug-log comments. |
| doc/app/at_coap.rst | New AT command reference for XCOAPCREQ/XCOAPCDATA/XCOAPCCANCEL. |
| doc/app/at_commands.rst | Adds toctree entry for the new CoAP page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
58a3c26 to
22a8e7c
Compare
MarkusLassila
left a comment
There was a problem hiding this comment.
Some comments. Continuing on Monday.
| } | ||
|
|
||
| req->nrf_handle = handle; | ||
| req->zsock_fd = nrf_modem_lib_nrf_fd_to_zsock_fd(handle); |
There was a problem hiding this comment.
Is this something that you have implemented yourself?
There was a problem hiding this comment.
Yes, it's used for testing the client and will be removed before the PR is merged.
22a8e7c to
edd552d
Compare
MarkusLassila
left a comment
There was a problem hiding this comment.
Looks good. Some comments.
| } else if (op == DATAMODE_EXIT) { | ||
| struct coap_request *req = coap_pending_req; | ||
|
|
||
| if (req && req->payload_len <= CONFIG_COAP_CLIENT_BLOCK_SIZE) { |
There was a problem hiding this comment.
Is there a way to make the both of these to use the longer way of making the request? There are many places where there is separate logic for the two of these.
| * #XCOAPCSTAT: <handle>,<status>,<total_bytes> | ||
| * | ||
| * <handle>: AT socket handle from AT#XSOCKET or AT#XSSOCKET. | ||
| * <status>: CoAP response code (e.g. 69 for 2.05 Content); -1 on error/cancel. |
There was a problem hiding this comment.
This maps to response, not the request. The others in here are specific to request.
6100424 to
caed1a4
Compare
caed1a4 to
60688cb
Compare
SeppoTakalo
left a comment
There was a problem hiding this comment.
Code Review
🔴 Bugs
1. opt_num type mismatch with at_parser_num_get (sm_at_coap.c ~line 730)
opt_num is declared uint16_t and passed by address to at_parser_num_get. Every other caller in the codebase (sm_at_socket.c, sm_at_httpc.c, sm_at_mqtt.c) passes int *. If the type-generic macro doesn't handle uint16_t *, the parser writes 4 bytes into a 2-byte location and corrupts adjacent stack variables (opt, decoded_len, …). Fix: use a local int tmp_num, pass &tmp_num, and assign opt->code = (uint16_t)tmp_num after range-checking.
🔴 Unrelated behavior change in prj.conf
CONFIG_SM_AUTO_CONNECT=y modifies default product behavior (auto network attach on boot) and has nothing to do with CoAP. Likewise, the commented-out CONFIG_NET_LOG / CONFIG_COAP_LOG_LEVEL_INF lines look like leftover debug artifacts. Both should be removed before merge.
🟡 Design concerns
extern struct sm_socket *find_socket(int fd) without a header
find_socket is an internal symbol of sm_at_socket.c, and this PR accesses it via a bare extern declaration without adding it to sm_at_socket.h. This is fragile — the declaration can silently drift from the definition. Add it to the header.
CONFIG_COAP_CLIENT_MAX_INSTANCES=3 in overlay-nrf-device-provisioning.conf
The comment says "support simultaneous provisioning and AT#XCOAPCREQ", implying 2 instances are needed. Why 3? Either bump with justification (e.g. "one spare for future expansion") or reduce to 2 with a matching comment.
CONFIG_SM_COAP defaults to y
The symbol selects EXPERIMENTAL and brings in the coap_client library in all default builds. Given it is experimental, consider defaulting to n (like CONFIG_SM_GNSS, CONFIG_SM_SMS, CONFIG_SM_PPP). If intentionally always-on, that is fine but worth an explicit note.
🟡 PR description out of sync with code
The overview examples still show bare hex strings without a prefix:
AT#XCOAPCREQ=0,"/obs",1,1,1,0,0,6,"00",17,"3c"
But the code (and RST docs) now require a 0x prefix ("0x00", "0x3c"). Update the PR description so reviewers are not confused.
🟢 What looks good
- Overall architecture (single in-flight request, two receive modes, small/large payload split via staging buffer + semaphores) is clean and well-documented in both the struct comment and per-function docstrings.
- Loop bound is correct:
(i + 1) < param_countafter the odd-count rejection guard. - Cancel command now documents the "single shared client" limitation directly in the code comment.
parse_option_valueis self-contained and readable. The0x-prefix convention is unambiguous and easier to use than an auto-detect heuristic.- RST documentation is thorough with multiple complete examples (including the full
AT#XSOCKET+AT#XCONNECTflow).
❌ CI
The Build/build-and-test-in-toolchain-bundle check is failing. Please investigate and fix before requesting a re-review.
160b1f6 to
51d5856
Compare
38a9fec to
42089e9
Compare
| * Set socket options with ``AT#XSOCKETOPT`` or ``AT#XSSOCKETOPT``. | ||
| * Close with ``AT#XCLOSE``. | ||
|
|
||
| Only one CoAP request may be active at a time across all sockets. |
There was a problem hiding this comment.
| Only one CoAP request may be active at a time across all sockets. | |
| Only one CoAP request might be active at a time across all sockets. |
| ----------- | ||
|
|
||
| The set command sends a CoAP request and returns ``OK`` immediately. | ||
| The server's response is delivered via unsolicited ``#XCOAPCDATA`` and ``#XCOAPCSTAT`` notifications. |
There was a problem hiding this comment.
| The server's response is delivered via unsolicited ``#XCOAPCDATA`` and ``#XCOAPCSTAT`` notifications. | |
| The server's response is delivered through unsolicited ``#XCOAPCDATA`` and ``#XCOAPCSTAT`` notifications. |
| It can accept the following values: | ||
|
|
||
| * ``1`` - Automatic mode (default). | ||
| Response bytes are forwarded to the host automatically via ``#XCOAPCDATA`` URCs. |
There was a problem hiding this comment.
| Response bytes are forwarded to the host automatically via ``#XCOAPCDATA`` URCs. | |
| Response bytes are forwarded to the host automatically through ``#XCOAPCDATA`` URCs. |
| * ``0`` - Binary (default). | ||
| Response payload bytes are forwarded to the host as raw binary. | ||
| * ``1`` - Hex string. | ||
| Response payload bytes are encoded as a lower-case ASCII hex string before delivery. |
There was a problem hiding this comment.
| Response payload bytes are encoded as a lower-case ASCII hex string before delivery. | |
| Response payload bytes are encoded as a lowercase ASCII hex string before delivery. |
|
|
||
| The host must send exactly this many bytes as the request payload. | ||
| Data mode exits and ``#XDATAMODE: 0`` is reported when all bytes have been consumed. | ||
| Data mode is the only mechanism for supplying the payload — there is no inline parameter alternative. |
There was a problem hiding this comment.
| Data mode is the only mechanism for supplying the payload — there is no inline parameter alternative. | |
| Data mode is the only mechanism for supplying the payload - there is no inline parameter alternative. |
| * The ``<opt_num_X>,<opt_val_X>`` parameters are optional CoAP option pairs. | ||
| When present, ``<payload_len>`` must also be present as the positional separator before the first pair. | ||
| ``<opt_num_X>`` is a decimal integer option number. | ||
| ``<opt_val_X>`` is a quoted string. | ||
| A value prefixed with ``0x`` or ``0X`` is decoded as raw hex bytes (for example ``"0x3c"`` → byte ``0x3c``). | ||
| The hex part after the prefix must be non-empty and even-length, otherwise the command returns ``ERROR``. | ||
| Any value without the ``0x`` prefix is used verbatim, including strings that look like hex (for example ``"abcd"`` → 4 ASCII bytes). | ||
| Common option numbers: | ||
|
|
||
| * ``6`` - Observe (``"0x00"`` = register, ``"0x01"`` = deregister). | ||
| * ``17`` - Accept (content format the client will accept, hex-encoded). | ||
| * ``35`` - Proxy-Uri (full URI passed to a proxy, verbatim string). |
There was a problem hiding this comment.
| * The ``<opt_num_X>,<opt_val_X>`` parameters are optional CoAP option pairs. | |
| When present, ``<payload_len>`` must also be present as the positional separator before the first pair. | |
| ``<opt_num_X>`` is a decimal integer option number. | |
| ``<opt_val_X>`` is a quoted string. | |
| A value prefixed with ``0x`` or ``0X`` is decoded as raw hex bytes (for example ``"0x3c"`` → byte ``0x3c``). | |
| The hex part after the prefix must be non-empty and even-length, otherwise the command returns ``ERROR``. | |
| Any value without the ``0x`` prefix is used verbatim, including strings that look like hex (for example ``"abcd"`` → 4 ASCII bytes). | |
| Common option numbers: | |
| * ``6`` - Observe (``"0x00"`` = register, ``"0x01"`` = deregister). | |
| * ``17`` - Accept (content format the client will accept, hex-encoded). | |
| * ``35`` - Proxy-Uri (full URI passed to a proxy, verbatim string). | |
| * The ``<opt_num_X>,<opt_val_X>`` parameters are optional CoAP option pairs: | |
| * When present, ``<payload_len>`` must also be present as the positional separator before the first pair. | |
| * ``<opt_num_X>`` is a decimal integer option number. | |
| * ``<opt_val_X>`` is a quoted string. | |
| * A value prefixed with ``0x`` or ``0X`` is decoded as raw hex bytes (for example, ``"0x3c"`` → byte ``0x3c``). | |
| * The hex part after the prefix must be non-empty and even-length, otherwise, the command returns ``ERROR``. | |
| * Any value without the ``0x`` prefix is used verbatim, including strings that look like hex (for example ``"abcd"`` → 4 ASCII bytes). | |
| * Common option numbers: | |
| * ``6`` - Observe (``"0x00"`` = register, ``"0x01"`` = deregister). | |
| * ``17`` - Accept (content format the client will accept, hex-encoded). | |
| * ``35`` - Proxy-Uri (full URI passed to a proxy, verbatim string). |
For readability.
| In hex mode (``<hex_format>=1``), ``AT#XCOAPCDATA`` will deliver ``2×<block_len>`` ASCII hex characters. | ||
| The host must call ``AT#XCOAPCDATA`` to retrieve this block before the next one arrives. | ||
|
|
||
| ``#XCOAPCSTAT`` is emitted when the request completes, fails, or is cancelled |
There was a problem hiding this comment.
| ``#XCOAPCSTAT`` is emitted when the request completes, fails, or is cancelled | |
| ``#XCOAPCSTAT`` is emitted when the request completes, fails, or is cancelled. |
|
|
||
| #XCOAPCSTAT: 0,68,0 | ||
|
|
||
| CoAP GET via proxy with Proxy-Uri option (option 35, value is a plain URI string): |
There was a problem hiding this comment.
| CoAP GET via proxy with Proxy-Uri option (option 35, value is a plain URI string): | |
| CoAP GET through proxy with Proxy-URI option (option 35, value is a plain URI string): |
| Response syntax | ||
| ~~~~~~~~~~~~~~~ | ||
|
|
||
| When a block is available |
There was a problem hiding this comment.
| When a block is available | |
| When a block is available: |
| ``<length>`` raw response bytes follow immediately with no additional separator. | ||
| ``OK`` follows on its own line after the response bytes. | ||
|
|
||
| When no block has been buffered yet (the firmware is still waiting for the server response) |
There was a problem hiding this comment.
| When no block has been buffered yet (the firmware is still waiting for the server response) | |
| When no block has been buffered yet (the firmware is still waiting for the server response). |
42089e9 to
b45c939
Compare
| * The ``<opt_num_X>,<opt_val_X>`` parameters are optional CoAP option pairs: | ||
| * When present, ``<payload_len>`` must also be present as the positional separator before the first pair. | ||
| * ``<opt_num_X>`` is a decimal integer option number. | ||
| * ``<opt_val_X>`` is a quoted string. | ||
| * A value prefixed with ``0x`` or ``0X`` is decoded as raw hex bytes (for example, ``"0x3c"`` → byte ``0x3c``). | ||
| * The hex part after the prefix must be non-empty and even-length, otherwise, the command returns ``ERROR``. | ||
| * Any value without the ``0x`` prefix is used verbatim, including strings that look like hex (for example ``"abcd"`` → 4 ASCII bytes). | ||
| * Common option numbers: | ||
| * ``6`` - Observe (``"0x00"`` = register, ``"0x01"`` = deregister). | ||
| * ``17`` - Accept (content format the client will accept, hex-encoded). | ||
| * ``35`` - Proxy-Uri (full URI passed to a proxy, verbatim string). |
There was a problem hiding this comment.
| * The ``<opt_num_X>,<opt_val_X>`` parameters are optional CoAP option pairs: | |
| * When present, ``<payload_len>`` must also be present as the positional separator before the first pair. | |
| * ``<opt_num_X>`` is a decimal integer option number. | |
| * ``<opt_val_X>`` is a quoted string. | |
| * A value prefixed with ``0x`` or ``0X`` is decoded as raw hex bytes (for example, ``"0x3c"`` → byte ``0x3c``). | |
| * The hex part after the prefix must be non-empty and even-length, otherwise, the command returns ``ERROR``. | |
| * Any value without the ``0x`` prefix is used verbatim, including strings that look like hex (for example ``"abcd"`` → 4 ASCII bytes). | |
| * Common option numbers: | |
| * ``6`` - Observe (``"0x00"`` = register, ``"0x01"`` = deregister). | |
| * ``17`` - Accept (content format the client will accept, hex-encoded). | |
| * ``35`` - Proxy-Uri (full URI passed to a proxy, verbatim string). | |
| * The ``<opt_num_X>,<opt_val_X>`` parameters are optional CoAP option pairs: | |
| * When present, ``<payload_len>`` must also be present as the positional separator before the first pair. | |
| * ``<opt_num_X>`` is a decimal integer option number. | |
| * ``<opt_val_X>`` is a quoted string. | |
| * A value prefixed with ``0x`` or ``0X`` is decoded as raw hex bytes (for example, ``"0x3c"`` → byte ``0x3c``). | |
| * The hex part after the prefix must be non-empty and even-length, otherwise, the command returns ``ERROR``. | |
| * Any value without the ``0x`` prefix is used verbatim, including strings that look like hex (for example ``"abcd"`` → 4 ASCII bytes). | |
| * Common option numbers: | |
| * ``6`` - Observe (``"0x00"`` = register, ``"0x01"`` = deregister). | |
| * ``17`` - Accept (content format the client will accept, hex-encoded). | |
| * ``35`` - Proxy-Uri (full URI passed to a proxy, verbatim string). |
A break after line no:117, to render correctly. also, after line no:124 - https://ncsbmdoc.z6.web.core.windows.net/addons/addon-serial_modem/PR-283/app/at_coap.html#syntax
f9492cb to
0006a63
Compare
Add AT#XCOAPCREQ, AT#XCOAPCCANCEL, and AT#XCOAPCDATA to drive Zephyr coap_client over a connected UDP/DTLS socket. Responses are delivered asynchronously via #XCOAPCDATA / #XCOAPCHEAD and #XCOAPCSTAT URCs. Jira: SM-225 Signed-off-by: Juha Ylinen <juha.ylinen@nordicsemi.no>
0006a63 to
1cbd48e
Compare
Overview
Adds three new AT commands that expose Zephyr's
coap_clientlibrary over the Serial Modem UARTinterface, enabling host devices to make CoAP requests over an existing DTLS or UDP socket.
New AT commands
AT#XCOAPCREQSends a CoAP request on the AT socket identified by
<handle>.<handle>AT#XSOCKET/AT#XSSOCKET<path>?key=val) are supported<method><auto_reception>AT#XCOAPCDATA<format><length>in URCs always reports the raw byte count<confirmable><content_format><payload_len>OKand enters data mode to receive the payload<opt_num><opt_val>. Zero or more pairs may be appended.<opt_val>0x/0Xis decoded as raw hex bytes (e.g."0x3c"→ byte0x3c); the hex part must be non-empty and even-length, otherwiseERRORis returned. Any value without the prefix is used verbatim.Returns
OKimmediately. Responses are delivered asynchronously:Auto receive mode (
auto_reception=1, default):#XCOAPCDATA: <handle>,<offset>,<len>followed by<len>raw bytes (binary) or2×<len>hex chars (hex mode), once per Block2 block#XCOAPCSTAT: <handle>,<status>,<total_bytes>on completion or error (status=-1;<total_bytes>is always the raw byte count)Manual receive mode (
auto_reception=0):#XCOAPCHEAD: <handle>,<code>,<len>— one URC per block (<len>is raw byte count); host must callAT#XCOAPCDATAto pull each block#XCOAPCSTAT: <handle>,<status>,<total_bytes>on completion or error (status=-1;<total_bytes>is always the raw byte count)Examples:
AT#XCOAPCCANCELCancels the active request on the given socket handle. The
coap_clientlibrary calls theresponse callback with a negative result code, which delivers a
#XCOAPCSTATerror URC beforeOKis returned.AT#XCOAPCDATAPulls one chunk of response body in manual receive mode (
auto_reception=0). Must be called afterreceiving a
#XCOAPCHEADURC. If<length>is smaller than the block size, repeat calls untilthe full block is drained — the firmware's CoAP callback remains blocked until
rx_bufis empty.<handle><length>CONFIG_COAP_CLIENT_BLOCK_SIZE)Response:
Payload handling
Two code paths depending on declared
<payload_len>:Small payloads (
≤ CONFIG_COAP_CLIENT_BLOCK_SIZE, default 1024 bytes):coap_start_request()is called on data mode exit withcoap_req.payloadpointing to thebuffer directly — no blockwise upload, no
payload_cb, no semaphores.coap_clientstores thepayloadpointer internally and re-reads it when reconstructingBlock2 continuation request packets.
Large payloads (
> CONFIG_COAP_CLIENT_BLOCK_SIZE):coap_client_payload_cb_t.thread and the
coap_clientbackground thread via two semaphores:staging_ready: data mode signals that staging is filledstaging_consumed:coap_clientsignals after copying staging to its send buffercoap_start_request()from within data mode;coap_client_req()calls
payload_cbsynchronously for block 0 before returning.coap_clientre-invokespayload_cbwithoffset=0. Detected bypayload_sent >= payload_len; cached staging contents are returnedwithout blocking.
Example
Jira: SM-225