Zephyr: mcumgr smp_svr sample not working over shell or serial transport

[bpbradley]

Describe the bug
Building a simple example for mcumgr is resulting in an NMP timeout on mcumgr cli tool when using shell or serial transport. Bluetooth untested. The shell is not showing up on a terminal either.

To Reproduce
Build and flash mcuboot

1. west build -b nrf52840dk_nrf52840 -d build_mcuboot bootloader/mcuboot/boot/zephyr
2. west flash -d build_mcuboot

Build sign and flash smp_srv

1. west build \
   -b nrf52840dk_nrf52840 \
   samples/subsys/mgmt/mcumgr/smp_svr \
   -- \
   -DOVERLAY_CONFIG='overlay-shell.conf'
2. west sign -t imgtool -- --key bootloader/mcuboot/root-rsa-2048.pem
3. west flash --hex-file build/zephyr/zephyr.signed.hex

Test mcumgr

1. mcumgr -c (serial conn) echo hello

Expected behavior
Successful echo, instead we get a timeout. Same happens without the shell overlay just using serial.

Tested on v2.3 and it works as expected (I get a shell available when I access it via terminal, and mcumgr correctly responds to echo as well as image list)

Also, trying to flash the signed bin using --bin-file instead of hex results in mcuboot unable to fine image0, but the documentation recommends this method.

Impact
showstopper unless there is a workaround

Environment (please complete the following information):

• OS: Tested Linux and Windows
• Toolchain gnuarmemb
• Commit SHA or Version: Tested 2.4.0-rc2 and latest master

Additional context
Tested with nrf52840dk. I had different issues with stm32 which some configurations resulted in Error: 8 instead of timeout, which I believe means it thinks the operation is unsupported.

[utzig]

Have you tried using -DOVERLAY_CONFIG='overlay-serial.conf' instead?

[bpbradley]

Have you tried using -DOVERLAY_CONFIG='overlay-serial.conf' instead?

Yes, same result both in the serial output from the terminal as well as the mcumgr cli timeout.

Here is the terminal output when it boots by the way

*** Booting Zephyr OS build v2.4.0-rc2-33-gc54a511d26d0  ***
[00:00:00.257,415] <inf> mcuboot: Starting bootloader
[00:00:00.263,885] <inf> mcuboot: Primary image: magic=unset, swap_type=0x1, copy_done=0x3, image_ok=0x3
[00:00:00.274,169] <inf> mcuboot: Boot source: none
[00:00:00.279,846] <inf> mcuboot: Swap type: none
[00:00:00.390,747] <inf> mcuboot: Bootloader chainload address offset: 0xc000
[00:00:00.398,620] <inf> mcuboot: Jumping to the first image slot
*** Booting Zephyr OS build v2.4.0-rc2-33-gc54a511d26d0  ***
[00:00:00.005,340] <inf> smp_sample: build time: Sep 22 2020 12:07:26

Edit: Also same result on 2.4-rc1

[bpbradley]

If it helps, I believe I narrowed down the breaking commit to 6e2ffd28d0fe998095014482cb7067f2c0e5c8ee which appears to just be configuration changes. So this may be simple enough to fix hopefully.

[nvlsianpu]

@bpbradley
I can't confirm your results. Sample works on my desk without any problem.
I just noticed that usually first command sent to DUT is ignored, although it have been working like this since i can recall (looks like i have an issue with my development VM configuration.

[bpbradley]

Okay as just a quick test, I threw in the configurations which were removed from that commit

# Enable logging
CONFIG_LOG=y

CONFIG_BT_L2CAP_TX_MTU=260
CONFIG_BT_RX_BUF_LEN=260

# Enable the Bluetooth (unauthenticated) and shell mcumgr transports.
CONFIG_MCUMGR_SMP_BT=y
CONFIG_MCUMGR_SMP_BT_AUTHEN=n
CONFIG_MCUMGR_SMP_SHELL=y

# Enable the LittleFS file system.
CONFIG_FILE_SYSTEM=y
CONFIG_FILE_SYSTEM_LITTLEFS=y

# Enable file system commands
CONFIG_MCUMGR_CMD_FS_MGMT=y

And now it works. A shell shows up in the terminal and mcumgr cli works as expected on latest master.

I am not sure exactly which configuration is causing the weird behavior, but this does work.

[utzig]

Could you paste your final configuration without the commit drop here?

[bpbradley]

Could you paste your final configuration without the commit drop here?

Here is the complete configuration I am using which works.

# Enable mcumgr.
CONFIG_MCUMGR=y

# Some command handlers require a large stack.
CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=2048

# Ensure an MCUboot-compatible binary is generated.
CONFIG_BOOTLOADER_MCUBOOT=y

# Enable flash operations.
CONFIG_FLASH=y

# Required by the `taskstat` command.
CONFIG_THREAD_MONITOR=y

# Enable statistics and statistic names.
CONFIG_STATS=y
CONFIG_STATS_NAMES=y

# Enable most core commands.
CONFIG_MCUMGR_CMD_IMG_MGMT=y
CONFIG_MCUMGR_CMD_OS_MGMT=y
CONFIG_MCUMGR_CMD_STAT_MGMT=y

# Enable logging
CONFIG_LOG=y

CONFIG_BT_L2CAP_TX_MTU=260
CONFIG_BT_RX_BUF_LEN=260

# Enable the Bluetooth (unauthenticated) and shell mcumgr transports.
CONFIG_MCUMGR_SMP_BT=y
CONFIG_MCUMGR_SMP_BT_AUTHEN=n
CONFIG_MCUMGR_SMP_SHELL=y

# Enable the LittleFS file system.
CONFIG_FILE_SYSTEM=y
CONFIG_FILE_SYSTEM_LITTLEFS=y

# Enable file system commands
CONFIG_MCUMGR_CMD_FS_MGMT=y

I just pasted the old configuration from the last working commit that I found onto the end of the current prj.conf

[bpbradley]

@bpbradley
I can't confirm your results. Sample works on my desk without any problem.
I just noticed that usually first command sent to DUT is ignored, although it have been working like this since i can recall (looks like i have an issue with my development VM configuration.

Interesting. I certainly can't explain why you aren't able to reproduce. Currently I just setup a completely new install of zephyr and was able to immediately reproduce the results. Then I just started searching for the breaking commit and eventually found it to be config related.

[bpbradley]

Hmm.. I think this may be my fault. Looking at the build log the overlay file isn't being merged when I build. It seems I just needed to manually add the configuration to get it to work, and it worked before that one commit because the overlays weren't needed for shell. During my tests I was building with a bad path for the overlay config and didn't notice.

I think this issue can be closed / deleted. I don't think this is an issue after all 🤦‍♂️

[utzig]

Somehow I have the feeling that if someone passes an overlay file which cannot be properly used, it should not be a warning but a failure instead...

[nvlsianpu]

@utzig ^^ can yo open issue for discussion - looks like good idea.