Skip to content

Commit 78d4762

Browse files
finger563h2zero
authored andcommitted
fix: #200 Use data()/size() instead of c_str()/length()
refactor to still support fallback of c_str/length for Arduino strings fix notify fix comments Reduce duplication, only allow template function in characteristic and remote value attr if the type is not a pointer (otherwise sizeof is useless). add appropriate notes clean up AttValue::setValue to remove unnecessary length parameter enabling requirement of non-pointer type switch from `requires` to `std::enable_if_t` for compatibility with very old legacy code move back to `std::enable_if` since apparrently these old systems only allow `c++11`?! switch to `std::is_pointer` similarly add missing typename address comments
1 parent a79941c commit 78d4762

File tree

6 files changed

+62
-110
lines changed

6 files changed

+62
-110
lines changed

src/NimBLEAttValue.h

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,20 @@
3838
# error CONFIG_NIMBLE_CPP_ATT_VALUE_INIT_LENGTH cannot be less than 1; Range = 1 : 512
3939
# endif
4040

41+
/* Used to determine if the type passed to a template has a data() and size() method. */
42+
template <typename T, typename = void, typename = void>
43+
struct Has_data_size : std::false_type {};
44+
45+
template <typename T>
46+
struct Has_data_size<T, decltype(void(std::declval<T&>().data())), decltype(void(std::declval<T&>().size()))>
47+
: std::true_type {};
48+
4149
/* Used to determine if the type passed to a template has a c_str() and length() method. */
4250
template <typename T, typename = void, typename = void>
43-
struct Has_c_str_len : std::false_type {};
51+
struct Has_c_str_length : std::false_type {};
4452

4553
template <typename T>
46-
struct Has_c_str_len<T, decltype(void(std::declval<T&>().c_str())), decltype(void(std::declval<T&>().length()))>
54+
struct Has_c_str_length<T, decltype(void(std::declval<T&>().c_str())), decltype(void(std::declval<T&>().length()))>
4755
: std::true_type {};
4856

4957
/**
@@ -216,39 +224,18 @@ class NimBLEAttValue {
216224
/**
217225
* @brief Template to set value to the value of <type\>val.
218226
* @param [in] s The <type\>value to set.
219-
* @param [in] len The length of the value in bytes, defaults to sizeof(T).
220-
* @details Only used for types without a `c_str()` method.
227+
* @note This function is only availabe if the type T is not a pointer.
221228
*/
222229
template <typename T>
223-
# ifdef _DOXYGEN_
224-
bool
225-
# else
226-
typename std::enable_if<!Has_c_str_len<T>::value, bool>::type
227-
# endif
228-
setValue(const T& s, uint16_t len = 0) {
229-
if (len == 0) {
230-
len = sizeof(T);
231-
}
232-
return setValue(reinterpret_cast<const uint8_t*>(&s), len);
233-
}
234-
235-
/**
236-
* @brief Template to set value to the value of <type\>val.
237-
* @param [in] s The <type\>value to set.
238-
* @param [in] len The length of the value in bytes, defaults to string.length().
239-
* @details Only used if the <type\> has a `c_str()` method.
240-
*/
241-
template <typename T>
242-
# ifdef _DOXYGEN_
243-
bool
244-
# else
245-
typename std::enable_if<Has_c_str_len<T>::value, bool>::type
246-
# endif
247-
setValue(const T& s, uint16_t len = 0) {
248-
if (len == 0) {
249-
len = s.length();
230+
typename std::enable_if<!std::is_pointer<T>::value, bool>::type
231+
setValue(const T& s) {
232+
if constexpr (Has_data_size<T>::value) {
233+
return setValue(reinterpret_cast<const uint8_t*>(s.data()), s.size());
234+
} else if constexpr (Has_c_str_length<T>::value) {
235+
return setValue(reinterpret_cast<const uint8_t*>(s.c_str()), s.length());
236+
} else {
237+
return setValue(reinterpret_cast<const uint8_t*>(&s), sizeof(s));
250238
}
251-
return setValue(reinterpret_cast<const uint8_t*>(s.c_str()), len);
252239
}
253240

254241
/**

src/NimBLECharacteristic.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -266,16 +266,6 @@ void NimBLECharacteristic::indicate(const uint8_t* value, size_t length, uint16_
266266
sendValue(value, length, false, conn_handle);
267267
} // indicate
268268

269-
/**
270-
* @brief Send an indication.
271-
* @param[in] value A std::vector<uint8_t> containing the value to send as the notification value.
272-
* @param[in] conn_handle Connection handle to send an individual indication, or BLE_HS_CONN_HANDLE_NONE to send
273-
* the indication to all subscribed clients.
274-
*/
275-
void NimBLECharacteristic::indicate(const std::vector<uint8_t>& value, uint16_t conn_handle) const {
276-
sendValue(value.data(), value.size(), false, conn_handle);
277-
} // indicate
278-
279269
/**
280270
* @brief Send a notification.
281271
* @param[in] conn_handle Connection handle to send an individual notification, or BLE_HS_CONN_HANDLE_NONE to send
@@ -296,16 +286,6 @@ void NimBLECharacteristic::notify(const uint8_t* value, size_t length, uint16_t
296286
sendValue(value, length, true, conn_handle);
297287
} // indicate
298288

299-
/**
300-
* @brief Send a notification.
301-
* @param[in] value A std::vector<uint8_t> containing the value to send as the notification value.
302-
* @param[in] conn_handle Connection handle to send an individual notification, or BLE_HS_CONN_HANDLE_NONE to send
303-
* the notification to all subscribed clients.
304-
*/
305-
void NimBLECharacteristic::notify(const std::vector<uint8_t>& value, uint16_t conn_handle) const {
306-
sendValue(value.data(), value.size(), true, conn_handle);
307-
} // notify
308-
309289
/**
310290
* @brief Sends a notification or indication.
311291
* @param[in] value A pointer to the data to send.

src/NimBLECharacteristic.h

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,8 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute {
5656
void setCallbacks(NimBLECharacteristicCallbacks* pCallbacks);
5757
void indicate(uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const;
5858
void indicate(const uint8_t* value, size_t length, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const;
59-
void indicate(const std::vector<uint8_t>& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const;
6059
void notify(uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const;
6160
void notify(const uint8_t* value, size_t length, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const;
62-
void notify(const std::vector<uint8_t>& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const;
6361

6462
NimBLEDescriptor* createDescriptor(const char* uuid,
6563
uint32_t properties = NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::WRITE,
@@ -77,36 +75,47 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute {
7775
/*********************** Template Functions ************************/
7876

7977
/**
80-
* @brief Template to send a notification from a class type that has a c_str() and length() method.
78+
* @brief Template to send a notification for classes which may have
79+
* data()/size() or c_str()/length() methods. Falls back to sending
80+
* the data by casting the first element of the array to a uint8_t
81+
* pointer and getting the length of the array using sizeof.
8182
* @tparam T The a reference to a class containing the data to send.
8283
* @param[in] value The <type\>value to set.
83-
* @param[in] is_notification if true sends a notification, false sends an indication.
84-
* @details Only used if the <type\> has a `c_str()` method.
84+
* @param[in] conn_handle The connection handle to send the notification to.
85+
* @note This function is only available if the type T is not a pointer.
8586
*/
8687
template <typename T>
87-
# ifdef _DOXYGEN_
88-
void
89-
# else
90-
typename std::enable_if<Has_c_str_len<T>::value, void>::type
91-
# endif
92-
notify(const T& value, bool is_notification = true) const {
93-
notify(reinterpret_cast<const uint8_t*>(value.c_str()), value.length(), is_notification);
88+
typename std::enable_if<!std::is_pointer<T>::value, void>::type
89+
notify(const T& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const {
90+
if constexpr (Has_data_size<T>::value) {
91+
notify(reinterpret_cast<const uint8_t*>(value.data()), value.size(), conn_handle);
92+
} else if constexpr (Has_c_str_length<T>::value) {
93+
notify(reinterpret_cast<const uint8_t*>(value.c_str()), value.length(), conn_handle);
94+
} else {
95+
notify(reinterpret_cast<const uint8_t*>(&value), sizeof(value), conn_handle);
96+
}
9497
}
9598

9699
/**
97-
* @brief Template to send an indication from a class type that has a c_str() and length() method.
100+
* @brief Template to send an indication for classes which may have
101+
* data()/size() or c_str()/length() methods. Falls back to sending
102+
* the data by casting the first element of the array to a uint8_t
103+
* pointer and getting the length of the array using sizeof.
98104
* @tparam T The a reference to a class containing the data to send.
99105
* @param[in] value The <type\>value to set.
100-
* @details Only used if the <type\> has a `c_str()` method.
106+
* @param[in] conn_handle The connection handle to send the indication to.
107+
* @note This function is only available if the type T is not a pointer.
101108
*/
102109
template <typename T>
103-
# ifdef _DOXYGEN_
104-
void
105-
# else
106-
typename std::enable_if<Has_c_str_len<T>::value, void>::type
107-
# endif
108-
indicate(const T& value) const {
109-
indicate(reinterpret_cast<const uint8_t*>(value.c_str()), value.length());
110+
typename std::enable_if<!std::is_pointer<T>::value, void>::type
111+
indicate(const T& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const {
112+
if constexpr (Has_data_size<T>::value) {
113+
indicate(reinterpret_cast<const uint8_t*>(value.data()), value.size(), conn_handle);
114+
} else if constexpr (Has_c_str_length<T>::value) {
115+
indicate(reinterpret_cast<const uint8_t*>(value.c_str()), value.length(), conn_handle);
116+
} else {
117+
indicate(reinterpret_cast<const uint8_t*>(&value), sizeof(value), conn_handle);
118+
}
110119
}
111120

112121
private:

src/NimBLEHIDDevice.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,12 @@ NimBLECharacteristic* NimBLEHIDDevice::batteryLevel() {
225225
return m_batteryLevelCharacteristic;
226226
}
227227

228-
/*
229-
230-
BLECharacteristic* BLEHIDDevice::reportMap() {
228+
NimBLECharacteristic* NimBLEHIDDevice::reportMap() {
231229
return m_reportMapCharacteristic;
232230
}
233231

232+
/*
233+
234234
BLECharacteristic* BLEHIDDevice::pnp() {
235235
return m_pnpCharacteristic;
236236
}

src/NimBLEHIDDevice.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class NimBLEHIDDevice {
6060
void setBatteryLevel(uint8_t level);
6161

6262

63-
//NimBLECharacteristic* reportMap();
63+
NimBLECharacteristic* reportMap();
6464
NimBLECharacteristic* hidControl();
6565
NimBLECharacteristic* inputReport(uint8_t reportID);
6666
NimBLECharacteristic* outputReport(uint8_t reportID);

src/NimBLERemoteValueAttribute.h

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,6 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute {
6363
*/
6464
bool writeValue(const uint8_t* data, size_t length, bool response = false) const;
6565

66-
/**
67-
* @brief Write a new value to the remote characteristic from a std::vector<uint8_t>.
68-
* @param [in] vec A std::vector<uint8_t> value to write to the remote characteristic.
69-
* @param [in] response Whether we require a response from the write.
70-
* @return false if not connected or otherwise cannot perform write.
71-
*/
72-
bool writeValue(const std::vector<uint8_t>& v, bool response = false) const {
73-
return writeValue(&v[0], v.size(), response);
74-
}
75-
7666
/**
7767
* @brief Write a new value to the remote characteristic from a const char*.
7868
* @param [in] str A character string to write to the remote characteristic.
@@ -88,32 +78,18 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute {
8878
* @brief Template to set the remote characteristic value to <type\>val.
8979
* @param [in] s The value to write.
9080
* @param [in] response True == request write response.
91-
* @details Only used for non-arrays and types without a `c_str()` method.
81+
* @note This function is only available if the type T is not a pointer.
9282
*/
9383
template <typename T>
94-
# ifdef _DOXYGEN_
95-
bool
96-
# else
97-
typename std::enable_if<!std::is_array<T>::value && !Has_c_str_len<T>::value, bool>::type
98-
# endif
84+
typename std::enable_if<!std::is_pointer<T>::value, bool>::type
9985
writeValue(const T& v, bool response = false) const {
100-
return writeValue(reinterpret_cast<const uint8_t*>(&v), sizeof(T), response);
101-
}
102-
103-
/**
104-
* @brief Template to set the remote characteristic value to <type\>val.
105-
* @param [in] s The value to write.
106-
* @param [in] response True == request write response.
107-
* @details Only used if the <type\> has a `c_str()` method.
108-
*/
109-
template <typename T>
110-
# ifdef _DOXYGEN_
111-
bool
112-
# else
113-
typename std::enable_if<Has_c_str_len<T>::value, bool>::type
114-
# endif
115-
writeValue(const T& s, bool response = false) const {
116-
return writeValue(reinterpret_cast<const uint8_t*>(s.c_str()), s.length(), response);
86+
if constexpr (Has_data_size<T>::value) {
87+
return writeValue(reinterpret_cast<const uint8_t*>(v.data()), v.size(), response);
88+
} else if constexpr (Has_c_str_length<T>::value) {
89+
return writeValue(reinterpret_cast<const uint8_t*>(v.c_str()), v.length(), response);
90+
} else {
91+
return writeValue(reinterpret_cast<const uint8_t*>(&v), sizeof(v), response);
92+
}
11793
}
11894

11995
/**

0 commit comments

Comments
 (0)