download_client: Allow use of DTLS CID #11493

SeppoTakalo · 2023-06-12T13:20:08Z

Few changes to the download client:

When connecting to DTLS service, enable Connection-ID. Controlled by Kconfig.
Refactor to use Zephyr socket API instead of POSIX socket API. This is easier regarding headers.
Clear CoAP timeout values on start of transmission.

NordicBuilder · 2023-06-12T13:21:50Z

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-ci-nrfconnect-boot-fw-update	X
test-fw-nrfconnect-nrf-iot_mosh	X
test-fw-nrfconnect-nrf-iot_samples	X
test-fw-nrfconnect-nrf-iot_serial_lte_modem	X
test-fw-nrfconnect-nrf-iot_zephyr_lwm2m	X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2023-06-12T13:59:42Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

umapraseeda · 2023-06-13T07:24:41Z

Are there any updates required in the library documentation?

rlubos

LGTM but please rebase

SeppoTakalo · 2023-06-13T09:32:47Z

Are there any updates required in the library documentation?

actually, you are right. Kconfig options are not mentioned in documentation, so I added a short note there as well.

SeppoTakalo · 2023-06-13T09:33:15Z

rebased.

doc/nrf/libraries/networking/download_client.rst

doc/nrf/releases/release-notes-changelog.rst

When connecting to DTLS service, enable Connection-ID. Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>

When transmission of a new block starts, clear timeouts. Otherwise previous CoAP transmission timeouts might be left. Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>

Using POSIX API directly brings some portability issues with headers, so it is safer to use zsock API on libraries. Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>

SeppoTakalo · 2023-06-26T11:46:57Z

Build failures on test application:

INFO: /tools/ncs/toolchains/fcef6322b4224bba80652b6ea86eebc963247c5d/usr/local/lib/python3.8/site-packages/ctf/targetcomm2/target_lib/tca_transports/uarte.c:20:9: note: '#pragma message: TCA_TRANSPORT=UART will be deprecated, it is recommended to use UART_NRFX for new code'

INFO:    20 | #pragma message "TCA_TRANSPORT=UART will be deprecated, it is recommended to use UART_NRFX for new code"

INFO:       |         ^~~~~~~

INFO: [93/324] Building C object CMakeFiles/app.dir/jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/subsys/modem_funcs/modem_funcs.c.obj

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/subsys/modem_funcs/modem_funcs.c: In function 'enable_and_connect_modem':

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/subsys/modem_funcs/modem_funcs.c:72:5: warning: implicit declaration of function 'nrf_modem_init'; did you mean 'nrf_modem_at_err'? [-Wimplicit-function-declaration]

INFO:    72 |     nrf_modem_init();

INFO:       |     ^~~~~~~~~~~~~~

INFO:       |     nrf_modem_at_err

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/subsys/modem_funcs/modem_funcs.c: At top level:

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/subsys/modem_funcs/modem_funcs.c:151:54: warning: 'enum at_cmd_state' declared inside parameter list will not be visible outside of this definition or declaration

INFO:   151 | int at_read_out_uuid(char *buf, size_t buf_len, enum at_cmd_state *state){

INFO:       |                                                      ^~~~~~~~~~~~

INFO: [94/324] Building C object CMakeFiles/app.dir/src/main.c.obj

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c: In function 'dfu_target_callback_handler':

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c:62:5: warning: implicit declaration of function 'send_evt' [-Wimplicit-function-declaration]

INFO:    62 |     send_evt(FOTA_DOWNLOAD_EVT_ERASE_PENDING);

INFO:       |     ^~~~~~~~

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c:68:5: warning: implicit declaration of function 'send_error_evt' [-Wimplicit-function-declaration]

INFO:    68 |     send_error_evt(FOTA_DOWNLOAD_ERROR_CAUSE_DOWNLOAD_FAILED);

INFO:       |     ^~~~~~~~~~~~~~

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c: In function 'start_download':

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c:82:61: warning: passing argument 4 of 'fota_download_start' makes integer from pointer without a cast [-Wint-conversion]

INFO:    82 |         int ret =  fota_download_start(host, file, sec_tag, NULL, fragment_size);

INFO:       |                                                             ^~~~

INFO:       |                                                             |

INFO:       |                                                             void *

INFO: In file included from /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c:3:

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/nrf/include/net/fota_download.h:126:33: note: expected 'uint8_t' {aka 'unsigned char'} but argument is of type 'void *'

INFO:   126 |                         uint8_t pdn_id, size_t fragment_size);

INFO:       |                         ~~~~~~~~^~~~~~

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c: In function 'init_dfu_target_params_oversized_buffer':

INFO: /jenkins_workspace/workspace/nrfconnect-boot-fw-update_master/test_fw_update/utils/test_applications/application/fw_download/src/main.c:120:9: warning: unused variable 'img_type' [-Wunused-variable]

INFO:   120 |     int img_type;

INFO:       |         ^~~~~~~~

INFO: [95/324] Building C object zephyr/CMakeFiles/zephyr.dir/lib/os/timeutil.c.obj

INFO: [96/324] Building C object zephyr/CMakeFiles/zephyr.dir/lib/os/thread_entry.c.obj

INFO: [97/324] Building C object zephyr/CMakeFiles/zephyr.dir/lib/os/printk.c.obj

INFO: [98/324] Building C object zephyr/CMakeFiles/zephyr.dir/lib/os/fdtable.c.obj

INFO: [99/324] Building C object zephyr/CMakeFiles/zephyr.dir/lib/os/rb.c.obj

INFO: [100/324] Building C object zephyr/CMakeFiles/zephyr.dir/lib/os/cbprintf_packaged.c.obj

INFO: [101/324] Building C object zephyr/CMakeFiles/zephyr.dir/lib/os/heap.c.obj

INFO: ninja: build stopped: subcommand failed.

Not sure who is responsible of fixing test-ci-nrfconnect-boot-fw-update test job and its related test app.

stig-bjorlykke · 2023-06-27T16:43:45Z

What is the use-case for DTLS CID in the download client which doesn't keep the connection?

lemrey · 2023-06-28T11:39:52Z

subsys/net/lib/download_client/src/download_client.c

@@ -134,7 +128,7 @@ static int set_recv_socket_timeout(int fd, int timeout_ms)
 .tv_usec = (timeout_ms % 1000) * 1000,
 };

- err = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
+ err = zsock_setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));


This is fine in principle, but we should try to keep this aligned throughout the codebase, at least in sdk-nrf. It would be a bit messy if some libraries used zsock_ and others the POSIX names; what guideline do we set for ourselves? Is it up to personal preference which one to use?

SeppoTakalo requested review from lemrey, gmarull and stig-bjorlykke June 12, 2023 13:20

SeppoTakalo requested a review from rlubos as a code owner June 12, 2023 13:20

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jun 12, 2023

SeppoTakalo force-pushed the download_client_cid branch from 4c20f0e to 995e25e Compare June 12, 2023 13:23

SeppoTakalo requested a review from carlescufi as a code owner June 12, 2023 13:23

github-actions bot added doc-required PR must not be merged without tech writer approval. and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Jun 12, 2023

SeppoTakalo requested a review from umapraseeda June 13, 2023 07:21

umapraseeda approved these changes Jun 13, 2023

View reviewed changes

SeppoTakalo removed the doc-required PR must not be merged without tech writer approval. label Jun 13, 2023

rlubos approved these changes Jun 13, 2023

View reviewed changes

SeppoTakalo force-pushed the download_client_cid branch from 995e25e to 3049b7a Compare June 13, 2023 09:31

github-actions bot added the doc-required PR must not be merged without tech writer approval. label Jun 13, 2023

SeppoTakalo force-pushed the download_client_cid branch from 3049b7a to b441cad Compare June 13, 2023 09:32

umapraseeda reviewed Jun 13, 2023

View reviewed changes

doc/nrf/libraries/networking/download_client.rst Outdated Show resolved Hide resolved

doc/nrf/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

SeppoTakalo mentioned this pull request Jun 13, 2023

application: serial_lte_modem: Support DTLS connection ID #11244

Closed

SeppoTakalo force-pushed the download_client_cid branch from e48e265 to bedf7db Compare June 13, 2023 09:38

SeppoTakalo added 3 commits June 26, 2023 12:50

net: lib: download_client: Allow use of DTLS CID

5a7ff07

When connecting to DTLS service, enable Connection-ID. Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>

net: lib: download_client: Clear CoAP timeouts on init

018f8ef

When transmission of a new block starts, clear timeouts. Otherwise previous CoAP transmission timeouts might be left. Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>

net: lib: download_client: Refactor to use zsock API, not POSIX

0bce520

Using POSIX API directly brings some portability issues with headers, so it is safer to use zsock API on libraries. Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>

SeppoTakalo force-pushed the download_client_cid branch from bedf7db to 0bce520 Compare June 26, 2023 09:50

SeppoTakalo removed the doc-required PR must not be merged without tech writer approval. label Jun 26, 2023

lemrey reviewed Jun 28, 2023

View reviewed changes

SeppoTakalo closed this Aug 23, 2023

SeppoTakalo deleted the download_client_cid branch August 23, 2023 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

download_client: Allow use of DTLS CID #11493

download_client: Allow use of DTLS CID #11493

SeppoTakalo commented Jun 12, 2023

NordicBuilder commented Jun 12, 2023 •

edited

Loading

NordicBuilder commented Jun 12, 2023

umapraseeda commented Jun 13, 2023

rlubos left a comment

SeppoTakalo commented Jun 13, 2023

SeppoTakalo commented Jun 13, 2023

SeppoTakalo commented Jun 26, 2023

stig-bjorlykke commented Jun 27, 2023

lemrey Jun 28, 2023

download_client: Allow use of DTLS CID #11493

download_client: Allow use of DTLS CID #11493

Conversation

SeppoTakalo commented Jun 12, 2023

NordicBuilder commented Jun 12, 2023 • edited Loading

Test specification

CI/Jenkins/NRF

CI/Jenkins/integration

NordicBuilder commented Jun 12, 2023

umapraseeda commented Jun 13, 2023

rlubos left a comment

Choose a reason for hiding this comment

SeppoTakalo commented Jun 13, 2023

SeppoTakalo commented Jun 13, 2023

SeppoTakalo commented Jun 26, 2023

stig-bjorlykke commented Jun 27, 2023

lemrey Jun 28, 2023

Choose a reason for hiding this comment

NordicBuilder commented Jun 12, 2023 •

edited

Loading