mgmt: mcumgr: transport: Move BM UART code out into transport file #395
Conversation
nordicjm
requested review from
bjorn-tore-taraldsen,
michalek-no and
nvlsianpu
github-actions
bot
added
the
changelog-entry-required
|
You can find the documentation preview for this PR here. |
nordicjm
force-pushed
the
uartmcumgr
branch
from
1a5771d to
53dae56
Compare
| @@ -0,0 +1,66 @@ | |||
| /* | |||
There was a problem hiding this comment.
We need documentation for this.
There was a problem hiding this comment.
Will need to be taken care of by @michalek-no or @nvlsianpu or others in a later PR
Allows including the BM UART transport in other applications and updates the UART MCUmgr sample to use this transport library Signed-off-by: Jamie McCrae <[email protected]>
There was a problem hiding this comment.
Looks good! Left some minor comments.
Is file https://github.com/nordicjm/sdk-nrf-bm/blob/uartmcumgr/samples/mcumgr/ble_mcumgr/mcumgr_handler.ld still used?
| static enum mgmt_cb_return os_mgmt_reboot_hook(uint32_t event, enum mgmt_cb_return prev_status, | ||
| int32_t *rc, uint16_t *group, bool *abort_more, | ||
| void *data, size_t data_size); |
There was a problem hiding this comment.
Could get rid of this function declaration if moving the os_mgmt_reboot_callback definition under the function definition. Up to you what you think is the cleanest.
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/smp.c | ||
| ^nrf-bm/subsys/mgmt/mcumgr/util/CMakeLists.txt | ||
| ^nrf-bm/sysbuild/Kconfig.bm | ||
| ^nrf-bm/samples/mcumgr/uart_mcumgr/src/smp_uart.c | ||
| ^nrf-bm/samples/mcumgr/uart_mcumgr/src/uart_mcumgr.c | ||
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/bm_uart_mcumgr.c | ||
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/smp_uart.c |
There was a problem hiding this comment.
Could sort these after the path change.
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/smp.c | |
| ^nrf-bm/subsys/mgmt/mcumgr/util/CMakeLists.txt | |
| ^nrf-bm/sysbuild/Kconfig.bm | |
| ^nrf-bm/samples/mcumgr/uart_mcumgr/src/smp_uart.c | |
| ^nrf-bm/samples/mcumgr/uart_mcumgr/src/uart_mcumgr.c | |
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/bm_uart_mcumgr.c | |
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/smp_uart.c | |
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/bm_uart_mcumgr.c | |
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/smp.c | |
| ^nrf-bm/subsys/mgmt/mcumgr/transport/src/smp_uart.c | |
| ^nrf-bm/subsys/mgmt/mcumgr/util/CMakeLists.txt | |
| ^nrf-bm/sysbuild/Kconfig.bm |
| CONFIG_SOC_FLASH_NRF_RRAM=y | ||
| CONFIG_SOC_FLASH_NRF_RRAM_BM=n | ||
| CONFIG_BM_UARTE_CONSOLE=n | ||
| CONFIG_PRINTK=n |
There was a problem hiding this comment.
I see the Bare Metal boot banner config (default y) selects PRINTK, triggering a build warning when trying to set PRINTK=n. Can disable the Bare Metal boot banner here in addition to PRINTK.
| CONFIG_PRINTK=n | |
| CONFIG_NCS_BARE_METAL_BOOT_BANNER=n |
5 participants
Allows including the BM UART transport in other applications and updates the UART MCUmgr sample to use this transport library