Skip to content
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

BLE: wiring minor fixes #1902

Merged
merged 4 commits into from Sep 9, 2019

Conversation

@XuGuohui
Copy link
Contributor

commented Sep 4, 2019

Bugfixes

  • Independent template type of the characteristic and service UUID arguments.
  • Add restriction for the BleCharacteristic::setValue() template type.
  • BLE UUID fails to compare with lower case string.

@XuGuohui XuGuohui added this to the 1.4.0 milestone Sep 4, 2019

@XuGuohui XuGuohui requested a review from avtolstoy Sep 4, 2019

@avtolstoy
Copy link
Member

left a comment

I'm approving but depending on the resolution about is_integral this might require changes.

@@ -386,7 +386,8 @@ class BleCharacteristic {
ssize_t getValue(String& str) const;

template<typename T>
ssize_t getValue(T* val) const {
typename std::enable_if<std::is_integral<T>::value, ssize_t>::type

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Sep 5, 2019

Member

Same comment as in the other PR: #1901 (comment)

template<typename T>
BleCharacteristic(const char* desc, BleCharacteristicProperty properties, T charUuid, T svcUuid, BleOnDataReceivedCallback callback = nullptr, void* context = nullptr) {
template<typename T1, typename T2>
BleCharacteristic(const char* desc, BleCharacteristicProperty properties, T1 charUuid, T2 svcUuid, BleOnDataReceivedCallback callback = nullptr, void* context = nullptr) {

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Sep 5, 2019

Member

Please add wiring/api tests.

@avtolstoy avtolstoy force-pushed the fix/wiring_ble branch from 88d13fc to 53d4d74 Sep 9, 2019

@avtolstoy avtolstoy merged commit 4886e9d into develop Sep 9, 2019

4 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 42.422%
Details

@avtolstoy avtolstoy deleted the fix/wiring_ble branch Sep 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.