Update NRF ble configuration options #13910
Conversation
|
@marcemmers, thank you for your changes. |
pan-
left a comment
pan-
left a comment
There was a problem hiding this comment.
Thanks @marcemmers, this is a useful addition. LGTM
| "macro_name": "CHCI_TR_CUSTOM" | ||
| } | ||
| }, | ||
| "target_overrides": { |
There was a problem hiding this comment.
Ideally this would be in the NRF5X driver but it is not possible du to the build system limitations.
There was a problem hiding this comment.
Can we fix it in CMake ? If yes, we should
| { | ||
| "name": "cordio-ll-nrf52840", | ||
| "config": { | ||
| "phy-coded-support": { |
There was a problem hiding this comment.
Wouldn't it be possible to preserve backward compatibility by keeping these parameters and overriding the cordio LL one if defined ?
For example in the HCI driver:
#if defined(MBED_CONF_CORDIO_LL_NRF52840_MAX_ACL_SIZE)
#warning "Please override the confirguration parameter cordio-ll.max-acl-size instead of cordio-ll-nrf52840.max-acl-size"
#undef MBED_CONF_CORDIO_LL_MAX_ACL_SIZE
#define MBED_CONF_CORDIO_LL_MAX_ACL_SIZE MBED_CONF_CORDIO_LL_NRF52840_MAX_ACL_SIZEIt would speed up this PR integration.
There was a problem hiding this comment.
One of the reasons for this PR was to get rid of those confusing redefinitions that are already being used in NRFCordioHCIDriver.cpp. In our mbed_config.h we have values specific for the NRF52840 and normal cordio_ll values and its quite hard to follow, especially when not looking at the source code.
It might be better to split the PR into two separate PRs. One will add the options the configure the buffers used in NRFCordioHCIDriver.cpp which stay compatible. The other will be breaking due to the cleaner way of configuring the ACL size etc.
|
CI started |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
6 participants
Summary of changes
This allows for changes to the buffers allocated in
NRFCordioHCIDriver.cpp. Since the amount of buffers can be configured using the mbed_app.json it is only natural to also configure the buffer sizes that go with the number of buffers.I also removed the confusing configuration settings used only by the NRF52840 which overruled other settings made by the cordio-ll config.
Impact of changes
No functionality change, only configuration changes
Migration actions required
All the default settings remained the same and if no custom configuration is used in mbed_app.json the no actions are required.
If you use the NRF52840 and have overruled any of the following settings you may need to change:
"cordio-ll-nrf52840.phy-coded-support"into"cordio-ll.phy-coded-support""cordio-ll-nrf52840.extended-advertising-size"into"cordio-ll.extended-advertising-size""cordio-ll-nrf52840.max-acl-size"into"cordio-ll.max-acl-size""cordio-ll-nrf52840.tx-buffers"into"cordio-ll.tx-buffers"Documentation
Pull request type
Test results
Reviewers