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

Added option to allow writeValue() without response #72

Closed
wants to merge 1 commit into from
Closed

Conversation

bitbank2
Copy link

@bitbank2 bitbank2 commented May 2, 2020

The existing code doesn't allow the writeValue() method to choose writeWithoutResponse when both properties are present. It will use the "with response" option when both properties are present. This lack of choice makes data transfer much slower when you would prefer to send data to a remote device that is not in your control (e.g. one that offers only characteristics with both write and writeWithoutResponse enabled). My patch adds another version of the writeValue() method with a boolean as the last parameter (bool bWithResponse) to allow you to specify false to select writing without a response.

@bitbank2
Copy link
Author

Can someone please have a look? It's a simple fix that should not cause any ill effects.

@facchinm
Copy link
Contributor

Hi Larry,
sorry for the delay.
I'd change the patch this way

diff --git a/src/BLECharacteristic.cpp b/src/BLECharacteristic.cpp
index 34f761f..9b0e777 100644
--- a/src/BLECharacteristic.cpp
+++ b/src/BLECharacteristic.cpp
@@ -234,22 +234,22 @@ int BLECharacteristic::readValue(int32_t& value)
   return readValue((uint8_t*)&value, sizeof(value));
 }
 
-int BLECharacteristic::writeValue(const uint8_t value[], int length)
+int BLECharacteristic::writeValue(const uint8_t value[], int length, bool bWithResponse)
 {
   if (_local) {
     return _local->writeValue(value, length);
   }
 
   if (_remote) {
-    return _remote->writeValue(value, length);
+    return _remote->writeValue(value, length, bWithResponse);
   }
 
   return 0;
 }
 
-int BLECharacteristic::writeValue(const void* value, int length)
+int BLECharacteristic::writeValue(const void* value, int length, bool bWithResponse)
 {
-  return writeValue((const uint8_t*)value, length);
+  return writeValue((const uint8_t*)value, length, bWithResponse);
 }
 
 int BLECharacteristic::writeValue(const char* value)
diff --git a/src/BLECharacteristic.h b/src/BLECharacteristic.h
index 03e1b61..d627b42 100644
--- a/src/BLECharacteristic.h
+++ b/src/BLECharacteristic.h
@@ -68,8 +68,8 @@ public:
   int readValue(uint32_t& value);
   int readValue(int32_t& value);
 
-  int writeValue(const uint8_t value[], int length);
-  int writeValue(const void* value, int length);
+  int writeValue(const uint8_t value[], int length, bool bWithResponse = true);
+  int writeValue(const void* value, int length, bool bWithResponse = true);
   int writeValue(const char* value);
   int writeValue(uint8_t value);
   int writeValue(int8_t value);
diff --git a/src/remote/BLERemoteCharacteristic.cpp b/src/remote/BLERemoteCharacteristic.cpp
index ecad622..9330a94 100644
--- a/src/remote/BLERemoteCharacteristic.cpp
+++ b/src/remote/BLERemoteCharacteristic.cpp
@@ -85,7 +85,7 @@ uint8_t BLERemoteCharacteristic::operator[] (int offset) const
   return 0;
 }
 
-int BLERemoteCharacteristic::writeValue(const uint8_t value[], int length)
+int BLERemoteCharacteristic::writeValue(const uint8_t value[], int length, bool bWithResponse)
 {
   if (!ATT.connected(_connectionHandle)) {
     return false;
@@ -104,7 +104,7 @@ int BLERemoteCharacteristic::writeValue(const uint8_t value[], int length)
     return 0;
   }
 
-  if (_properties & BLEWrite) {
+  if ((_properties & BLEWrite) && bWithResponse) {
     uint8_t resp[4];
     int respLength = ATT.writeReq(_connectionHandle, _valueHandle, value, length, resp);
 
diff --git a/src/remote/BLERemoteCharacteristic.h b/src/remote/BLERemoteCharacteristic.h
index c4bc140..1edeffc 100644
--- a/src/remote/BLERemoteCharacteristic.h
+++ b/src/remote/BLERemoteCharacteristic.h
@@ -38,7 +38,7 @@ public:
   int valueLength() const;
   uint8_t operator[] (int offset) const;
 
-  int writeValue(const uint8_t value[], int length);
+  int writeValue(const uint8_t value[], int length, bool bWithResponse = true);
   int writeValue(const char* value);
 
   bool valueUpdated();

They are equivalent but at least we avoid duplicating a function.

Or, even better, reversing the condition (checking for _properties & BLEWriteWithoutResponse before _properties & BLEWrite) would solve the issue (without giving any choice to the user but being fast at least)

@bitbank2
Copy link
Author

I did the PR with the intention of not breaking older code. I figured that an overloaded function was not a terrible coding choice. Please change it as you see fit. The performance increase is significant, so as long as it's possible to write without response, all will be well :)

@facchinm
Copy link
Contributor

Is the second option viable for your use case (inverting the ifs)? I'd go for that one 🙂

@bitbank2
Copy link
Author

Yes, your C++ foo is better than mine. Please merge the change.

facchinm added a commit to facchinm/ArduinoBLE that referenced this pull request Nov 30, 2020
When a characteristic is declared (Write | WriteWithoutResponse) the code always creates a request
and expects a response.
By setting withResponse=false the user can bypass the request and write a responseless command.

Reworks and supersedes arduino-libraries#72
facchinm added a commit to facchinm/ArduinoBLE that referenced this pull request Nov 30, 2020
When a characteristic is declared (Write | WriteWithoutResponse) the code always creates a request
and expects a response.
By setting withResponse=false the user can bypass the request and write a responseless command.

Reworks and supersedes arduino-libraries#72
@bitbank2 bitbank2 closed this Dec 1, 2020
polldo pushed a commit to facchinm/ArduinoBLE that referenced this pull request Dec 18, 2020
When a characteristic is declared (Write | WriteWithoutResponse) the code always creates a request
and expects a response.
By setting withResponse=false the user can bypass the request and write a responseless command.

Reworks and supersedes arduino-libraries#72
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself conclusion: duplicate Has already been submitted labels May 13, 2022
@wbadry
Copy link

wbadry commented Dec 31, 2023

Hello,
Is there a working example showing how to write a value without waiting for a response? I tried sending string using both flags and there is a delay in both.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: duplicate Has already been submitted topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants