Update mbed-coap to version 5.1.0 #11359

anttiylitokola · 2019-08-28T10:45:41Z

Description

Reduce heap footprint by storing only single block when receiving a blockwise message.
- Feature is controlled through SN_COAP_REDUCE_BLOCKWISE_HEAP_FOOTPRINT configuration flag. Flag is disabled by default to keep the backward compatibility in place. If flag is enabled, application must NOT free the payload when it gets the COAP_STATUS_PARSER_BLOCKWISE_MSG_RECEIVED status. And application must call sn_coap_protocol_block_remove() instead.
Bug fix: Request blockwise transfer if incoming payload length is too large and when it comes without block indication.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@yogpan01, @teetak01

- Reduce heap footprint by storing only single block when receiving a blockwise message. * User is now responsible of freeing the data by calling sn_coap_protocol_block_remove() and must not free the payload separately. - Bug fix: Request blockwise transfer if incoming payload length is too large and when it comes without block indication.

0xc0170 · 2019-08-28T10:48:44Z

Is this breaking change here: * User is now responsible of freeing the data by calling sn_coap_protocol_block_remove() and must not free the payload separately.
?

Or what is breaking and why? Please add release notes section above. Isn't this just functional change ?

yogpan01 · 2019-08-28T10:52:48Z

@0xc0170 Yes, you are right that this is a breaking change as the API behaviour has changed. Earlier using this API, this library handled the memory allocation but it was not memory optimized and caused inflated memory heap usage. In this version, we have given that control to the application which means application will have to implement the freeing logic on their end.
That's why we have also bumped the coap library version info to 5.0.0 reflecting this break.
@anttiylitokola Please update the CHANGELOG to clearly reflect that this will be a breaking change for previous users of the given blockwise API.

ciarmcom · 2019-08-28T11:00:32Z

@anttiylitokola, thank you for your changes.
@yogpan01 @teetak01 @ARMmbed/mbed-os-maintainers please review.

anttiylitokola · 2019-08-28T11:24:42Z

@0xc0170 Yes, you are right that this is a breaking change as the API behaviour has changed. Earlier using this API, this library handled the memory allocation but it was not memory optimized and caused inflated memory heap usage. In this version, we have given that control to the application which means application will have to implement the freeing logic on their end.
That's why we have also bumped the coap library version info to 5.0.0 reflecting this break.
@anttiylitokola Please update the CHANGELOG to clearly reflect that this will be a breaking change for previous users of the given blockwise API.

Changelog has been updated.

0xc0170 · 2019-08-28T11:55:18Z

@anttiylitokola also add release section (it was in the PR template) and add details there (we use these for real release notes).

0xc0170 · 2019-08-28T12:19:50Z

CI started

0xc0170

waiting for the release notes section

anttiylitokola · 2019-08-28T12:43:29Z

@anttiylitokola also add release section (it was in the PR template) and add details there (we use these for real release notes).

@0xc0170, is the release note section ok now?

mbed-ci · 2019-08-28T13:39:06Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2019-08-28T14:14:30Z

Restarting the build, wifi example failed but looks like timeout to me

mbed-ci · 2019-08-28T15:29:54Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

0xc0170 · 2019-08-28T16:13:41Z

Error ToolException: Compile did not finish in 10 minutes ? -11 from ARM again.

cc @ARMmbed/mbed-os-test Please review

0xc0170 · 2019-08-28T16:14:38Z

Restarted CI

mbed-ci · 2019-08-28T17:37:24Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM

0xc0170 · 2019-08-28T17:54:20Z

Failure not related

@ARMmbed/mbed-os-crypto updated the example for upcoming crypto, trying to fix it now

0xc0170 · 2019-08-28T18:01:29Z

Ci restarted (should be fixed now)

mbed-ci · 2019-08-28T23:37:04Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test

0xc0170 · 2019-08-29T04:37:14Z

cloud client restarted

anttiylitokola · 2019-08-29T04:46:31Z

cloud client restarted

cloud client tests will fail due to this breaking change.

0xc0170 · 2019-08-29T06:48:35Z

@anttiylitokola Please talk to @OPpuolitaival how to fix this. I would assume the test would be fixed for this PR and master (update, restart, integrate).

OPpuolitaival · 2019-08-29T07:02:02Z

Waiting for pelion device management client release candidate for testing in CI

0xc0170 · 2019-08-29T07:27:45Z

Waiting for Release candidate for testing in CI

I don't follow. What shall we do here?

yogpan01 · 2019-08-29T07:39:06Z

@OPpuolitaival there wont be any release candidate since it will require this fix to be merged to mbed-os. This is a typical "Catch22" situation.
I am asking @anttiylitokola to check if we can make this change as backward compatible to make your tests GREEN.
Although I fail to understand if we clearly mention that this is a breaking change and we are proposing it as part of a major release, why such bureaucracy around it.

By default CoAP will create a copy of the whole data to be passed to application and it keeps the backward compatibility. If enabled, application must NOT free the payload when it gets the COAP_STATUS_PARSER_BLOCKWISE_MSG_RECEIVED status. And application must call sn_coap_protocol_block_remove() instead.

anttiylitokola · 2019-08-29T10:40:15Z

Feature is now behind a flag which is disabled by default to maintain backward compatibility.

0xc0170 · 2019-08-29T10:43:03Z

CI restarted

mbed-ci · 2019-08-29T13:31:08Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 5
Build artifacts

0xc0170 added the release-version: 5.14.0-rc1 label Aug 28, 2019

ciarmcom requested review from teetak01, yogpan01 and a team August 28, 2019 11:00

ciarmcom added the needs: review label Aug 28, 2019

0xc0170 added the BREAKING-CHANGE label Aug 28, 2019

0xc0170 requested a review from bulislaw August 28, 2019 11:15

Update changelog about the breaking change

d74326e

yogpan01 approved these changes Aug 28, 2019

View reviewed changes

teetak01 approved these changes Aug 28, 2019

View reviewed changes

bulislaw approved these changes Aug 28, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Aug 28, 2019

0xc0170 requested changes Aug 28, 2019

View reviewed changes

0xc0170 approved these changes Aug 28, 2019

View reviewed changes

anttiylitokola changed the title ~~Update mbed-coap to version 5.0.0~~ Update mbed-coap to version 5.1.0 Aug 29, 2019

0xc0170 added ready for merge and removed needs: CI labels Aug 29, 2019

0xc0170 merged commit 40a84fc into ARMmbed:master Aug 29, 2019

0xc0170 removed BREAKING-CHANGE ready for merge labels Aug 29, 2019

Update mbed-coap to version 5.1.0 #11359

Update mbed-coap to version 5.1.0 #11359

Uh oh!

Conversation

anttiylitokola commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

yogpan01 commented Aug 28, 2019

Uh oh!

ciarmcom commented Aug 28, 2019

Uh oh!

anttiylitokola commented Aug 28, 2019

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

anttiylitokola commented Aug 28, 2019

Uh oh!

mbed-ci commented Aug 28, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

mbed-ci commented Aug 28, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

mbed-ci commented Aug 28, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

0xc0170 commented Aug 28, 2019

Uh oh!

mbed-ci commented Aug 28, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Aug 29, 2019

Uh oh!

anttiylitokola commented Aug 29, 2019

Uh oh!

0xc0170 commented Aug 29, 2019

Uh oh!

OPpuolitaival commented Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Aug 29, 2019

Uh oh!

yogpan01 commented Aug 29, 2019

Uh oh!

anttiylitokola commented Aug 29, 2019

Uh oh!

0xc0170 commented Aug 29, 2019

Uh oh!

mbed-ci commented Aug 29, 2019

Test run: SUCCESS

Uh oh!

Uh oh!

anttiylitokola commented Aug 28, 2019 •

edited

Loading

OPpuolitaival commented Aug 29, 2019 •

edited

Loading