From e1cf516100a31ad0ecea8c6713df1c7c77ebb5da Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 11 Dec 2018 15:08:16 +0000 Subject: [PATCH 1/9] fix max payload and hci length values --- .../ble/gap/AdvertisingParameters.h | 11 ++++++ features/FEATURE_BLE/ble/gap/Gap.h | 6 ++++ features/FEATURE_BLE/ble/generic/GenericGap.h | 5 ++- features/FEATURE_BLE/ble/pal/PalGap.h | 17 ++++++++++ features/FEATURE_BLE/source/gap/Gap.cpp | 6 ++++ .../FEATURE_BLE/source/generic/GenericGap.cpp | 34 ++++++++++--------- .../targets/TARGET_CORDIO/CordioPalGap.h | 4 +++ .../TARGET_CORDIO/source/CordioPalGap.cpp | 10 ++++++ 8 files changed, 76 insertions(+), 17 deletions(-) diff --git a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h index 304abddaad..0141b9164a 100644 --- a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h +++ b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h @@ -124,6 +124,9 @@ public: * A range is provided to the LE subsystem, so it can adjust the advertising * interval with other transmission happening on the BLE radio. * + * @note If CONNECTABLE_UNDIRECTED or CONNECTABLE_DIRECTED advertising is used + * you must use legacy PDU. + * * @note If values in input are out of range, they will be normalized. */ AdvertisingParameters( @@ -162,6 +165,9 @@ public: /** * Update the advertising type. * + * @note If legacy PDU is not used then you cannot use + * CONNECTABLE_UNDIRECTED nor CONNECTABLE_DIRECTED. + * * @param[in] newAdvType The new advertising type. * * @return reference to this object. @@ -448,6 +454,9 @@ public: * * @param enable If true, legacy PDU will be used. * + * @note If CONNECTABLE_UNDIRECTED or CONNECTABLE_DIRECTED advertising is used + * you must use legacy PDU. + * * @return A reference to this object. */ AdvertisingParameters &setUseLegacyPDU(bool enable = true) @@ -490,6 +499,8 @@ public: * * @param enable Advertising anonymous if true. * + * @note You may not use anonymous advertising with periodic advertising on the same set. + * * @return reference to this object. */ AdvertisingParameters &setAnonymousAdvertising(bool enable) diff --git a/features/FEATURE_BLE/ble/gap/Gap.h b/features/FEATURE_BLE/ble/gap/Gap.h index 194369c623..0a5835c6c7 100644 --- a/features/FEATURE_BLE/ble/gap/Gap.h +++ b/features/FEATURE_BLE/ble/gap/Gap.h @@ -538,6 +538,12 @@ public: */ virtual uint16_t getMaxAdvertisingDataLength(); + /** Return maximum advertising data length supported for connectable advertising. + * + * @return Maximum advertising data length supported for connectable advertising. + */ + virtual uint16_t getMaxConnectableAdvertisingDataLength(); + /** Create an advertising set and apply the passed in parameters. The handle returned * by this function must be used for all other calls that accept an advertising handle. * When done with advertising, remove from the system using destroyAdvertisingSet(). diff --git a/features/FEATURE_BLE/ble/generic/GenericGap.h b/features/FEATURE_BLE/ble/generic/GenericGap.h index 8b3fd4af13..7ef9e66080 100644 --- a/features/FEATURE_BLE/ble/generic/GenericGap.h +++ b/features/FEATURE_BLE/ble/generic/GenericGap.h @@ -50,7 +50,6 @@ class GenericGap : public: /* TODO: move to config */ static const uint8_t MAX_ADVERTISING_SETS = 15; - static const size_t MAX_HCI_DATA_LENGTH = 251; /** * Construct a GenericGap. @@ -92,6 +91,10 @@ public: */ virtual uint16_t getMaxAdvertisingDataLength(); + /** @copydoc Gap::getMaxConnectableAdvertisingDataLength + */ + virtual uint16_t getMaxConnectableAdvertisingDataLength(); + /** @copydoc Gap::createAdvertisingSet */ virtual ble_error_t createAdvertisingSet( diff --git a/features/FEATURE_BLE/ble/pal/PalGap.h b/features/FEATURE_BLE/ble/pal/PalGap.h index 0a9a017ae3..be9dade825 100644 --- a/features/FEATURE_BLE/ble/pal/PalGap.h +++ b/features/FEATURE_BLE/ble/pal/PalGap.h @@ -764,6 +764,23 @@ struct Gap { */ virtual uint16_t get_maximum_advertising_data_length() = 0; + /** + * Query the maximum data length the controller supports in an advertising set + * using connectable advertising. + * + * @return The length in byte the controller can support in an advertising set + * for connectable advertising. + */ + virtual uint16_t get_maximum_connectable_advertising_data_length() = 0; + + /** + * Query the maximum payload length for a single HCI packet carrying partial + * (or complete if it fits) data for advertising set. + * + * @return Max size of the HCI packet transporting the data. + */ + virtual uint8_t get_max_hci_advertising_data_length() = 0; + /** * Query the maximum number of concurrent advertising sets that is supported * by the controller. diff --git a/features/FEATURE_BLE/source/gap/Gap.cpp b/features/FEATURE_BLE/source/gap/Gap.cpp index 3515ddd531..6a5d5d4cef 100644 --- a/features/FEATURE_BLE/source/gap/Gap.cpp +++ b/features/FEATURE_BLE/source/gap/Gap.cpp @@ -36,6 +36,12 @@ uint16_t Gap::getMaxAdvertisingDataLength() return LEGACY_ADVERTISING_MAX_SIZE; } +uint16_t Gap::getMaxConnectableAdvertisingDataLength() +{ + /* Requesting action from porter(s): override this API if this capability is supported. */ + return LEGACY_ADVERTISING_MAX_SIZE; +} + ble_error_t Gap::createAdvertisingSet( advertising_handle_t *handle, const AdvertisingParameters ¶meters diff --git a/features/FEATURE_BLE/source/generic/GenericGap.cpp b/features/FEATURE_BLE/source/generic/GenericGap.cpp index a45ce22149..c9bf963439 100644 --- a/features/FEATURE_BLE/source/generic/GenericGap.cpp +++ b/features/FEATURE_BLE/source/generic/GenericGap.cpp @@ -1989,8 +1989,6 @@ void GenericGap::set_connection_event_handler(pal::ConnectionEventMonitor::Event const uint8_t GenericGap::MAX_ADVERTISING_SETS; -const size_t GenericGap::MAX_HCI_DATA_LENGTH; - uint8_t GenericGap::getMaxAdvertisingSetNumber() { useVersionTwoAPI(); @@ -2009,6 +2007,12 @@ uint16_t GenericGap::getMaxAdvertisingDataLength() return _pal_gap.get_maximum_advertising_data_length(); } +uint16_t GenericGap::getMaxConnectableAdvertisingDataLength() +{ + useVersionTwoAPI(); + return _pal_gap.get_maximum_connectable_advertising_data_length(); +} + ble_error_t GenericGap::createAdvertisingSet( advertising_handle_t *handle, const AdvertisingParameters ¶meters @@ -2259,24 +2263,23 @@ ble_error_t GenericGap::setAdvertisingData( &pal::Gap::set_extended_scan_response_data : &pal::Gap::set_extended_advertising_data; - for (size_t i = 0, end = payload.size(); - (i < end) || (i == 0 && end == 0); - i += MAX_HCI_DATA_LENGTH) - { + const size_t hci_length = _pal_gap.get_max_hci_advertising_data_length(); + + for (size_t i = 0, end = payload.size(); (i < end) || (i == 0 && end == 0); i += hci_length) { // select the operation based on the index op_t op(op_t::INTERMEDIATE_FRAGMENT); - if (end < MAX_HCI_DATA_LENGTH) { + if (end < hci_length) { op = op_t::COMPLETE_FRAGMENT; } else if (i == 0) { op = op_t::FIRST_FRAGMENT; - } else if ((end - i) <= MAX_HCI_DATA_LENGTH) { + } else if ((end - i) <= hci_length) { op = op_t::LAST_FRAGMENT; } // extract the payload mbed::Span<const uint8_t> sub_payload = payload.subspan( i, - std::min(MAX_HCI_DATA_LENGTH, (end - i)) + std::min(hci_length, (end - i)) ); // set the payload @@ -2467,24 +2470,23 @@ ble_error_t GenericGap::setPeriodicAdvertisingPayload( typedef pal::advertising_fragment_description_t op_t; - for (size_t i = 0, end = payload.size(); - (i < end) || (i == 0 && end == 0); - i += MAX_HCI_DATA_LENGTH - ) { + const size_t hci_length = _pal_gap.get_max_hci_advertising_data_length(); + + for (size_t i = 0, end = payload.size(); (i < end) || (i == 0 && end == 0); i += hci_length) { // select the operation based on the index op_t op(op_t::INTERMEDIATE_FRAGMENT); - if (end < MAX_HCI_DATA_LENGTH) { + if (end < hci_length) { op = op_t::COMPLETE_FRAGMENT; } else if (i == 0) { op = op_t::FIRST_FRAGMENT; - } else if ((end - i) <= MAX_HCI_DATA_LENGTH) { + } else if ((end - i) <= hci_length) { op = op_t::LAST_FRAGMENT; } // extract the payload mbed::Span<const uint8_t> sub_payload = payload.subspan( i, - std::min(MAX_HCI_DATA_LENGTH, (end - i)) + std::min(hci_length, (end - i)) ); ble_error_t err = _pal_gap.set_periodic_advertising_data( diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h index 02e6910d18..e31a252e1e 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h @@ -221,6 +221,10 @@ public: virtual uint16_t get_maximum_advertising_data_length(); + virtual uint16_t get_maximum_connectable_advertising_data_length(); + + virtual uint8_t get_max_hci_advertising_data_length(); + virtual uint8_t get_max_number_of_advertising_sets(); virtual ble_error_t remove_advertising_set( diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp index c3330e9145..7d2e27f6ae 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp @@ -921,6 +921,16 @@ uint16_t Gap::get_maximum_advertising_data_length() return HciGetMaxAdvDataLen(); } +uint16_t Gap::get_maximum_connectable_advertising_data_length() +{ + return HCI_EXT_ADV_CONN_DATA_LEN; +} + +uint8_t Gap::get_max_hci_advertising_data_length() +{ + return HCI_EXT_ADV_DATA_LEN; +} + uint8_t Gap::get_max_number_of_advertising_sets() { return std::min(HciGetNumSupAdvSets(), (uint8_t) DM_NUM_ADV_SETS); From 7dd90e181098eb66039c2ffedc0c8babfd949cda Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Tue, 11 Dec 2018 17:01:17 +0000 Subject: [PATCH 2/9] Docuement active set length and add informative call for querying the limit --- features/FEATURE_BLE/ble/gap/Gap.h | 19 +++++++++++++++++++ features/FEATURE_BLE/ble/generic/GenericGap.h | 4 ++++ features/FEATURE_BLE/source/gap/Gap.cpp | 6 ++++++ .../FEATURE_BLE/source/generic/GenericGap.cpp | 6 ++++++ 4 files changed, 35 insertions(+) diff --git a/features/FEATURE_BLE/ble/gap/Gap.h b/features/FEATURE_BLE/ble/gap/Gap.h index 0a5835c6c7..2fad7af116 100644 --- a/features/FEATURE_BLE/ble/gap/Gap.h +++ b/features/FEATURE_BLE/ble/gap/Gap.h @@ -544,6 +544,12 @@ public: */ virtual uint16_t getMaxConnectableAdvertisingDataLength(); + /** Return maximum advertising data length you may set if advertising set is active. + * + * @return Maximum advertising data length you may set if advertising set is active. + */ + virtual uint8_t getMaxActiveSetAdvertisingDataLength(); + /** Create an advertising set and apply the passed in parameters. The handle returned * by this function must be used for all other calls that accept an advertising handle. * When done with advertising, remove from the system using destroyAdvertisingSet(). @@ -589,6 +595,10 @@ public: * @param handle Advertising set handle. * @param payload Advertising payload. * + * @note If advertising set is active you may only set payload of length equal or less + * than getMaxActiveSetAdvertisingDataLength(). If you require a longer payload you must + * stop the advertising set, set the payload and restart the set. + * * @return BLE_ERROR_NONE on success. * * @see ble::AdvertisingDataBuilder to build a payload. @@ -604,6 +614,10 @@ public: * @param handle Advertising set handle. * @param response Advertising scan response. * + * @note If advertising set is active you may only set payload of length equal or less + * than getMaxActiveSetAdvertisingDataLength(). If you require a longer payload you must + * stop the advertising set, set the payload and restart the set. + * * @return BLE_ERROR_NONE on success. * * @see ble::AdvertisingDataBuilder to build a payload. @@ -669,6 +683,11 @@ public: * @param payload Advertising payload. * @return BLE_ERROR_NONE on success. * + * @note If advertising set is active you may only set payload of length equal or less + * than getMaxActiveSetAdvertisingDataLength(). If you require a longer payload you must + * stop the advertising set, set the payload and restart the set. Stopping the set will + * cause peers to lose sync on the periodic set. + * * @see ble::AdvertisingDataBuilder to build a payload. * * @version 5+ diff --git a/features/FEATURE_BLE/ble/generic/GenericGap.h b/features/FEATURE_BLE/ble/generic/GenericGap.h index 7ef9e66080..6ce595b53c 100644 --- a/features/FEATURE_BLE/ble/generic/GenericGap.h +++ b/features/FEATURE_BLE/ble/generic/GenericGap.h @@ -95,6 +95,10 @@ public: */ virtual uint16_t getMaxConnectableAdvertisingDataLength(); + /** @copydoc Gap::getMaxActiveSetAdvertisingDataLength + */ + virtual uint8_t getMaxActiveSetAdvertisingDataLength(); + /** @copydoc Gap::createAdvertisingSet */ virtual ble_error_t createAdvertisingSet( diff --git a/features/FEATURE_BLE/source/gap/Gap.cpp b/features/FEATURE_BLE/source/gap/Gap.cpp index 6a5d5d4cef..7b6f1dc02b 100644 --- a/features/FEATURE_BLE/source/gap/Gap.cpp +++ b/features/FEATURE_BLE/source/gap/Gap.cpp @@ -42,6 +42,12 @@ uint16_t Gap::getMaxConnectableAdvertisingDataLength() return LEGACY_ADVERTISING_MAX_SIZE; } +uint8_t Gap::getMaxActiveSetAdvertisingDataLength() +{ + /* Requesting action from porter(s): override this API if this capability is supported. */ + return LEGACY_ADVERTISING_MAX_SIZE; +} + ble_error_t Gap::createAdvertisingSet( advertising_handle_t *handle, const AdvertisingParameters ¶meters diff --git a/features/FEATURE_BLE/source/generic/GenericGap.cpp b/features/FEATURE_BLE/source/generic/GenericGap.cpp index c9bf963439..90c4fef464 100644 --- a/features/FEATURE_BLE/source/generic/GenericGap.cpp +++ b/features/FEATURE_BLE/source/generic/GenericGap.cpp @@ -2013,6 +2013,12 @@ uint16_t GenericGap::getMaxConnectableAdvertisingDataLength() return _pal_gap.get_maximum_connectable_advertising_data_length(); } +uint8_t GenericGap::getMaxActiveSetAdvertisingDataLength() +{ + useVersionTwoAPI(); + return _pal_gap.get_max_hci_advertising_data_length(); +} + ble_error_t GenericGap::createAdvertisingSet( advertising_handle_t *handle, const AdvertisingParameters ¶meters From c5bad804e2fd6ecfb261498b18c4a2dee4a30972 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Wed, 12 Dec 2018 09:57:44 +0000 Subject: [PATCH 3/9] add asserts for illegal compinations of adv params --- features/FEATURE_BLE/ble/gap/AdvertisingParameters.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h index 0141b9164a..ca018abf42 100644 --- a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h +++ b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h @@ -174,6 +174,11 @@ public: */ AdvertisingParameters &setType(advertising_type_t newAdvType) { + if (newAdvType == advertising_type_t::CONNECTABLE_UNDIRECTED || + newAdvType == advertising_type_t::CONNECTABLE_DIRECTED) { + /* these types can only be used with legacy PDUs */ + MBED_ASSERT(_legacyPDU); + } _advType = newAdvType; return *this; } @@ -461,6 +466,12 @@ public: */ AdvertisingParameters &setUseLegacyPDU(bool enable = true) { + if (!enable) { + /* these types can only be used with legacy PDUs */ + MBED_ASSERT((_advType != advertising_type_t::CONNECTABLE_UNDIRECTED) && + (_advType != advertising_type_t::CONNECTABLE_DIRECTED)); + } + _legacyPDU = enable; return *this; } From ff0a2a907e30781b79baebd0f2b32339fbfc6031 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 13 Dec 2018 14:42:55 +0000 Subject: [PATCH 4/9] check connectible sizes --- features/FEATURE_BLE/ble/generic/GenericGap.h | 2 + .../FEATURE_BLE/source/generic/GenericGap.cpp | 44 ++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/features/FEATURE_BLE/ble/generic/GenericGap.h b/features/FEATURE_BLE/ble/generic/GenericGap.h index 6ce595b53c..47fb01777e 100644 --- a/features/FEATURE_BLE/ble/generic/GenericGap.h +++ b/features/FEATURE_BLE/ble/generic/GenericGap.h @@ -787,6 +787,8 @@ private: BitArray<MAX_ADVERTISING_SETS> _existing_sets; BitArray<MAX_ADVERTISING_SETS> _active_sets; BitArray<MAX_ADVERTISING_SETS> _active_periodic_sets; + BitArray<MAX_ADVERTISING_SETS> _connectable_payload_size_exceeded; + BitArray<MAX_ADVERTISING_SETS> _set_is_connectable; // deprecation flags mutable bool _deprecated_scan_api_used : 1; diff --git a/features/FEATURE_BLE/source/generic/GenericGap.cpp b/features/FEATURE_BLE/source/generic/GenericGap.cpp index 90c4fef464..0788aadc1d 100644 --- a/features/FEATURE_BLE/source/generic/GenericGap.cpp +++ b/features/FEATURE_BLE/source/generic/GenericGap.cpp @@ -1380,6 +1380,8 @@ ble_error_t GenericGap::reset(void) _existing_sets.clear(); _active_sets.clear(); _active_periodic_sets.clear(); + _connectable_payload_size_exceeded.clear(); + _set_is_connectable.clear(); /* clear advertising set data on the controller */ _pal_gap.clear_advertising_sets(); @@ -2033,6 +2035,8 @@ ble_error_t GenericGap::createAdvertisingSet( uint8_t new_handle = LEGACY_ADVERTISING_HANDLE + 1; uint8_t end = getMaxAdvertisingSetNumber(); + *handle = INVALID_ADVERTISING_HANDLE; + for (; new_handle < end; ++new_handle) { if (!_existing_sets.get(new_handle)) { ble_error_t err = setExtendedAdvertisingParameters( @@ -2045,11 +2049,11 @@ ble_error_t GenericGap::createAdvertisingSet( _existing_sets.set(new_handle); *handle = new_handle; + return BLE_ERROR_NONE; } } - *handle = INVALID_ADVERTISING_HANDLE; return BLE_ERROR_NO_MEM; } @@ -2086,6 +2090,8 @@ ble_error_t GenericGap::destroyAdvertisingSet(advertising_handle_t handle) return err; } + _connectable_payload_size_exceeded.clear(handle); + _set_is_connectable.clear(handle); _existing_sets.clear(handle); return BLE_ERROR_NONE; } @@ -2140,6 +2146,10 @@ ble_error_t GenericGap::setExtendedAdvertisingParameters( return BLE_ERROR_INVALID_PARAM; } + if (_active_sets.get(handle)) { + return BLE_ERROR_OPERATION_NOT_PERMITTED; + } + pal::advertising_event_properties_t event_properties(params.getType()); event_properties.include_tx_power = params.getTxPowerInHeader(); event_properties.omit_advertiser_address = params.getAnonymousAdvertising(); @@ -2173,6 +2183,12 @@ ble_error_t GenericGap::setExtendedAdvertisingParameters( return err; } + if (event_properties.connectable) { + _set_is_connectable.set(handle); + } else { + _set_is_connectable.clear(handle); + } + return _pal_gap.set_advertising_set_random_address( handle, _random_static_identity_address @@ -2261,9 +2277,30 @@ ble_error_t GenericGap::setAdvertisingData( } if (payload.size() > getMaxAdvertisingDataLength()) { + MBED_WARNING(MBED_ERROR_INVALID_SIZE, "Payload size exceeds getMaxAdvertisingDataLength()."); return BLE_ERROR_INVALID_PARAM; } + if (!_active_sets.get(handle) && payload.size() > getMaxActiveSetAdvertisingDataLength()) { + MBED_WARNING(MBED_ERROR_INVALID_SIZE, "Payload size for active sets needs to fit in a single operation" + " - not greater than getMaxActiveSetAdvertisingDataLength()."); + return BLE_ERROR_INVALID_PARAM; + } + + if (!scan_response) { + if (payload.size() > getMaxConnectableAdvertisingDataLength()) { + if (_active_sets.get(handle) && _set_is_connectable.get(handle)) { + MBED_WARNING(MBED_ERROR_INVALID_SIZE, "Payload size for connectable advertising" + " exceeds getMaxAdvertisingDataLength()."); + return BLE_ERROR_INVALID_PARAM; + } else { + _connectable_payload_size_exceeded.set(handle); + } + } else { + _connectable_payload_size_exceeded.clear(handle); + } + } + // select the pal function set_data_fn_t set_data = scan_response ? &pal::Gap::set_extended_scan_response_data : @@ -2322,6 +2359,11 @@ ble_error_t GenericGap::startAdvertising( return BLE_ERROR_INVALID_PARAM; } + if (_connectable_payload_size_exceeded.get(handle) && _set_is_connectable.get(handle)) { + MBED_WARNING(MBED_ERROR_INVALID_SIZE, "Payload size exceeds size allowed for connectable advertising."); + return BLE_ERROR_INVALID_STATE; + } + if (is_extended_advertising_available()) { error = _pal_gap.extended_advertising_enable( /* enable */ true, From 67db3215007602db813c7d272a888f94162bcc14 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 13 Dec 2018 16:39:24 +0000 Subject: [PATCH 5/9] check illegal adv params combimnation --- .../ble/gap/AdvertisingParameters.h | 23 +++++++++++++++++++ .../FEATURE_BLE/source/generic/GenericGap.cpp | 8 +++++++ 2 files changed, 31 insertions(+) diff --git a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h index ca018abf42..e2efeacf82 100644 --- a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h +++ b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h @@ -161,6 +161,29 @@ public: } public: + /** + * Update the advertising type and whether to use legacy PDU. + * + * @note If legacy PDU is not used then you cannot use + * CONNECTABLE_UNDIRECTED nor CONNECTABLE_DIRECTED. + * + * @param[in] newAdvType The new advertising type. + * + * @param[in] legacy If true, legacy PDU will be used. + * + * @return reference to this object. + */ + AdvertisingParameters &setType(advertising_type_t newAdvType, bool legacy) + { + if (newAdvType == advertising_type_t::CONNECTABLE_UNDIRECTED || + newAdvType == advertising_type_t::CONNECTABLE_DIRECTED) { + /* these types can only be used with legacy PDUs */ + MBED_ASSERT(legacy); + } + _advType = newAdvType; + _legacyPDU = legacy; + return *this; + } /** * Update the advertising type. diff --git a/features/FEATURE_BLE/source/generic/GenericGap.cpp b/features/FEATURE_BLE/source/generic/GenericGap.cpp index 0788aadc1d..e887cde423 100644 --- a/features/FEATURE_BLE/source/generic/GenericGap.cpp +++ b/features/FEATURE_BLE/source/generic/GenericGap.cpp @@ -2150,6 +2150,14 @@ ble_error_t GenericGap::setExtendedAdvertisingParameters( return BLE_ERROR_OPERATION_NOT_PERMITTED; } + /* check for illegal parameter combination */ + if ((params.getType() == advertising_type_t::CONNECTABLE_UNDIRECTED || + params.getType() == advertising_type_t::CONNECTABLE_DIRECTED) && + params.getUseLegacyPDU() == false) { + /* these types can only be used with legacy PDUs */ + return BLE_ERROR_INVALID_PARAM; + } + pal::advertising_event_properties_t event_properties(params.getType()); event_properties.include_tx_power = params.getTxPowerInHeader(); event_properties.omit_advertiser_address = params.getAnonymousAdvertising(); From 8b39071cd8ae7a050b8d42ed685d04742f58e237 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Wed, 2 Jan 2019 14:29:30 +0000 Subject: [PATCH 6/9] make function name longer --- features/FEATURE_BLE/ble/pal/PalGap.h | 2 +- features/FEATURE_BLE/source/generic/GenericGap.cpp | 6 +++--- features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h | 2 +- .../targets/TARGET_CORDIO/source/CordioPalGap.cpp | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/features/FEATURE_BLE/ble/pal/PalGap.h b/features/FEATURE_BLE/ble/pal/PalGap.h index be9dade825..39e06f2622 100644 --- a/features/FEATURE_BLE/ble/pal/PalGap.h +++ b/features/FEATURE_BLE/ble/pal/PalGap.h @@ -779,7 +779,7 @@ struct Gap { * * @return Max size of the HCI packet transporting the data. */ - virtual uint8_t get_max_hci_advertising_data_length() = 0; + virtual uint8_t get_maximum_hci_advertising_data_length() = 0; /** * Query the maximum number of concurrent advertising sets that is supported diff --git a/features/FEATURE_BLE/source/generic/GenericGap.cpp b/features/FEATURE_BLE/source/generic/GenericGap.cpp index e887cde423..17160cfac6 100644 --- a/features/FEATURE_BLE/source/generic/GenericGap.cpp +++ b/features/FEATURE_BLE/source/generic/GenericGap.cpp @@ -2018,7 +2018,7 @@ uint16_t GenericGap::getMaxConnectableAdvertisingDataLength() uint8_t GenericGap::getMaxActiveSetAdvertisingDataLength() { useVersionTwoAPI(); - return _pal_gap.get_max_hci_advertising_data_length(); + return _pal_gap.get_maximum_hci_advertising_data_length(); } ble_error_t GenericGap::createAdvertisingSet( @@ -2314,7 +2314,7 @@ ble_error_t GenericGap::setAdvertisingData( &pal::Gap::set_extended_scan_response_data : &pal::Gap::set_extended_advertising_data; - const size_t hci_length = _pal_gap.get_max_hci_advertising_data_length(); + const size_t hci_length = _pal_gap.get_maximum_hci_advertising_data_length(); for (size_t i = 0, end = payload.size(); (i < end) || (i == 0 && end == 0); i += hci_length) { // select the operation based on the index @@ -2526,7 +2526,7 @@ ble_error_t GenericGap::setPeriodicAdvertisingPayload( typedef pal::advertising_fragment_description_t op_t; - const size_t hci_length = _pal_gap.get_max_hci_advertising_data_length(); + const size_t hci_length = _pal_gap.get_maximum_hci_advertising_data_length(); for (size_t i = 0, end = payload.size(); (i < end) || (i == 0 && end == 0); i += hci_length) { // select the operation based on the index diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h index e31a252e1e..d2df95d860 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGap.h @@ -223,7 +223,7 @@ public: virtual uint16_t get_maximum_connectable_advertising_data_length(); - virtual uint8_t get_max_hci_advertising_data_length(); + virtual uint8_t get_maximum_hci_advertising_data_length(); virtual uint8_t get_max_number_of_advertising_sets(); diff --git a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp index 7d2e27f6ae..18ffd079e5 100644 --- a/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp +++ b/features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioPalGap.cpp @@ -926,7 +926,7 @@ uint16_t Gap::get_maximum_connectable_advertising_data_length() return HCI_EXT_ADV_CONN_DATA_LEN; } -uint8_t Gap::get_max_hci_advertising_data_length() +uint8_t Gap::get_maximum_hci_advertising_data_length() { return HCI_EXT_ADV_DATA_LEN; } From a66ffa34bee91aca9ef363ee1062beace2ee0a12 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Wed, 2 Jan 2019 15:34:50 +0000 Subject: [PATCH 7/9] add constructor suggested by Vincent --- .../ble/gap/AdvertisingParameters.h | 68 +++++++++++++++++-- 1 file changed, 61 insertions(+), 7 deletions(-) diff --git a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h index e2efeacf82..992ecf31dc 100644 --- a/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h +++ b/features/FEATURE_BLE/ble/gap/AdvertisingParameters.h @@ -123,16 +123,20 @@ public: * @param[in] minInterval, maxInterval Time interval between two advertisement. * A range is provided to the LE subsystem, so it can adjust the advertising * interval with other transmission happening on the BLE radio. + * @param[in] useLegacyPDU If true legacy PDU shall be used for advertising. * * @note If CONNECTABLE_UNDIRECTED or CONNECTABLE_DIRECTED advertising is used * you must use legacy PDU. * * @note If values in input are out of range, they will be normalized. + * + * @note If type selected is incompatible with non legacy PDU, legacy PDU will be used. */ AdvertisingParameters( advertising_type_t advType = advertising_type_t::CONNECTABLE_UNDIRECTED, adv_interval_t minInterval = adv_interval_t(DEFAULT_ADVERTISING_INTERVAL_MIN), - adv_interval_t maxInterval = adv_interval_t(DEFAULT_ADVERTISING_INTERVAL_MAX) + adv_interval_t maxInterval = adv_interval_t(DEFAULT_ADVERTISING_INTERVAL_MAX), + bool useLegacyPDU = true ) : _advType(advType), _minInterval(minInterval), @@ -150,14 +154,47 @@ public: _channel39(true), _anonymous(false), _notifyOnScan(false), - _legacyPDU(true), + _legacyPDU(useLegacyPDU), _includeHeaderTxPower(false) { - /* Min interval is slightly larger than in other modes. */ - if (_advType == advertising_type_t::NON_CONNECTABLE_UNDIRECTED) { - _minInterval = adv_interval_t(std::max(_minInterval.value(), GAP_ADV_PARAMS_INTERVAL_MIN_NONCON)); - _maxInterval = adv_interval_t(std::max(_maxInterval.value(), GAP_ADV_PARAMS_INTERVAL_MIN_NONCON)); - } + normalize(); + } + + /** + * Construct an instance of GapAdvertisingParams. + * + * @param[in] advType Type of advertising. + * @param[in] useLegacyPDU If true legacy PDU shall be used for advertising. + * + * @note If CONNECTABLE_UNDIRECTED or CONNECTABLE_DIRECTED advertising is used + * you must use legacy PDU. + * + * @note If type selected is incompatible with non legacy PDU, legacy PDU will be used. + */ + AdvertisingParameters( + advertising_type_t advType, + bool useLegacyPDU + ) : + _advType(advType), + _minInterval(adv_interval_t(DEFAULT_ADVERTISING_INTERVAL_MIN)), + _maxInterval(adv_interval_t(DEFAULT_ADVERTISING_INTERVAL_MAX)), + _peerAddressType(target_peer_address_type_t::PUBLIC), + _ownAddressType(own_address_type_t::RANDOM), + _policy(advertising_filter_policy_t::NO_FILTER), + _primaryPhy(phy_t::LE_1M), + _secondaryPhy(phy_t::LE_1M), + _peerAddress(), + _txPower(127), + _maxSkip(0), + _channel37(true), + _channel38(true), + _channel39(true), + _anonymous(false), + _notifyOnScan(false), + _legacyPDU(useLegacyPDU), + _includeHeaderTxPower(false) + { + normalize(); } public: @@ -552,6 +589,23 @@ public: return _anonymous; } +private: + /** + * Enforce limits on parameters. + */ + void normalize() + { + /* Min interval is slightly larger than in other modes. */ + if (_advType == advertising_type_t::NON_CONNECTABLE_UNDIRECTED) { + _minInterval = adv_interval_t(std::max(_minInterval.value(), GAP_ADV_PARAMS_INTERVAL_MIN_NONCON)); + _maxInterval = adv_interval_t(std::max(_maxInterval.value(), GAP_ADV_PARAMS_INTERVAL_MIN_NONCON)); + } + if (_advType == advertising_type_t::CONNECTABLE_DIRECTED || + _advType == advertising_type_t::CONNECTABLE_UNDIRECTED) { + _legacyPDU = true; + } + } + private: advertising_type_t _advType; /* The advertising interval in ADV duration units (in other words, 0.625ms). */ From dd1d473375954c2dec5c0237609cd5d20de91ecf Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Wed, 2 Jan 2019 15:42:14 +0000 Subject: [PATCH 8/9] future proof return size --- features/FEATURE_BLE/ble/gap/Gap.h | 2 +- features/FEATURE_BLE/ble/generic/GenericGap.h | 2 +- features/FEATURE_BLE/source/gap/Gap.cpp | 2 +- features/FEATURE_BLE/source/generic/GenericGap.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/features/FEATURE_BLE/ble/gap/Gap.h b/features/FEATURE_BLE/ble/gap/Gap.h index 2fad7af116..98f60f4596 100644 --- a/features/FEATURE_BLE/ble/gap/Gap.h +++ b/features/FEATURE_BLE/ble/gap/Gap.h @@ -548,7 +548,7 @@ public: * * @return Maximum advertising data length you may set if advertising set is active. */ - virtual uint8_t getMaxActiveSetAdvertisingDataLength(); + virtual uint16_t getMaxActiveSetAdvertisingDataLength(); /** Create an advertising set and apply the passed in parameters. The handle returned * by this function must be used for all other calls that accept an advertising handle. diff --git a/features/FEATURE_BLE/ble/generic/GenericGap.h b/features/FEATURE_BLE/ble/generic/GenericGap.h index 47fb01777e..e3658f3f49 100644 --- a/features/FEATURE_BLE/ble/generic/GenericGap.h +++ b/features/FEATURE_BLE/ble/generic/GenericGap.h @@ -97,7 +97,7 @@ public: /** @copydoc Gap::getMaxActiveSetAdvertisingDataLength */ - virtual uint8_t getMaxActiveSetAdvertisingDataLength(); + virtual uint16_t getMaxActiveSetAdvertisingDataLength(); /** @copydoc Gap::createAdvertisingSet */ diff --git a/features/FEATURE_BLE/source/gap/Gap.cpp b/features/FEATURE_BLE/source/gap/Gap.cpp index 7b6f1dc02b..090cbb942b 100644 --- a/features/FEATURE_BLE/source/gap/Gap.cpp +++ b/features/FEATURE_BLE/source/gap/Gap.cpp @@ -42,7 +42,7 @@ uint16_t Gap::getMaxConnectableAdvertisingDataLength() return LEGACY_ADVERTISING_MAX_SIZE; } -uint8_t Gap::getMaxActiveSetAdvertisingDataLength() +uint16_t Gap::getMaxActiveSetAdvertisingDataLength() { /* Requesting action from porter(s): override this API if this capability is supported. */ return LEGACY_ADVERTISING_MAX_SIZE; diff --git a/features/FEATURE_BLE/source/generic/GenericGap.cpp b/features/FEATURE_BLE/source/generic/GenericGap.cpp index 17160cfac6..a64f55edf9 100644 --- a/features/FEATURE_BLE/source/generic/GenericGap.cpp +++ b/features/FEATURE_BLE/source/generic/GenericGap.cpp @@ -2015,7 +2015,7 @@ uint16_t GenericGap::getMaxConnectableAdvertisingDataLength() return _pal_gap.get_maximum_connectable_advertising_data_length(); } -uint8_t GenericGap::getMaxActiveSetAdvertisingDataLength() +uint16_t GenericGap::getMaxActiveSetAdvertisingDataLength() { useVersionTwoAPI(); return _pal_gap.get_maximum_hci_advertising_data_length(); From 3be792a3b3d2e6c4a7a83ade84cff8c85074f148 Mon Sep 17 00:00:00 2001 From: paul-szczepanek-arm <33840200+paul-szczepanek-arm@users.noreply.github.com> Date: Thu, 3 Jan 2019 12:24:33 +0000 Subject: [PATCH 9/9] simplify for statement --- features/FEATURE_BLE/source/generic/GenericGap.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/features/FEATURE_BLE/source/generic/GenericGap.cpp b/features/FEATURE_BLE/source/generic/GenericGap.cpp index a64f55edf9..842f2f4e0e 100644 --- a/features/FEATURE_BLE/source/generic/GenericGap.cpp +++ b/features/FEATURE_BLE/source/generic/GenericGap.cpp @@ -2527,8 +2527,11 @@ ble_error_t GenericGap::setPeriodicAdvertisingPayload( typedef pal::advertising_fragment_description_t op_t; const size_t hci_length = _pal_gap.get_maximum_hci_advertising_data_length(); + size_t i = 0; + size_t end = payload.size(); - for (size_t i = 0, end = payload.size(); (i < end) || (i == 0 && end == 0); i += hci_length) { + /* always do at least one iteration for empty payload */ + do { // select the operation based on the index op_t op(op_t::INTERMEDIATE_FRAGMENT); if (end < hci_length) { @@ -2555,7 +2558,9 @@ ble_error_t GenericGap::setPeriodicAdvertisingPayload( if (err) { return err; } - } + + i += hci_length; + } while (i < end); return BLE_ERROR_NONE; }