Skip to content
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

BLE: Cordio host upgrade #9864

Merged
merged 11 commits into from Mar 2, 2019

Conversation

Projects
None yet
9 participants
@paul-szczepanek-arm
Copy link
Member

commented Feb 26, 2019

Description

Upgrade Cordio host to 19.02.
Open source Cordio controller.

Pull request type

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

Reviewers

@donatieng
@pan-

Release Notes

Upgrade of cordio host to new version (from 2.4 to 19.02). Should not impact users and be a in-place upgrade.
Replace compiled cordio controller libs with sources - no functional change and no impact on users.

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2019

Please add needs work. This is based on the pre-release and needs updating.

features/FEATURE_BLE/targets/TARGET_CORDIO/stack/ble-host/include/att_api.h Outdated

/*************************************************************************************************/
/*!
* \brief Attribute protocol client and server API. */

This comment has been minimized.

Copy link
@paul-szczepanek-arm

paul-szczepanek-arm Feb 26, 2019

Author Member

blame regex, will fix

This comment has been minimized.

Copy link
@paul-szczepanek-arm

paul-szczepanek-arm Feb 27, 2019

Author Member

no, really, eclipse regex did this, very unhappy with it now, will write a stern letter to the Eclipse Foundation

@ciarmcom ciarmcom requested review from donatieng, pan- and ARMmbed/mbed-os-maintainers Feb 26, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

@cmonr cmonr added needs: work and removed needs: review labels Feb 27, 2019

@paul-szczepanek-arm paul-szczepanek-arm force-pushed the paul-szczepanek-arm:cordio-host-upgrade branch to 48d2a7c Feb 27, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

based on final 19.02 cordio release

@donatieng
Copy link
Member

left a comment

Thanks @paul-szczepanek-arm - a few minor things but looks good!

@@ -41,6 +41,10 @@
/*! WSF handler ID */
wsfHandlerId_t stack_handler_id;

/* WSF heap allocation */
uint8_t *SystemHeapStart;

This comment has been minimized.

Copy link
@donatieng

donatieng Feb 27, 2019

Member

Could these be made static? (and stack_handler_id above as well)

This comment has been minimized.

Copy link
@paul-szczepanek-arm

paul-szczepanek-arm Feb 27, 2019

Author Member

no, they need to be provided for wsf

Show resolved Hide resolved ...ures/FEATURE_BLE/targets/TARGET_CORDIO/stack/ble-host/include/att_defs.h Outdated
extern "C" {
#endif

/* UART */

This comment has been minimized.

Copy link
@donatieng

donatieng Feb 27, 2019

Member

Please remove these implementations if not needed :)

This comment has been minimized.

Copy link
@paul-szczepanek-arm

paul-szczepanek-arm Feb 27, 2019

Author Member

I can't, we need to provide something for the linker, I will signpost and comment them

@0xc0170 0xc0170 added the risk: A label Feb 27, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

ready for review, testing

@0xc0170 0xc0170 added needs: review and removed needs: work labels Feb 27, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Upgrade of cordio host to new version. Should not impact users and be a in-place upgrade.

Can you be more specific - what version are we updating "from to" ?

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

Added versions but since numbering has clearly changed to year/month style, this is not that important.

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

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 28, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-IAR8
@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

looks like CI problem

@paul-szczepanek-arm paul-szczepanek-arm force-pushed the paul-szczepanek-arm:cordio-host-upgrade branch Feb 28, 2019

/* verify signed write is permitted */
if ((pAttr->settings & ATTS_SET_ALLOW_SIGNED) == 0)
/* verify permissions */
if (attsPermissions(pCcb->connId, ATTS_PERMIT_WRITE, handle, pAttr->permissions) != ATT_SUCCESS)

This comment has been minimized.

Copy link
@pan-

pan- Feb 28, 2019

Member

I remember we made changes here as the check from the stack were incorrect. We should double check if they are correct now

@paul-szczepanek-arm paul-szczepanek-arm force-pushed the paul-szczepanek-arm:cordio-host-upgrade branch Mar 1, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

yes and yes

@paul-szczepanek-arm paul-szczepanek-arm force-pushed the paul-szczepanek-arm:cordio-host-upgrade branch to e247852 Mar 1, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

still testing but ready for CI

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

@pan- Happy with it as it is?

@0xc0170 0xc0170 added needs: review and removed needs: work labels Mar 1, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

he said yes

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

he said yes

@pan- lost his voice? 😀

CI restarted

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Mar 1, 2019

@pan-

pan- approved these changes Mar 1, 2019

Copy link
Member

left a comment

Lets get this in.

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 1, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-IAR8
@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

fixed IAR problem

@paul-szczepanek-arm paul-szczepanek-arm force-pushed the paul-szczepanek-arm:cordio-host-upgrade branch to f740985 Mar 1, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

IAR still complaining (different file), hang on

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

my IAR was out of date, fixed, ready for CI

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

CI restarted

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 1, 2019

Test run: FAILED

Summary: 2 of 14 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_tools-test
  • jenkins-ci/mbed-os-ci_greentea-test
@donatieng

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

Hmmm... Looks like a K66F filesystem test being flaky 😿

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

@donatieng Fortunately, we can restart just jenkins-ci/greentea-test.

It's been restarted.

@cmonr cmonr merged commit 4043623 into ARMmbed:master Mar 2, 2019

28 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARMC5 Success
Details
jenkins-ci/mbed2-build-ARMC6 Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR8 Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9937 cycles
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the needs: CI label Mar 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.