Skip to content

sys/suit/v4: switch to nanocbor#12562

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
kaspar030:pr/suit_v4_nanocbor
Oct 24, 2019
Merged

sys/suit/v4: switch to nanocbor#12562
benpicco merged 1 commit intoRIOT-OS:masterfrom
kaspar030:pr/suit_v4_nanocbor

Conversation

@kaspar030
Copy link
Contributor

Contribution description

This PR changes suit/v4 to use nanocbor instead of tinycbor. The former was already used by suit's dependency libcose, causing two cbor parsers to be used.

Code size goes down from 75964b 74616b in nrf52dk.

Most of the work was done by @fjmolinas. I just fixed some bugs and put the diff into PR shape.

Testing procedure

WARNING the test will currently fail without #12408 and #12458 merged (same as on master).

Confirm the examples/suit_example test application runs successful.

Issues/PRs references

@kaspar030 kaspar030 added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: sys Area: System labels Oct 24, 2019
@fjmolinas
Copy link
Contributor

Hmm.. I'm getting an error when validating the digest... investigating

iotboot_flashwrite: processing bytes 77376-77439
riotboot_flashwrite: processing bytes 77440-77479
riotboot: verifying digest at 0x20003577 (img at: 0x20800 size: 77480)
no handler found
handler returned <0
)suit_v4_parse() failed. res=-1
Timeout in expect script at "timeout=UPDATING_TIMEOUT)" (examples/suit_update/tests/01-run.py:71)

@kaspar030
Copy link
Contributor Author

Hmm.. I'm getting an error when validating the digest... investigating

I'm getting the same now. I must have messed something up while polishing. Invastigating, too...

@kaspar030
Copy link
Contributor Author

I think I got it. I've accidentally added an error return for the "no handler found" case.

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

Test still working (if the correct source address is chosen), minor comments otherwise all good. ~1kB less as well :)

Image start address: 0x00001100
Header chksum: 0x69e1c0a7

[auto_init_netif] initializing ethos #0
----> ethos: hello received
Failed to send flush request: Operation not permitted
gnrc_uhcpc: Using 5 as border interface and 0 as wireless interface.
gnrc_uhcpc: only one interface found, skipping setup.
main(): This is RIOT! (Version: 2020.01-devel-301-gb91d5-pr-12562)
RIOT SUIT update example application
running from slot 0
TEST PASSED

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK, please squash!

Co-authored-by: Francisco <femolina@uc.cl>
@kaspar030 kaspar030 force-pushed the pr/suit_v4_nanocbor branch from 2d41d9b to 4ccb4e5 Compare October 24, 2019 14:48
@kaspar030
Copy link
Contributor Author

  • squashed

@benpicco benpicco merged commit afecb9a into RIOT-OS:master Oct 24, 2019
@kaspar030 kaspar030 deleted the pr/suit_v4_nanocbor branch October 25, 2019 18:55
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
@benpicco
Copy link
Contributor

btw what was the reason to convert SUIT to NanoCBOR instead of converting libcose to tinycbor?

@kaspar030
Copy link
Contributor Author

nanocbor has a much nicer API and uses less code.

@benpicco
Copy link
Contributor

It also has a lot more bugs and doesn't implement the full CBOR specification…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants