Upgrade to zcbor 0.6.0 #51219

oyvindronningstad · 2022-10-12T14:11:06Z

No description provided.

zephyrbot · 2022-10-12T14:12:04Z

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

Name	Old Revision	New Revision	Diff
zcbor	zephyrproject-rtos/zcbor@`a0d6981`	zephyrproject-rtos/[email protected]	zephyrproject-rtos/[email protected]

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

oyvindronningstad · 2022-10-12T14:13:09Z

cc @VeijoPesonen

nordicjm · 2022-10-13T07:13:07Z

subsys/net/lib/lwm2m/lwm2m_senml_cbor_encode.c

-			    ((((*input)._record_bt >= INT64_MIN) &&
-			      ((*input)._record_bt <= INT64_MAX)) ||
+			    ((((*input)._record_bt >= -9223372036854775807LL) &&
+			      ((*input)._record_bt <= 9223372036854775807LL)) ||


Any reason for the switch from defines to literals?

These two lines are to be reverted back, as you already noticed. INT64_MIN and INT64_MAX shall be used.

VeijoPesonen

Array size calculation coming from ZCBOR side is a nice addition.

VeijoPesonen · 2022-10-13T09:30:29Z

subsys/net/lib/lwm2m/lwm2m_senml_cbor_encode.c

-			    ((((*input)._record_bt >= INT64_MIN) &&
-			      ((*input)._record_bt <= INT64_MAX)) ||
+			    ((((*input)._record_bt >= -9223372036854775807LL) &&
+			      ((*input)._record_bt <= 9223372036854775807LL)) ||


These two lines are to be reverted back, as you already noticed. INT64_MIN and INT64_MAX shall be used.

subsys/net/lib/lwm2m/lwm2m_senml_cbor_decode.c

subsys/net/lib/lwm2m/lwm2m_senml_cbor_encode.c

subsys/net/lib/lwm2m/lwm2m_senml_cbor_types.h

VeijoPesonen · 2022-10-13T09:49:09Z

@jheiskan81 Please review.

subsys/net/lib/lwm2m/lwm2m_senml_cbor_decode.c

subsys/net/lib/lwm2m/lwm2m_senml_cbor_encode.c

subsys/net/lib/lwm2m/lwm2m_senml_cbor_types.h

subsys/net/lib/lwm2m/lwm2m_senml_cbor_encode.c

rlubos · 2022-10-13T10:59:57Z

@oyvindronningstad Out of curiosity, I was under the impression that this code was autogenerated and there's not much room for modifications?

oyvindronningstad · 2022-10-13T11:05:12Z

@rlubos The generated code is formatted with clang-format, and then a patch is applied.

jheiskan81 · 2022-10-13T11:44:07Z

subsys/net/lib/lwm2m/lwm2m_senml_cbor_encode.c

@@ -37,9 +36,8 @@ static bool encode_repeated_record_bn(zcbor_state_t *state, const struct record_
 	bool tmp_result = ((((zcbor_int32_put(state, (-2)))) &&
 			    (zcbor_tstr_encode(state, (&(*input)._record_bn)))));

-	if (!tmp_result) {


Is this clang rules different than normal zephyr. I was wondering that {
} is mandatory.

I think the clang-format automatically uses the zephyr rules.

VeijoPesonen

Looks good to me.

nordicjm · 2022-10-18T12:02:14Z

doc/releases/release-notes-3.3.rst

+changes for Zephyr:
+
+* In the zcbor codebase, the ``ARRAY_SIZE`` macro has been renamed to
+  ``ZCBOR_ARRAY_SIZE`` to not collide with Zephyr's ``ARRAY_SIZE_`` macro.


Suggested change

``ZCBOR_ARRAY_SIZE`` to not collide with Zephyr's ``ARRAY_SIZE_`` macro.

``ZCBOR_ARRAY_SIZE`` to not collide with Zephyr's ``ARRAY_SIZE`` macro.

Changed to

:c:macro:`ARRAY_SIZE`

nordicjm · 2022-10-18T12:03:29Z

west.yml

@@ -253,7 +253,7 @@ manifest:
      revision: e8920192b66db4f909eb9cd3f155d5245c1ae825
      path: modules/lib/uoscore-uedhoc
    - name: zcbor
-      revision: a0d6981f14d4001d6f0d608d1a427f9bc6bb6d02
+      revision: 0.6.0


Should this not be a commit ID?

We have tags now on zcbor fork. The tags are the same as in upstream.

Brings a few quality-of-life improvements for Zephyr, notably: - No more collision with ARRAY_SIZE() - C++ improvements Add entry in release notes. Signed-off-by: Øyvind Rønningstad <[email protected]>

Since it's better at resolving conflicts. Commit regenerated files before applying patch. Signed-off-by: Øyvind Rønningstad <[email protected]>

zcbor 0.6.0 has changed the argument order to put code/convert/validate first. Signed-off-by: Øyvind Rønningstad <[email protected]>

Also do clang-format and apply patch. Signed-off-by: Øyvind Rønningstad <[email protected]>

File created by commiting changes and running git diff HEAD~1.. > lwm2m_senml_cbor.patch Signed-off-by: Øyvind Rønningstad <[email protected]>

de-nordic · 2022-10-19T14:16:05Z

@carlescufi You gave me 1minute to approve ;). I know that I am to slow with reviews, but at least give me a chance :D.
Just kidding: I have already approved yesterday, and there were no changes.

The West manifest in Zephyr 3.3 contains a tag (instead of a SHA) for zcbor [1]. OE standard practice is to always specifiy revisions using SHAs, so in the Jinja template, use the new resolve_revision filter to resolve any tags. [1] zephyrproject-rtos/zephyr#51219 (comment) Signed-off-by: Peter Hoyes <[email protected]> Signed-off-by: Naveen Saini <[email protected]>

oyvindronningstad requested a review from rlubos as a code owner October 12, 2022 14:11

zephyrbot added the area: LWM2M label Oct 12, 2022

zephyrbot requested a review from SeppoTakalo October 12, 2022 14:11

zephyrbot assigned rlubos Oct 12, 2022

zephyrbot added manifest west manifest-zcbor DNM This PR should not be merged (Do Not Merge) labels Oct 12, 2022

oyvindronningstad requested review from de-nordic and removed request for SeppoTakalo October 12, 2022 14:12

rlubos previously approved these changes Oct 12, 2022

View reviewed changes

nordicjm reviewed Oct 13, 2022

View reviewed changes

VeijoPesonen suggested changes Oct 13, 2022

View reviewed changes

oyvindronningstad commented Oct 13, 2022

View reviewed changes

oyvindronningstad dismissed rlubos’s stale review via dfd8f42 October 13, 2022 10:48

zephyrbot requested a review from SeppoTakalo October 13, 2022 10:48

oyvindronningstad commented Oct 13, 2022

View reviewed changes

subsys/net/lib/lwm2m/lwm2m_senml_cbor_encode.c Outdated Show resolved Hide resolved

oyvindronningstad force-pushed the zcbor-0.6.0 branch from 587728e to 6867b47 Compare October 13, 2022 10:51

jheiskan81 reviewed Oct 13, 2022

View reviewed changes

oyvindronningstad force-pushed the zcbor-0.6.0 branch from f2337c8 to a4d3558 Compare October 13, 2022 11:56

VeijoPesonen previously approved these changes Oct 17, 2022

View reviewed changes

oyvindronningstad dismissed VeijoPesonen’s stale review via 0e97245 October 18, 2022 11:26

oyvindronningstad force-pushed the zcbor-0.6.0 branch from a4d3558 to 0e97245 Compare October 18, 2022 11:26

zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Oct 18, 2022

oyvindronningstad force-pushed the zcbor-0.6.0 branch from 0e97245 to d66f14c Compare October 18, 2022 11:58

oyvindronningstad requested a review from nashif as a code owner October 18, 2022 11:58

zephyrbot added Release Notes To be mentioned in the release notes area: Documentation labels Oct 18, 2022

zephyrbot requested review from carlescufi, laurenmurphyx64 and stephanosio October 18, 2022 11:58

nordicjm reviewed Oct 18, 2022

View reviewed changes

oyvindronningstad added 5 commits October 18, 2022 14:36

west.yml: Update zcbor to 0.6.0

dedc9ea

Brings a few quality-of-life improvements for Zephyr, notably: - No more collision with ARRAY_SIZE() - C++ improvements Add entry in release notes. Signed-off-by: Øyvind Rønningstad <[email protected]>

lwm2m_senml_cbor_regenerate.sh: Use git am instead of git apply

bcb3656

Since it's better at resolving conflicts. Commit regenerated files before applying patch. Signed-off-by: Øyvind Rønningstad <[email protected]>

lwm2m_senml_cbor_regenerate.sh: Fix argument order in zcbor call

d0c5b3d

zcbor 0.6.0 has changed the argument order to put code/convert/validate first. Signed-off-by: Øyvind Rønningstad <[email protected]>

lwm2m_senml: Regenerate cbor code using zcbor

d899018

Also do clang-format and apply patch. Signed-off-by: Øyvind Rønningstad <[email protected]>

lwm2m_senml_cbor.patch: Update patch file to match current changes

5467512

File created by commiting changes and running git diff HEAD~1.. > lwm2m_senml_cbor.patch Signed-off-by: Øyvind Rønningstad <[email protected]>

oyvindronningstad force-pushed the zcbor-0.6.0 branch from d66f14c to 5467512 Compare October 18, 2022 12:36

nordicjm approved these changes Oct 18, 2022

View reviewed changes

oyvindronningstad requested a review from nordicjm October 18, 2022 12:38

de-nordic approved these changes Oct 18, 2022

View reviewed changes

carlescufi requested review from jheiskan81, rlubos, VeijoPesonen and de-nordic and removed request for jheiskan81 and VeijoPesonen October 19, 2022 14:04

carlescufi merged commit 1e88e7c into zephyrproject-rtos:main Oct 19, 2022

oyvindronningstad deleted the zcbor-0.6.0 branch October 20, 2022 09:51

	``ZCBOR_ARRAY_SIZE`` to not collide with Zephyr's ``ARRAY_SIZE_`` macro.
	``ZCBOR_ARRAY_SIZE`` to not collide with Zephyr's ``ARRAY_SIZE`` macro.

Upgrade to zcbor 0.6.0 #51219

Upgrade to zcbor 0.6.0 #51219

Uh oh!

Conversation

oyvindronningstad commented Oct 12, 2022

Uh oh!

zephyrbot commented Oct 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oyvindronningstad commented Oct 12, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VeijoPesonen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VeijoPesonen commented Oct 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rlubos commented Oct 13, 2022

Uh oh!

oyvindronningstad commented Oct 13, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VeijoPesonen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

de-nordic commented Oct 19, 2022

Uh oh!

Uh oh!

zephyrbot commented Oct 12, 2022 •

edited

Loading