Skip to content

boot_serial: port CBOR encoding to use cddl-gen #997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 25, 2021

Conversation

oyvindronningstad
Copy link
Contributor

Also update cddl-gen to newer version.
This removes the dependency on tinycbor.

@oyvindronningstad
Copy link
Contributor Author

oyvindronningstad commented May 7, 2021

FYI @nvlsianpu @de-nordic @carlescufi

@de-nordic
Copy link
Collaborator

@oyvindronningstad Do you have test manifest PR to Zephyr that I can fetch to test this?

@oyvindronningstad
Copy link
Contributor Author

@de-nordic
Copy link
Collaborator

de-nordic commented May 11, 2021

Is it working for you? I am building this with CONFIG_MCUBOOT_SERIAL=y, CONFIG_UART_CONSOLE=n, CONFIG_MULTITHREADING=y (to make it compile) and the mcumgr hands trying to communicate to device.
The Zephyr origin/master (977aba6d02c87f884ecc79672690befcbcfd0186) with the same options work.

@oyvindronningstad
Copy link
Contributor Author

oyvindronningstad commented May 12, 2021

@de-nordic I'm able to transfer using the provided branch.

west build ../bootloader/mcuboot/boot/zephyr/ -p -- -DCONFIG_MCUBOOT_SERIAL=y -DCONFIG_UART_CONSOLE=n -DCONFIG_MULTITHREADING=y -DCONFIG_CONSOLE_HANDLER=n -DCONFIG_BOOT_SERIAL_DETECT_PIN_VAL=1

CONFIG_BOOT_SERIAL_DETECT_PIN_VAL is just because I'm remote and can't push the button :).

I'm on nRF9160

@de-nordic
Copy link
Collaborator

Okay, so seems that upload is ok but "image list" does not work.
When I invoke

sudo ~/go/bin/mcumgr  -t 1200 --conntype serial --connstring dev=/dev/ttyUSB0,baud=115200 image list

It waits, although when interrupted (Ctrl + C kind) you may than upload image,

When I run the command to dump debug:

sudo ~/go/bin/mcumgr  -l "debug" -t 1200 --conntype serial --connstring dev=/dev/ttyUSB0,baud=115200 image list

I got this

time="2021-05-12 14:22:15.148" level=debug msg="Using connection profile: name=unnamed type=serial connstring=dev=/dev/ttyUSB0,baud=115200"
time="2021-05-12 14:22:15.149" level=debug msg="{add-nmp-listener} [serial_sesn.go:213] seq=66"
time="2021-05-12 14:22:15.149" level=debug msg="Encoded &{NmpBase:{hdr:{Op:0 Flags:0 Len:0 Group:1 Seq:66 Id:0}}} to:
00000000  a0                                                |.|
"
time="2021-05-12 14:22:15.149" level=debug msg="Encoded:
00000000  00 00 00 01 00 01 42 00  a0                       |......B..|
"
time="2021-05-12 14:22:15.149" level=debug msg="Tx NMP request: 00000000  00 00 00 01 00 01 42 00  a0                       |......B..|
"
time="2021-05-12 14:22:15.149" level=debug msg="Base64 encoding request:
00000000  00 00 00 01 00 01 42 00  a0                       |......B..|
"
time="2021-05-12 14:22:15.149" level=debug msg="Tx serial
00000000  06 09                                             |..|
"
time="2021-05-12 14:22:15.149" level=debug msg="Tx serial
00000000  41 41 73 41 41 41 41 42  41 41 46 43 41 4b 44 31  |AAsAAAABAAFCAKD1|
00000010  4d 77 3d 3d                                       |Mw==|
"
time="2021-05-12 14:22:15.149" level=debug msg="Tx serial
00000000  0a                                                |.|
"
time="2021-05-12 14:22:15.246" level=debug msg="Rx serial:
00000000  06 09 41 44 4d 42 41 41  41 70 41 41 46 43 41 47  |..ADMBAAApAAFCAG|
00000010  5a 70 62 57 46 6e 5a 58  4f 66 76 32 52 7a 62 47  |ZpbWFnZXOfv2RzbG|
00000020  39 30 41 47 64 32 5a 58  4a 7a 61 57 39 75 5a 7a  |90AGd2ZXJzaW9uZz|
00000030  41 75 4d 43 34 77 4c 6a  42 6e 4d 43 34 77 4c 6a  |AuMC4wLjBnMC4wLj|
00000040  41 75 4d 50 2f 2f 45 4c  59 3d                    |AuMP//ELY=|
"
time="2021-05-12 14:22:15.246" level=debug msg="Decoded input:
00000000  01 00 00 29 00 01 42 00  66 69 6d 61 67 65 73 9f  |...)..B.fimages.|
00000010  bf 64 73 6c 6f 74 00 67  76 65 72 73 69 6f 6e 67  |.dslot.gversiong|
00000020  30 2e 30 2e 30 2e 30 67  30 2e 30 2e 30 2e 30 ff  |0.0.0.0g0.0.0.0.|
00000030  ff                                                |.|
"
time="2021-05-12 14:22:15.246" level=debug msg="rx nmp response: 00000000  01 00 00 29 00 01 42 00  66 69 6d 61 67 65 73 9f  |...)..B.fimages.|
00000010  bf 64 73 6c 6f 74 00 67  76 65 72 73 69 6f 6e 67  |.dslot.gversiong|
00000020  30 2e 30 2e 30 2e 30 67  30 2e 30 2e 30 2e 30 ff  |0.0.0.0g0.0.0.0.|
00000030  ff                                                |.|
"
time="2021-05-12 14:22:15.246" level=debug msg="Failure decoding NMP rsp: Invalid response: cbor decode error [pos 1]: only encoded map or array can be decoded into a struct
packet=
00000000  01 00 00 29 00 01 42 00  66 69 6d 61 67 65 73 9f  |...)..B.fimages.|
00000010  bf 64 73 6c 6f 74 00 67  76 65 72 73 69 6f 6e 67  |.dslot.gversiong|
00000020  30 2e 30 2e 30 2e 30 67  30 2e 30 2e 30 2e 30 ff  |0.0.0.0g0.0.0.0.|
00000030  ff                                                |.|
"

Update and rename submodule.
Regenerate code and copy updated files.
Update regeneration script.

Signed-off-by: Øyvind Rønningstad <[email protected]>
@oyvindronningstad
Copy link
Contributor Author

@de-nordic Thanks! I think I found the error. I tested image list, and it works for me now.

@de-nordic de-nordic self-requested a review May 21, 2021 09:37
@de-nordic
Copy link
Collaborator

Nearly there, the imge list works only up to first successful upload, afterwards it is this:

DEBU[2021-05-21 10:40:52.043] Using connection profile: name=unnamed type=serial connstring=dev=/dev/ttyACM0,baud=115200
DEBU[2021-05-21 10:40:52.044] {add-nmp-listener} [serial_sesn.go:213] seq=66
DEBU[2021-05-21 10:40:52.044] Encoded &{NmpBase:{hdr:{Op:0 Flags:0 Len:0 Group:1 Seq:66 Id:0}}} to:
00000000  a0                                                |.|
DEBU[2021-05-21 10:40:52.044] Encoded:
00000000  00 00 00 01 00 01 42 00  a0                       |......B..|
DEBU[2021-05-21 10:40:52.044] Tx NMP request: 00000000  00 00 00 01 00 01 42 00  a0                       |......B..|
DEBU[2021-05-21 10:40:52.044] Base64 encoding request:
00000000  00 00 00 01 00 01 42 00  a0                       |......B..|
DEBU[2021-05-21 10:40:52.044] Tx serial
00000000  06 09                                             |..|
DEBU[2021-05-21 10:40:52.044] Tx serial
00000000  41 41 73 41 41 41 41 42  41 41 46 43 41 4b 44 31  |AAsAAAABAAFCAKD1|
00000010  4d 77 3d 3d                                       |Mw==|
DEBU[2021-05-21 10:40:52.044] Tx serial
00000000  0a                                                |.|
DEBU[2021-05-21 10:40:52.138] Rx serial:
00000000  06 09 41 44 55 42 41 41  41 72 41 41 46 43 41 4c  |..ADUBAAArAAFCAL|
00000010  39 6d 61 57 31 68 5a 32  56 7a 6e 37 39 6b 63 32  |9maW1hZ2Vzn79kc2|
00000020  78 76 64 41 42 6e 64 6d  56 79 63 32 6c 76 62 6d  |xvdABndmVyc2lvbm|
00000030  63 77 4c 6a 41 75 4d 43  34 77 5a 7a 41 75 4d 43  |cwLjAuMC4wZzAuMC|
00000040  34 77 4c 6a 44 2f 2f 2f  39 51 59 77 3d 3d        |4wLjD///9QYw==|
DEBU[2021-05-21 10:40:52.138] Decoded input:
00000000  01 00 00 2b 00 01 42 00  bf 66 69 6d 61 67 65 73  |...+..B..fimages|
00000010  9f bf 64 73 6c 6f 74 00  67 76 65 72 73 69 6f 6e  |..dslot.gversion|
00000020  67 30 2e 30 2e 30 2e 30  67 30 2e 30 2e 30 2e 30  |g0.0.0.0g0.0.0.0|
00000030  ff ff ff                                          |...|
DEBU[2021-05-21 10:40:52.138] rx nmp response: 00000000  01 00 00 2b 00 01 42 00  bf 66 69 6d 61 67 65 73  |...+..B..fimages|
00000010  9f bf 64 73 6c 6f 74 00  67 76 65 72 73 69 6f 6e  |..dslot.gversion|
00000020  67 30 2e 30 2e 30 2e 30  67 30 2e 30 2e 30 2e 30  |g0.0.0.0g0.0.0.0|
00000030  ff ff ff                                          |...|
DEBU[2021-05-21 10:40:52.139] Failure decoding NMP rsp: Invalid response: cbor decode error [pos 41]: decodeNaked: Unrecognized d.bd: 0xff
packet=
00000000  01 00 00 2b 00 01 42 00  bf 66 69 6d 61 67 65 73  |...+..B..fimages|
00000010  9f bf 64 73 6c 6f 74 00  67 76 65 72 73 69 6f 6e  |..dslot.gversion|
00000020  67 30 2e 30 2e 30 2e 30  67 30 2e 30 2e 30 2e 30  |g0.0.0.0g0.0.0.0|
00000030  ff ff ff
``

Non-generated. Using cbor_encode directly
This removes the dependence on TinyCBOR.

Fixes mcu-tools#978

Signed-off-by: Øyvind Rønningstad <[email protected]>
@oyvindronningstad
Copy link
Contributor Author

@de-nordic Thanks again, fixed :).

Copy link
Collaborator

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on nrf52840dk: all features of serial recovery work correctly.

Looks OK, couldn't find anything to stick to. Thanks!

@oyvindronningstad
Copy link
Contributor Author

@de-nordic @nvlsianpu I'm not allowed to add reviewers, so please add relevant people.

@de-nordic de-nordic requested a review from utzig May 21, 2021 12:39
Copy link
Member

@utzig utzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, works on Mynewt as well.

@oyvindronningstad
Copy link
Contributor Author

Great, this is ready from my side now.

@carlescufi
Copy link
Collaborator

@nvlsianpu is this ready for merging now?

@utzig utzig merged commit 9f4aefd into mcu-tools:main May 25, 2021
@utzig
Copy link
Member

utzig commented May 25, 2021

@nvlsianpu is this ready for merging now?

Hopefully it is :-)

@oyvindronningstad oyvindronningstad deleted the encode branch May 25, 2021 16:53
@de-nordic
Copy link
Collaborator

Btw, mcuboot flas size got tiny reduction:

Memory region         Used Size  Region Size  %age Used
           FLASH:       43952 B        48 KB     89.42%
            SRAM:       26272 B       256 KB     10.02%
        IDT_LIST:          0 GB         2 KB      0.00%

vs the PR base:

Memory region         Used Size  Region Size  %age Used
           FLASH:       43972 B        48 KB     89.46%
            SRAM:       26272 B       256 KB     10.02%
        IDT_LIST:          0 GB         2 KB      0.00%

@sjanc
Copy link
Contributor

sjanc commented Jun 27, 2022

Hi,

sorry for digging this out but I recently wanted to update Mynewt dependency from 1.7.2 to 1.9 and that commit broke 'newt test all'. ie mcuboot/boot/boot_serial/test/ doesn't build

Compiling bin/targets/unittest/boot_boot_serial_test/generated/src/boot_boot_serial_test-sysinit-app.c
repos/mcuboot/boot/boot_serial/test/src/testcases/boot_serial_upload_bigger_image.c: In function ‘TEST_CASE_boot_serial_upload_bigger_image’:
repos/mcuboot/boot/boot_serial/test/src/testcases/boot_serial_upload_bigger_image.c:91:42: error: ‘Value8Bit’ undeclared (first use in this function)
91 | buf[payload_off + len - 2] = Value8Bit;
| ^~~~~~~~~

So two questions:

  1. should I simply move those tests from mcuboot into mynewt-core?
  2. Is Mynewt target no longer tested for mcuboot releases?

@sjanc
Copy link
Contributor

sjanc commented Jun 27, 2022

looks like it was easy to fix in mcuboot :) #1400

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants