west.yml: Update zcbor to 0.8.0 #67418

oyvindronningstad · 2024-01-10T08:12:27Z

Add support for unordered maps
Performance improvements
Naming improvements for generated code
Bugfixes

zephyrbot · 2024-01-10T08:13:27Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
mcuboot	zephyrproject-rtos/mcuboot@`b994ba2`	zephyrproject-rtos/mcuboot@`f09e205` (main,upstream-sync)	zephyrproject-rtos/[email protected]
uoscore-uedhoc	zephyrproject-rtos/uoscore-uedhoc@`5024d8c`	zephyrproject-rtos/uoscore-uedhoc@`150f4eb` (zephyr)	zephyrproject-rtos/[email protected]
zcbor	zephyrproject-rtos/zcbor@`67fd8bb` (main)	zephyrproject-rtos/zcbor@`dbe20af`	zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

nordicjm · 2024-01-15T09:44:35Z

subsys/mgmt/mcumgr/grp/os_mgmt_client/src/os_mgmt_client.c

@@ -138,7 +138,8 @@ int os_mgmt_client_echo(struct os_mgmt_client *client, const char *echo_string)
 	zcbor_new_encode_state(zse, ARRAY_SIZE(zse), nb->data + nb->len, net_buf_tailroom(nb), 0);

 	ok = zcbor_map_start_encode(zse, 2) &&
-	     zcbor_tstr_put_lit(zse, "d") && zcbor_tstr_put_term(zse, echo_string) &&
+	     zcbor_tstr_put_lit(zse, "d") &&
+	     zcbor_tstr_put_term(zse, echo_string, CONFIG_ZCBOR_MAX_STR_LEN) &&


I know this isn't actually changing behaviour but kind of seems a bit wrong that this is implying that the pointer is (by default) 4096 bytes long?

@nordicjm I agree. I managed to limit it to only be used in tests, and I changed it to be 256 instead of 4096. And I added help text discouraging its use.

SeppoTakalo

I did not check the code, but at least LwM2M interop tests against Leshan seem to pass. Verified that manually.

So I cannot see anything broken. OK from LwM2M point of view.

nordicjm · 2024-01-16T12:35:00Z

subsys/stats/stats.c

+	if (strnlen(name, CONFIG_STATS_MAX_NAME_LEN) == CONFIG_STATS_MAX_NAME_LEN) {
+		return -ENAMETOOLONG;
+	}


Not really sure on this? Seems like an MCUmgr limitation, don't see why it should apply to stats as a whole

It's a sensible check on its own I'd say, but if we don't want to make any changes there, I could make a config in stat_mgmt/Kconfig instead, just for use when encoding.

Stats system has been in zephyr since version 1.11 so would prefer not to make possible breaking changes to it at this point unless they're fixes

Gotcha, I think I found a good solution in stats_mgmt instead.

nordicjm · 2024-01-17T08:24:58Z

subsys/mgmt/mcumgr/grp/stat_mgmt/Kconfig

@@ -32,6 +32,14 @@ config MCUMGR_GRP_STAT_MAX_NAME_LEN
 	  stat read commands.  If a stat group's name exceeds this limit, it will
 	  be impossible to retrieve its values with a stat show command.

+config MCUMGR_STAT_MAX_NAME_LEN


Must follow naming rule from top of file:

# Options defined in this file should be prefixed: # MCUMGR_GRP_STAT_ -- general group options;

I changed it to reuse the existing config.

nordicjm · 2024-01-17T09:17:02Z

west.yml

      path: modules/lib/uoscore-uedhoc
    - name: zcbor
-      revision: 67fd8bb88d3136738661fa8bb5f9989103f4599e
+      revision: 0.8.0


Need to use a commit ID for this, not a tag

Highlights of zcbor 0.8.0: - Add support for unordered maps - Performance improvements - Naming improvements for generated code - Bugfixes Update MCUboot and uoscore-uedhoc to bring in changes related to zcbor 0.8.0. Signed-off-by: Øyvind Rønningstad <[email protected]>

If people are using the zcbor script for code generation, it needs to be at the latest version Signed-off-by: Øyvind Rønningstad <[email protected]>

For use with zcbor_tstr_put_term() which needs a maximum string length. Signed-off-by: Øyvind Rønningstad <[email protected]>

Regenerate from CDDL, update patch, fix references in code. Signed-off-by: Øyvind Rønningstad <[email protected]>

zcbor_new_state and zcbor_tstr_put_term Signed-off-by: Øyvind Rønningstad <[email protected]>

doc/releases/migration-guide-3.6.rst

kartben · 2024-01-25T09:31:56Z

doc/releases/migration-guide-3.6.rst

+  For example:
+    * Leading single underscores and all double underscores are largely gone,
+    * Names sometimes gain suffixes like ``_m`` or ``_l`` for disambiguation.
+    * All enum (choice) names have now gained a ``_c`` suffix, so the enum name no longer matches
+      the corresponding member name exactly (because this broke C++ namespace rules).


probably should be:

Suggested change

For example:

* Leading single underscores and all double underscores are largely gone,

* Names sometimes gain suffixes like ``_m`` or ``_l`` for disambiguation.

* All enum (choice) names have now gained a ``_c`` suffix, so the enum name no longer matches

the corresponding member name exactly (because this broke C++ namespace rules).

For example:

* Leading single underscores and all double underscores are largely gone,

* Names sometimes gain suffixes like ``_m`` or ``_l`` for disambiguation.

* All enum (choice) names have now gained a ``_c`` suffix, so the enum name no longer matches

the corresponding member name exactly (because this broke C++ namespace rules).

A short release note referring to the official release notes, and a migration guide with the most likely changes that need to be made. Signed-off-by: Øyvind Rønningstad <[email protected]>

oyvindronningstad · 2024-01-25T11:13:06Z

Thanks @de-nordic and @kartben, doc build fixed.

nordicjm · 2024-01-25T11:21:04Z

doc/releases/migration-guide-3.6.rst

+  been overhauled, so the code using the generated code must likely be changed.
+  For example:
+
+  * Leading single underscores and all double underscores are largely gone,


(only if another change is requested) replace comma with full stop

zephyrbot added manifest manifest-zcbor labels Jan 10, 2024

nordicjm self-requested a review January 10, 2024 08:46

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 9f7a43b to 882114f Compare January 10, 2024 12:55

zephyrbot added manifest-mcuboot DNM This PR should not be merged (Do Not Merge) labels Jan 10, 2024

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 882114f to b1610a2 Compare January 10, 2024 13:42

zephyrbot added the manifest-uoscore-uedhoc label Jan 10, 2024

oyvindronningstad force-pushed the zcbor-0.8.0 branch from b1610a2 to 2d6295c Compare January 11, 2024 13:42

oyvindronningstad marked this pull request as ready for review January 11, 2024 14:54

zephyrbot added area: CBOR Concise Binary Object Representation area: LWM2M area: mcumgr labels Jan 11, 2024

zephyrbot requested review from de-nordic, rlubos and SeppoTakalo January 11, 2024 14:54

zephyrbot assigned nordicjm Jan 11, 2024

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 2d6295c to 4e2c839 Compare January 11, 2024 14:55

nordicjm reviewed Jan 15, 2024

View reviewed changes

SeppoTakalo previously approved these changes Jan 15, 2024

View reviewed changes

oyvindronningstad dismissed SeppoTakalo’s stale review via 22c4853 January 16, 2024 12:14

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 4e2c839 to 22c4853 Compare January 16, 2024 12:14

zephyrbot added the area: Stats label Jan 16, 2024

oyvindronningstad force-pushed the zcbor-0.8.0 branch 2 times, most recently from 2e2dbff to 98aaa81 Compare January 16, 2024 12:27

nordicjm reviewed Jan 16, 2024

View reviewed changes

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 98aaa81 to 8bb48cc Compare January 16, 2024 13:02

nordicjm requested changes Jan 17, 2024

View reviewed changes

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 697982e to 4ad5c63 Compare January 25, 2024 08:21

zephyrbot added the Release Notes To be mentioned in the release notes label Jan 25, 2024

zephyrbot requested review from henrikbrixandersen, kartben and MaureenHelm January 25, 2024 08:21

zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Jan 25, 2024

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 4ad5c63 to 3201361 Compare January 25, 2024 08:26

oyvindronningstad added 5 commits January 25, 2024 09:27

scripts: requirements-extra.txt: Update zcbor from 0.6.0 to 0.8.0

89ea523

If people are using the zcbor script for code generation, it needs to be at the latest version Signed-off-by: Øyvind Rønningstad <[email protected]>

modules: zcbor: Add config CONFIG_ZCBOR_MAX_STR_LEN

48637ed

For use with zcbor_tstr_put_term() which needs a maximum string length. Signed-off-by: Øyvind Rønningstad <[email protected]>

lwm2m: Adapt to zcbor 0.8.0

f6b1d64

Regenerate from CDDL, update patch, fix references in code. Signed-off-by: Øyvind Rønningstad <[email protected]>

mgmt: Adapt to API changes in zcbor 0.8.0

060945f

zcbor_new_state and zcbor_tstr_put_term Signed-off-by: Øyvind Rønningstad <[email protected]>

tests: mgmt: Adapt to API changes in zcbor 0.8.0

9a4e332

zcbor_new_state and zcbor_tstr_put_term Signed-off-by: Øyvind Rønningstad <[email protected]>

oyvindronningstad force-pushed the zcbor-0.8.0 branch 2 times, most recently from 6450676 to 1755c66 Compare January 25, 2024 08:30

nordicjm previously approved these changes Jan 25, 2024

View reviewed changes

nordicjm requested a review from fabiobaltieri January 25, 2024 08:32

de-nordic previously approved these changes Jan 25, 2024

View reviewed changes

de-nordic reviewed Jan 25, 2024

View reviewed changes

doc/releases/migration-guide-3.6.rst Outdated Show resolved Hide resolved

kartben reviewed Jan 25, 2024

View reviewed changes

doc: Add release notes and migration guide for zcbor 0.8.0

adda9b4

A short release note referring to the official release notes, and a migration guide with the most likely changes that need to be made. Signed-off-by: Øyvind Rønningstad <[email protected]>

oyvindronningstad dismissed stale reviews from de-nordic and nordicjm via adda9b4 January 25, 2024 10:56

oyvindronningstad force-pushed the zcbor-0.8.0 branch from 1755c66 to adda9b4 Compare January 25, 2024 10:56

nordicjm approved these changes Jan 25, 2024

View reviewed changes

de-nordic requested review from kartben and SeppoTakalo January 25, 2024 11:22

de-nordic approved these changes Jan 25, 2024

View reviewed changes

fabiobaltieri merged commit 0176a27 into zephyrproject-rtos:main Jan 25, 2024
30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

west.yml: Update zcbor to 0.8.0 #67418

west.yml: Update zcbor to 0.8.0 #67418

oyvindronningstad commented Jan 10, 2024

zephyrbot commented Jan 10, 2024 •

edited

Loading

nordicjm Jan 15, 2024

oyvindronningstad Jan 16, 2024

SeppoTakalo left a comment

nordicjm Jan 16, 2024

oyvindronningstad Jan 16, 2024

nordicjm Jan 16, 2024

oyvindronningstad Jan 16, 2024

nordicjm Jan 17, 2024

oyvindronningstad Jan 17, 2024

nordicjm Jan 17, 2024

oyvindronningstad Jan 17, 2024

kartben Jan 25, 2024

oyvindronningstad commented Jan 25, 2024

nordicjm Jan 25, 2024

west.yml: Update zcbor to 0.8.0 #67418

west.yml: Update zcbor to 0.8.0 #67418

Conversation

oyvindronningstad commented Jan 10, 2024

zephyrbot commented Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeppoTakalo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oyvindronningstad commented Jan 25, 2024

Choose a reason for hiding this comment

zephyrbot commented Jan 10, 2024 •

edited

Loading