ARMmbed / ble-nrf51822

Nordic stack and drivers for the mbed BLE_API
Other
46 stars 51 forks source link

Detailed errors on write. #45

Closed fabiencomte closed 8 years ago

fabiencomte commented 8 years ago

As discussed on issue posted this morning.

fabiencomte commented 8 years ago

It's more natural to select the connection you want when you have multiple connections.

rgrover commented 8 years ago

Fabein,

How about the following diff instead? Could you please review? And could you please re-submit your pull request against the develop branch? It will then find its way into master with the next release.

diff --git a/source/nRF5xGattServer.cpp b/source/nRF5xGattServer.cpp
index d5e7151..75dae3a 100644
--- a/source/nRF5xGattServer.cpp
+++ b/source/nRF5xGattServer.cpp
@@ -204,7 +204,6 @@ ble_error_t nRF5xGattServer::write(GattAttribute::Handle_t attributeHandle, cons

 ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute::Handle_t attributeHandle, const uint8_t buffer[], uint16_t len, bool localOnly)
 {
-    uint16_t gapConnectionHandle = nRF5xGap::getInstance().getConnectionHandle();
     ble_error_t returnValue = BLE_ERROR_NONE;

     ble_gatts_value_t value = {
@@ -223,8 +222,7 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute

     int characteristicIndex = resolveValueHandleToCharIndex(attributeHandle);
     if ((characteristicIndex != -1) &&
-        (p_characteristics[characteristicIndex]->getProperties() & (GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_INDICATE | GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_NOTIFY)) &&
-        (gapConnectionHandle != connectionHandle)) {
+        (p_characteristics[characteristicIndex]->getProperties() & (GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_INDICATE | GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_NOTIFY))) {
         /* HVX update for the characteristic value */
         ble_gatts_hvx_params_t hvx_params;

@@ -235,20 +233,25 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute
         hvx_params.p_data = const_cast<uint8_t *>(buffer);
         hvx_params.p_len  = &len;

-        error_t error = (error_t) sd_ble_gatts_hvx(gapConnectionHandle, &hvx_params);
+        error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params);
+        if (error != ERROR_NONE) {
+            switch (error) {
+                case ERROR_BLE_NO_TX_BUFFERS: /*  Notifications consume application buffers. The return value can be used for resending notifications. */
+                case ERROR_BUSY:
+                    returnValue = BLE_STACK_BUSY;
+                    break;

-        /* ERROR_INVALID_STATE, ERROR_BUSY, ERROR_GATTS_SYS_ATTR_MISSING and ERROR_NO_TX_BUFFERS the ATT table has been updated. */
-        if ((error != ERROR_NONE) && (error != ERROR_INVALID_STATE) && (error != ERROR_BLE_NO_TX_BUFFERS) && (error != ERROR_BUSY) && (error != ERROR_BLEGATTS_SYS_ATTR_MISSING)) {
-            ASSERT_INT( ERROR_NONE,
-                        sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value),
-                        BLE_ERROR_PARAM_OUT_OF_RANGE );
-        }
+                case ERROR_INVALID_STATE:
+                case ERROR_BLEGATTS_SYS_ATTR_MISSING:
+                    returnValue = BLE_ERROR_INVALID_STATE;
+                    break;

-        /*  Notifications consume application buffers. The return value can
-            be used for resending notifications.
-        */
-        if (error != ERROR_NONE) {
-            returnValue = BLE_STACK_BUSY;
+                default :
+                    ASSERT_INT( ERROR_NONE,
+                                sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value),
+                                BLE_ERROR_PARAM_OUT_OF_RANGE );
+                    break;
+            }
         }
     } else {
         ASSERT_INT( ERROR_NONE,
fabiencomte commented 8 years ago

Your patch looks correct but notifications don't work on my try and i don't know why. I'm searching.

rgrover commented 8 years ago

thanks for the update. I'm sure this will be easy to track. Perhaps I should have mentioned that this is the patch against ble-nrf51822/develop (not against your PR).

rgrover commented 8 years ago

this is now a part of release v0.4.8