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: Add MTU events #9537

Merged
merged 18 commits into from Feb 15, 2019

Conversation

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

commented Jan 29, 2019

Description

Added events in the gap event handler to inform the user of ATT_MTU and packet payload size changes.

Pull request type

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

Reviewers

Release notes

This adds optional events for informing the user of MTU changes and an option to attempt an MTU negotiation.

There is no effect on existing users; the usage is optional and will not affect users who don't use it. There is no migration path because the events are a new feature.

@paul-szczepanek-arm paul-szczepanek-arm requested a review from pan- Jan 29, 2019

@paul-szczepanek-arm paul-szczepanek-arm referenced this pull request Jan 29, 2019

Closed

Mtu events #60

@cmonr cmonr added the needs: review label Jan 29, 2019

@pan-
Copy link
Member

left a comment

Could you document the ATT MTU and DLE behaviour ? When is it triggered automatically, when it is not ? It would be better to offer a consistent behaviour across ports. If we can't this must be documented.

Show resolved Hide resolved features/FEATURE_BLE/ble/gap/Gap.h Outdated
Show resolved Hide resolved features/FEATURE_BLE/ble/GattClient.h Outdated
@cmonr

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2019

@pan- All good with the updates?

@pan-

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

@cmonr I resolved what has been solved and left open what is not yet solved.

@cmonr cmonr added needs: work and removed needs: review labels Jan 31, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 1, 2019

I rerouted the event handling to gatt server and and client - this is tentative, must review on Monday

Show resolved Hide resolved features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalAttClient.cpp Outdated
@pan-

pan- approved these changes Feb 5, 2019

@pan-

pan- approved these changes Feb 5, 2019

@NirSonnenschein

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2019

@pan- , some additional changes here, can you please review the latest changes.

@pan-

pan- approved these changes Feb 6, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 11, 2019

@0xc0170 the system is not satisfied with Vincent's review sadly, can you help, please?

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Please add Release notes to the first comment, following https://os.mbed.com/docs/mbed-os/v5.11/contributing/workflow.html#functionality-change

@@ -0,0 +1,74 @@
/* mbed Microcontroller Library
* Copyright (c) 2019 ARM Limited
*

This comment has been minimized.

Copy link
@0xc0170

0xc0170 Feb 11, 2019

Member

add SPDX identifier to all new files please

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 11, 2019

thanks - added

@0xc0170 0xc0170 requested a review from AnotherButler Feb 11, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

@AnotherButler Please review

@paul-szczepanek-arm paul-szczepanek-arm force-pushed the paul-szczepanek-arm:mtu-events branch to d801ed3 Feb 14, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

CI restarted

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Feb 14, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 14, 2019

Test run: FAILED

Summary: 1 of 8 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM

@cmonr cmonr added needs: work and removed needs: CI labels Feb 14, 2019

@paul-szczepanek-arm

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2019

looking at the logs looks like CI failure to me

Loading library mbed-os-ci@master
java.io.FileNotFoundException: https://api.github.com/repos/ARMmbed/mbed-os-ci
	at com.squareup.okhttp.internal.huc.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:243)
	at com.squareup.okhttp.internal.huc.DelegatingHttpsURLConnection.getInputStream(DelegatingHttpsURLConnection.java:210)
	at com.squareup.okhttp.internal.huc.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:25)
	at org.kohsuke.github.Requester.parse(Requester.java:617)
	at org.kohsuke.github.Requester.parse(Requester.java:599)
	at org.kohsuke.github.Requester._to(Requester.java:277)
Caused: org.kohsuke.github.GHFileNotFoundException: {
  "message": "Server Error"
}
@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Restarted CI

@cmonr cmonr added needs: CI and removed needs: work labels Feb 15, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 15, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 7
Build artifacts

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Feb 15, 2019

@0xc0170 0xc0170 changed the title BLE: Added MTU events BLE: Add MTU events Feb 15, 2019

@0xc0170 0xc0170 merged commit a7b9490 into ARMmbed:master Feb 15, 2019

27 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-ARM Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(-48 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR 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 10174 cycles (+993 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 (+0.00%)
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
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.