app: Add CoAP support to nRF Cloud AT commands#174
app: Add CoAP support to nRF Cloud AT commands#174juhaylinen wants to merge 2 commits intonrfconnect:mainfrom
Conversation
89fa7ae to
661f320
Compare
661f320 to
a6609d2
Compare
2c4cd7a to
c61d096
Compare
c61d096 to
138ba28
Compare
| { | ||
| int err; | ||
| struct nrf_modem_gnss_agnss_data_frame req; | ||
| static char agnss_rest_data_buf[NRF_CLOUD_AGNSS_MAX_DATA_SIZE]; |
There was a problem hiding this comment.
This is 4kB buffer. This is a bit more general question on how we should deal with these. We use quite a lot of RAM for a feature that some/most hosts don't use at all.
| .wifi_info = nrfcloud_wifi_pos ? &nrfcloud_wifi_data : NULL}; | ||
|
|
||
| if (err) { | ||
| err = nrf_cloud_coap_location_get(&request, &result); |
There was a problem hiding this comment.
This is synchronous operation. Can this be a problem? nrf_cloud_location_request was asynchronous, right?
I guess this discussion is valid for all nrf_cloud_coap operations.
There was a problem hiding this comment.
Yeah, nrf_cloud_coap operations are synchronous.
app/src/sm_at_nrfcloud.c
Outdated
| LOG_ERR("Failed to allocate memory for message copy"); | ||
| return -ENOMEM; | ||
| } | ||
| memcpy(msg_copy, message, len); |
There was a problem hiding this comment.
Would be nice to avoid this copy. Is it just for the NULL termination?
There was a problem hiding this comment.
Yes, it's just for the NULL terminator.
There was a problem hiding this comment.
I think we can avoid the copy by using direct call to nrf_cloud_coap_post()
| default y | ||
| select EXPERIMENTAL | ||
|
|
||
| config SM_NRF_CLOUD_LOCATION |
There was a problem hiding this comment.
I'd like to put this on by default if CONFIG_SM_NRF_CLOUD is on but that wouldn't be backwards compatible. As it's not such a big deal, I guess we'll just leve if OFF by default and enable in prj.conf as done earlier for CONFIG_NRF_CLOUD_LOCATION.
Add help text for this though as it's not clear what parts of nRF Cloud location functionality this means. I.e., is it cell and wifi positioning or A-GNSS or P-GPS. Some or all of them.
doc/app/sm_configuration.rst
Outdated
| CONFIG_SM_NRF_CLOUD_LOCATION - Send GNSS location to nRF Cloud | ||
| This option enables automatic sending of GNSS location data to nRF Cloud when a fix is acquired. |
There was a problem hiding this comment.
Isn't this about cell and wifi positioning. Sending of GNSS location to cloud is controlled by run-time <send_location> parameter in AT#XNRFCLOUD
8a15a37 to
99cbbe1
Compare
doc/app/sm_configuration.rst
Outdated
| .. _CONFIG_SM_NRF_CLOUD_LOCATION: | ||
|
|
||
| CONFIG_SM_NRF_CLOUD_LOCATION - nRF Cloud Location support | ||
| This option enables the nRF Cloud Location service for cloud-assisted geolocation. |
There was a problem hiding this comment.
| This option enables the nRF Cloud Location service for cloud-assisted geolocation. | |
| This option enables the `nRF Cloud Location Services <nRF Cloud Location Services documentation_>`_ for cloud-assisted geolocation. |
nRF Cloud Location Services documentation - Could link to this page - https://docs.nordicsemi.com/bundle/nrf-cloud/page/LocationServices/LSOverview.html
doc/app/at_nrfcloud.rst
Outdated
|
|
||
| * The device must be connected to nRF Cloud using :ref:`#XNRFCLOUD <SM_AT_NRFCLOUD>`. | ||
| * The ``CONFIG_NRF_CLOUD_LOCATION`` Kconfig option must be enabled. | ||
| * The ``CONFIG_SM_NRF_CLOUD_LOCATION`` Kconfig option must be enabled. |
There was a problem hiding this comment.
| * The ``CONFIG_SM_NRF_CLOUD_LOCATION`` Kconfig option must be enabled. | |
| * The :ref:`CONFIG_SM_NRF_CLOUD_LOCATION <CONFIG_SM_NRF_CLOUD_LOCATION>` Kconfig option must be enabled. |
doc/migration_notes.rst
Outdated
| * Functions and other symbols in the code have been renamed accordingly making automatic patching to likely fail. | ||
|
|
||
| * Changed the default AT command terminator from ``\r\n`` (``CONFIG_SM_CR_LF_TERMINATION`` and ``CONFIG_SM_AT_CLIENT_CR_LF_TERMINATION``) to ``\r`` (``CONFIG_SM_CR_TERMINATION`` and ``CONFIG_SM_AT_CLIENT_CR_TERMINATION``). | ||
| * Changed the nRF Cloud Location Kconfig option from ``CONFIG_NRF_CLOUD_LOCATION`` to |SM| specific ``CONFIG_SM_NRF_CLOUD_LOCATION``. |
There was a problem hiding this comment.
| * Changed the nRF Cloud Location Kconfig option from ``CONFIG_NRF_CLOUD_LOCATION`` to |SM| specific ``CONFIG_SM_NRF_CLOUD_LOCATION``. | |
| * Changed the nRF Cloud Location Kconfig option from ``CONFIG_NRF_CLOUD_LOCATION`` to |SM|-specific :ref:`CONFIG_SM_NRF_CLOUD_LOCATION <CONFIG_SM_NRF_CLOUD_LOCATION>`. |
doc/migration_notes.rst
Outdated
|
|
||
| * Changed the default AT command terminator from ``\r\n`` (``CONFIG_SM_CR_LF_TERMINATION`` and ``CONFIG_SM_AT_CLIENT_CR_LF_TERMINATION``) to ``\r`` (``CONFIG_SM_CR_TERMINATION`` and ``CONFIG_SM_AT_CLIENT_CR_TERMINATION``). | ||
| * Changed the nRF Cloud Location Kconfig option from ``CONFIG_NRF_CLOUD_LOCATION`` to |SM| specific ``CONFIG_SM_NRF_CLOUD_LOCATION``. | ||
| This option is now used to enable the nRF Cloud Location service for cloud-assisted geolocation, which supports A-GNSS, P-GPS, and Wi-Fi positioning. |
There was a problem hiding this comment.
| This option is now used to enable the nRF Cloud Location service for cloud-assisted geolocation, which supports A-GNSS, P-GPS, and Wi-Fi positioning. | |
| You can now use this option to enable the `nRF Cloud Location Services <nRF Cloud Location Services documentation_>`_ for cloud-assisted geolocation, which supports A-GNSS, P-GPS, and Wi-Fi positioning. |
Add overlay to enable CoAP connectivity for nRF Cloud AT commands. Use CONFIG_SM_NRF_CLOUD_LOCATION to enable location support, since CONFIG_NRF_CLOUD_LOCATION depends on NRF_CLOUD_MQTT and cannot be enabled when using NRF_CLOUD_COAP. Jira: SM-235 Signed-off-by: Juha Ylinen <juha.ylinen@nordicsemi.no>
Use more lightweight CoAP protocol instead of MQTT. Signed-off-by: Juha Ylinen <juha.ylinen@nordicsemi.no>
99cbbe1 to
ebf0f5c
Compare
Remove support for nRF Cloud MQTT protocol and use more lightweight CoAP connectivity.
Use CONFIG_SM_NRF_CLOUD_LOCATION to enable location support, since CONFIG_NRF_CLOUD_LOCATION depends on NRF_CLOUD_MQTT and cannot be enabled when using NRF_CLOUD_COAP.
Jira: SM-235