Peer Manager: fixes for central#449
Conversation
github-actions
Bot
added
changelog-entry-required
|
You can find the documentation preview for this PR here. |
| #endif /* CONFIG_SOFTDEVICE_CENTRAL */ | ||
| #endif /* CONFIG_PM_CENTRAL_FUNCTIONALITY */ | ||
|
|
||
| #ifdef BLE_GAP_ROLE_PERIPH |
There was a problem hiding this comment.
It seems we use three different switches within Peer Manager to decide when to compile functionality releated to the BLE role:
BLE_GAP_ROLE_*defines coming from SoftDeviceSOFTDEVICE_*coming from the SoftDevice KconfigPM_CENTRAL_FUNCTIONALITYcoming in this PR
I think we should stick with one, possibly the SOFTDEVICE_* which can be used everywhere regardless of whether ble_gap.h is included or the Peer Manager is enabled.
There was a problem hiding this comment.
From what I can see, using the SOFTDEVICE_* Kconfig will increase a peripheral application using S145 by ~400 bytes, compared to the option of disabling PM_CENTRA_FUNCTIONALITY.
BLE_GAP_ROLE_* is mainly used for peripheral, which will be defined for all our SoftDevice variants.
I am fine with using SOFTDEVICE_* Kconfig for this.
There was a problem hiding this comment.
From what I can see, using the SOFTDEVICE_* Kconfig will increase a peripheral application using S145 by ~400 bytes, compared to the option of disabling PM_CENTRA_FUNCTIONALITY.
Why would an application that only acts as a peripheral use the S145?
And there shouldn't be any difference in size when using one Kconfig or the other..
BLE_GAP_ROLE_* is mainly used for peripheral, which will be defined for all our SoftDevice variants.
Imo we could either get rid of it entirely or switch on SOFTDEVICE_PERIPHERAL.
There was a problem hiding this comment.
Good discussion. Went back on the introduction of Kconfig PM_CENTRA_FUNCTIONALITY in favor of using SOFTDEVICE_CENTRAL.
Added a commit for updating #ifdef BLE_GAP_ROLE_PERIPH to #if defined(CONFIG_SOFTDEVICE_PERIPHERAL)
anhmolt
force-pushed
the
pm-fix-for-central-role
branch
from
368f88e to
20b6940
Compare
github-actions
Bot
removed
the
doc-required
| const ble_gap_enc_key_t *existing_key = NULL; | ||
| bool lesc = false; | ||
| struct pm_peer_data_bonding bonding_data = { 0 }; | ||
| struct pm_peer_data_bonding bonding_data = {}; |
There was a problem hiding this comment.
use {0} if you want to zero it
| bool lesc = false; | ||
| struct pm_peer_data_bonding bonding_data = { 0 }; | ||
| struct pm_peer_data_bonding bonding_data = {}; | ||
| const uint32_t buf_size = sizeof(struct pm_peer_data_bonding); |
There was a problem hiding this comment.
Why is this a constant if it's passed by reference to pds_peer_data_read()?
Does that function not modify the size?
There was a problem hiding this comment.
This is the signature of pds_peer_data_read:
uint32_t pds_peer_data_read(uint16_t peer_id, enum pm_peer_data_id data_id, struct pm_peer_data *const data, const uint32_t *const buf_len)
so buf_len is constant.
There was a problem hiding this comment.
In nRF5 SDK this would allow setting buf_size to NULL to only have the pointer to memory being copied, but this is not supported with the new file system. So some cleanup is in order here I think.
Fix incorrect pds_peer_data_read() call from the peer manager central specific function link_secure_central_encryption(). Signed-off-by: Andreas Moltumyr <andreas.moltumyr@nordicsemi.no>
Fix use of incorrect logging macro in the security dispatcher link_secure_central_encryption() function. It incorrectly used the nRF5 SDK macro. Signed-off-by: Andreas Moltumyr <andreas.moltumyr@nordicsemi.no>
Add missing central functionality for handling BLE event BLE_GAP_EVT_SEC_REQUEST from the SoftDevice. Signed-off-by: Andreas Moltumyr <andreas.moltumyr@nordicsemi.no>
Switch on Kconfig CONFIG_SOFTDEVICE_PERIPHERAL instead of SoftDevice define BLE_GAP_ROLE_PERIPH when adding peripheral specific code in security dispatcher module of the peer manager. Signed-off-by: Andreas Moltumyr <andreas.moltumyr@nordicsemi.no>
anhmolt
force-pushed
the
pm-fix-for-central-role
branch
from
20b6940 to
ae6bf33
Compare
eivindj-nordic
removed
the
changelog-entry-required
4 participants
Fixes for using Peer Manager with S145 SoftDevice.