Skip to content

Ble extended advertising fixes #8998

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

Merged
merged 28 commits into from
Dec 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d9d4a21
BLE: Revert change in stack setup initialisation.
pan- Dec 6, 2018
0543442
BLE: Fix conversion of advertising type
pan- Dec 6, 2018
c83dccf
BLE: Set advertising random address when appropriate.
pan- Dec 6, 2018
d372f16
fix return value
paul-szczepanek-arm Dec 6, 2018
58c7c38
check return value of DmSyncStart
paul-szczepanek-arm Dec 6, 2018
5735456
fix swapped errors
paul-szczepanek-arm Dec 6, 2018
22a117a
always set filter policy
paul-szczepanek-arm Dec 6, 2018
df443c2
copy periodic payload
paul-szczepanek-arm Dec 6, 2018
df95a1f
BLE: Allow null value for periodic interval in advertising report event.
pan- Dec 7, 2018
698447b
BLE: Fix address type allowed to create a periodic sync.
pan- Dec 7, 2018
1c71713
BLE: Fix recursion in ble::advertising_data_status_t raw constructor.
pan- Dec 7, 2018
0d398bc
BLE: Revert changes introduced by debugging.
pan- Dec 7, 2018
a36b04f
BLE: Add an option to inject the random static address during the res…
pan- Dec 7, 2018
20e0cd1
BLE: Inject random static address during reset sequence.
pan- Dec 7, 2018
57b79d9
BLE: Set the number of supported phy by the host to 3.
pan- Dec 7, 2018
4e5240b
BLE: Set the number of the advertising sets supported by the host to 3.
pan- Dec 7, 2018
e7f81fe
BLE: Fix the number of advertising sets supported
pan- Dec 7, 2018
c13dcf3
baseband clock rate increased to 1 000 000
paul-szczepanek-arm Dec 7, 2018
bdabada
added arm version of libs
paul-szczepanek-arm Dec 7, 2018
a483696
iar libs for cordio
paul-szczepanek-arm Dec 7, 2018
674ff28
Remove duplicate symbols in libcordio_stack.a
Dec 7, 2018
3f00595
Remove use of GPIOs (LEDs and diag pins) from Cordio LL for Nordic
Dec 7, 2018
db6b09a
ARMCC only likes armar
Dec 7, 2018
6f94339
Replace ARMCC libs
Dec 8, 2018
642b2df
working libs for GCC and IAR
paul-szczepanek-arm Dec 8, 2018
97df8f5
working ARM lib
paul-szczepanek-arm Dec 10, 2018
c998287
fixed ARM compilation problem caused by noreturn
paul-szczepanek-arm Dec 10, 2018
a545da7
remove noreturn error function to avoid compilation issues
paul-szczepanek-arm Dec 10, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions features/FEATURE_BLE/ble/gap/Events.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ struct AdvertisingReportEvent {
advertising_sid_t SID,
advertising_power_t txPower,
rssi_t rssi,
periodic_interval_t periodicInterval,
uint16_t periodicInterval,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd. Why not replace/update the constructor instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see, that constructor is not part of the documentation because it is not meant to be used by user code. We cannot use friend in this case because friend cannot be inherited and we don't know the vendor file that will use it. There is no reason for us to keep non usable constructor if it is not part of the public API.

To give a bit more context, the periodic interval returned in an advertising event can be equal to 0 if there is no periodic interval or a value comprised between [0x0006 : 0xFFFF]. The type periodic_interval_t is not able to represent the absence of value as it normalizes values passed in input.

const peer_address_type_t &directAddressType,
const address_t &directAddress,
const mbed::Span<const uint8_t> &advertisingData
Expand Down Expand Up @@ -133,10 +133,15 @@ struct AdvertisingReportEvent {
return rssi;
}

/** Indicate if periodic interval is valid */
bool isPeriodicIntervalPresent() const {
return periodicInterval != 0;
}

/** Get interval. */
periodic_interval_t getPeriodicInterval() const
{
return periodicInterval;
return periodic_interval_t(periodicInterval);
}

/** Get target address type in directed advertising. */
Expand Down Expand Up @@ -166,7 +171,7 @@ struct AdvertisingReportEvent {
advertising_sid_t SID;
advertising_power_t txPower;
rssi_t rssi;
periodic_interval_t periodicInterval;
uint16_t periodicInterval;
peer_address_type_t directAddressType;
const address_t &directAddress;
mbed::Span<const uint8_t> advertisingData;
Expand Down
2 changes: 1 addition & 1 deletion features/FEATURE_BLE/ble/gap/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ struct advertising_data_status_t : SafeEnum<advertising_data_status_t, uint8_t>
* Explicit constructor from a raw value.
*/
explicit advertising_data_status_t(uint8_t raw_value) :
SafeEnum(static_cast<advertising_data_status_t>(raw_value))
SafeEnum(raw_value)
{
}

Expand Down
3 changes: 2 additions & 1 deletion features/FEATURE_BLE/source/BLE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ BLE::initImplementation(FunctionPointerWithContext<InitializationCompleteCallbac

// this stub is required by ARMCC otherwise link will systematically fail
MBED_WEAK BLEInstanceBase* createBLEInstance() {
MBED_ERROR(MBED_MAKE_ERROR(MBED_MODULE_BLE, MBED_ERROR_CODE_BLE_BACKEND_CREATION_FAILED), "Please provide an implementation for mbed BLE");
MBED_ASSERT("No BLE instance implementation.");
printf("Please provide an implementation for mbed BLE");
return NULL;
}

Expand Down
32 changes: 13 additions & 19 deletions features/FEATURE_BLE/source/generic/GenericGap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1589,7 +1589,7 @@ void GenericGap::on_advertising_report(const pal::GapAdvertisingReportEvent &e)
/* SID - NO ADI FIELD IN THE PDU */ 0xFF,
/* tx power information not available */ 127,
advertising.rssi,
/* NO PERIODIC ADVERTISING */ periodic_interval_t(0),
/* NO PERIODIC ADVERTISING */ 0,
peer_address_type_t::ANONYMOUS,
ble::address_t (),
mbed::Span<const uint8_t>(advertising.data.data(), advertising.data.size())
Expand Down Expand Up @@ -2141,7 +2141,7 @@ ble_error_t GenericGap::setExtendedAdvertisingParameters(
params.getChannel39()
);

return _pal_gap.set_extended_advertising_parameters(
ble_error_t err = _pal_gap.set_extended_advertising_parameters(
handle,
event_properties,
params.getMinPrimaryInterval().value(),
Expand All @@ -2158,6 +2158,15 @@ ble_error_t GenericGap::setExtendedAdvertisingParameters(
/* SID */ (handle % 0x10),
params.getScanRequestNotification()
);

if (err) {
return err;
}

return _pal_gap.set_advertising_set_random_address(
handle,
_random_static_identity_address
);
}

ble_error_t GenericGap::setAdvertisingPayload(
Expand Down Expand Up @@ -2305,21 +2314,6 @@ ble_error_t GenericGap::startAdvertising(
}

if (is_extended_advertising_available()) {
ble::address_t random_address;

if (!getUnresolvableRandomAddress(random_address)) {
return BLE_ERROR_INTERNAL_STACK_FAILURE;
}

error = _pal_gap.set_advertising_set_random_address(
handle,
random_address
);

if (error) {
return error;
}

error = _pal_gap.extended_advertising_enable(
/* enable */ true,
/* number of advertising sets */ 1,
Expand Down Expand Up @@ -2648,7 +2642,7 @@ void GenericGap::on_extended_advertising_report(
advertising_sid,
tx_power,
rssi,
periodic_interval_t(periodic_advertising_interval),
periodic_advertising_interval,
(PeerAddressType_t::type) direct_address_type.value(),
(BLEProtocol::AddressBytes_t &) direct_address,
mbed::make_Span(data, data_length)
Expand Down Expand Up @@ -2927,7 +2921,7 @@ ble_error_t GenericGap::createSync(
return BLE_ERROR_NOT_IMPLEMENTED;
}

if (peerAddressType != peer_address_type_t::PUBLIC ||
if (peerAddressType != peer_address_type_t::PUBLIC &&
peerAddressType != peer_address_type_t::RANDOM
) {
return BLE_ERROR_INVALID_PARAM;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <stddef.h>
#include <string.h>

#include "CordioBLE.h"
#include "CordioHCIDriver.h"
#include "hci_api.h"
#include "hci_cmd.h"
Expand Down Expand Up @@ -139,10 +140,26 @@ void CordioHCIDriver::handle_reset_sequence(uint8_t *pMsg)
HciReadBdAddrCmd();
break;

case HCI_OPCODE_READ_BD_ADDR:
case HCI_OPCODE_READ_BD_ADDR: {
/* parse and store event parameters */
BdaCpy(hciCoreCb.bdAddr, pMsg);

ble::address_t static_address;

if (get_random_static_address(static_address)) {
// note: will send the HCI command to send the random address
cordio::BLE::deviceInstance().getGap().setAddress(
BLEProtocol::AddressType::RANDOM_STATIC,
static_address.data()
);
} else {
/* send next command in sequence */
HciLeReadBufSizeCmd();
}
break;
}

case HCI_OPCODE_LE_SET_RAND_ADDR:
/* send next command in sequence */
HciLeReadBufSizeCmd();
break;
Expand Down Expand Up @@ -246,6 +263,11 @@ void CordioHCIDriver::handle_reset_sequence(uint8_t *pMsg)
}
}

bool CordioHCIDriver::get_random_static_address(ble::address_t& address)
{
return false;
}

void CordioHCIDriver::signal_reset_sequence_done()
{
hci_mbed_os_signal_reset_sequence_done();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <stddef.h>
#include <stdint.h>
#include <BLETypes.h>
#include "wsf_buf.h"
#include "CordioHCITransportDriver.h"

Expand Down Expand Up @@ -108,6 +109,13 @@ class CordioHCIDriver {
*/
virtual void handle_reset_sequence(uint8_t *msg);

/**
* Get the random static address of the controller
*
* @return false if the address has not been set and true otherwise.
*/
virtual bool get_random_static_address(ble::address_t& address);

/**
* Signal to the stack that the reset sequence has been done.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ wsfHandlerId_t stack_handler_id;
*/
MBED_WEAK ble::vendor::cordio::CordioHCIDriver& ble_cordio_get_hci_driver()
{
error("Please provide an implementation for the HCI driver");
MBED_ASSERT("No HCI driver");
printf("Please provide an implementation for the HCI driver");
ble::vendor::cordio::CordioHCIDriver* bad_instance = NULL;
return *bad_instance;
}
Expand Down Expand Up @@ -100,7 +101,7 @@ BLE::BLE(CordioHCIDriver& hci_driver) :
_last_update_us(0)
{
_hci_driver = &hci_driver;

stack_setup();
}

BLE::~BLE() { }
Expand Down Expand Up @@ -393,7 +394,6 @@ void BLE::stack_setup()
void BLE::start_stack_reset()
{
_hci_driver->initialize();
stack_setup();
DmDevReset();
}

Expand Down
97 changes: 84 additions & 13 deletions features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,76 @@ ble_error_t Gap::set_extended_advertising_parameters(
bool scan_request_notification
)
{
uint8_t adv_type;

if (event_properties.use_legacy_pdu) {
if (event_properties.directed == false) {
if (event_properties.high_duty_cycle) {
return BLE_ERROR_INVALID_PARAM;
}

if (event_properties.connectable && event_properties.scannable == false) {
return BLE_ERROR_INVALID_PARAM;
}

if (event_properties.connectable && event_properties.scannable) {
adv_type = DM_ADV_CONN_UNDIRECT;
} else if (event_properties.scannable) {
adv_type = DM_ADV_SCAN_UNDIRECT;
} else {
adv_type = DM_ADV_NONCONN_UNDIRECT;
}
} else {
if (event_properties.scannable) {
return BLE_ERROR_INVALID_PARAM;
}

if (event_properties.connectable == false) {
return BLE_ERROR_INVALID_PARAM;
}

if (event_properties.high_duty_cycle) {
adv_type = DM_ADV_CONN_DIRECT;
} else {
adv_type = DM_ADV_CONN_DIRECT_LO_DUTY;
}
}
} else {
if (event_properties.directed == false) {
if (event_properties.high_duty_cycle) {
return BLE_ERROR_INVALID_PARAM;
}

if (event_properties.connectable && event_properties.scannable) {
adv_type = DM_ADV_CONN_UNDIRECT;
} else if (event_properties.scannable) {
adv_type = DM_ADV_SCAN_UNDIRECT;
} else if (event_properties.connectable) {
adv_type = DM_EXT_ADV_CONN_UNDIRECT;
} else {
adv_type = DM_ADV_NONCONN_UNDIRECT;
}
} else {
// note: not sure how to act with the high duty cycle in scannable
// and non connectable mode. These cases looks correct from a Bluetooth
// standpoint

if (event_properties.connectable && event_properties.scannable) {
return BLE_ERROR_INVALID_PARAM;
} else if (event_properties.connectable) {
if (event_properties.high_duty_cycle) {
adv_type = DM_ADV_CONN_DIRECT;
} else {
adv_type = DM_ADV_CONN_DIRECT_LO_DUTY;
}
} else if (event_properties.scannable) {
adv_type = DM_EXT_ADV_SCAN_DIRECT;
} else {
adv_type = DM_EXT_ADV_NONCONN_DIRECT;
}
}
}

DmAdvSetInterval(
advertising_handle,
primary_advertising_interval_min,
Expand Down Expand Up @@ -709,7 +779,7 @@ ble_error_t Gap::set_extended_advertising_parameters(

DmAdvConfig(
advertising_handle,
event_properties.value(), // TODO: use the raw value here ???
adv_type,
peer_address_type.value(),
const_cast<uint8_t *>(peer_address.data())
);
Expand Down Expand Up @@ -853,7 +923,7 @@ uint16_t Gap::get_maximum_advertising_data_length()

uint8_t Gap::get_max_number_of_advertising_sets()
{
return HciGetNumSupAdvSets();
return std::min(HciGetNumSupAdvSets(), (uint8_t) DM_NUM_ADV_SETS);
}

ble_error_t Gap::remove_advertising_set(advertising_handle_t advertising_handle)
Expand Down Expand Up @@ -911,12 +981,11 @@ ble_error_t Gap::extended_scan_enable(
if (enable) {
uint32_t duration_ms = duration * 10;

DmScanModeExt();
DmScanStart(
scanning_phys.value(),
DM_DISC_MODE_NONE,
extended_scan_type,
filter_duplicates.value(), // TODO: cordio API incomplete ???
filter_duplicates.value(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any more TODOs to update? 😜

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why I removed it, the comment still stands 😆 . These TODO's and FIXME are important in our implementations and internal files when we work with APIs that do not implement all Bluetooth requirements or when we deliver subsets of BT features.

duration_ms > 0xFFFF ? 0xFFFF : duration_ms,
period
);
Expand All @@ -936,23 +1005,25 @@ ble_error_t Gap::periodic_advertising_create_sync(
uint16_t sync_timeout
)
{
if (use_periodic_advertiser_list) {
DmDevSetExtFilterPolicy(
DM_ADV_HANDLE_DEFAULT,
DM_FILT_POLICY_MODE_SYNC,
HCI_FILT_PER_ADV_LIST
);
}
DmDevSetExtFilterPolicy(
DM_ADV_HANDLE_DEFAULT,
DM_FILT_POLICY_MODE_SYNC,
use_periodic_advertiser_list ? HCI_FILT_PER_ADV_LIST : HCI_FILT_NONE
);

DmSyncStart(
dmSyncId_t sync_id = DmSyncStart(
advertising_sid,
peer_address_type.value(),
peer_address.data(),
allowed_skip,
sync_timeout
);

return BLE_ERROR_INVALID_PARAM;
if (sync_id == DM_SYNC_ID_NONE) {
return BLE_ERROR_INTERNAL_STACK_FAILURE;
} else {
return BLE_ERROR_NONE;
}
}

ble_error_t Gap::cancel_periodic_advertising_create_sync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ extern "C" {

/*! \brief Number of supported advertising sets: must be set to 1 for legacy advertising */
#ifndef DM_NUM_ADV_SETS
#define DM_NUM_ADV_SETS 1
#define DM_NUM_ADV_SETS 3
#endif

/*! \brief Number of scanner and initiator PHYs (LE 1M, LE 2M and LE Coded): must be set to 1 for
legacy scanner and initiator */
#ifndef DM_NUM_PHYS
#define DM_NUM_PHYS 1
#define DM_NUM_PHYS 3
#endif
/**@}*/

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ typedef struct
uint8_t advHandle;
uint8_t op;
uint8_t len;
uint8_t *pData;
uint8_t pData[];
} dmAdvPerApiSetData_t;

/* Data structure for DM_ADV_PER_MSG_API_START */
Expand Down
Loading