Jira 793 Support Characteristic UUID that is not subset of Service, g… #451

SidLeung · 2017-02-22T18:51:59Z

…it 384

Root cause:

Nordic stack does not return long UUID for Characteristic
discovery. It is expecting Characteristic UUID is a
subset of the Service UUID (eg. a sequential increament of
the Service UUID).
In order for the Central mode (to behave like Apple) to
discover long Characteristic UUID, the process is now done
in two steps (similar to the Service discovery). First,
initiate a read of the Characteristic and then discover the
long UUID upon successful read.

Code Mods:

BLECallbacks.cpp:
- Added a call back to handle the read Characteristic
  completion event.
BLECallbacks.h:
- Prototyping.
BLEProfileManager.cpp:
- Added getServiceBySubHandle() to enable the call back to
  locate the Service of a discovered Characteristic.
BLEProfileManager.h:
- Prototyping.
BLEServiceImp.cpp:
- This is where the two steps Characteristic discovery is
  implemented. At discoverAttributes(), reading of
  Characteristic is first initiated. When the stack
  completes the operation, extract the UUID thereafter.
- To support this two steps process, additional functions,
  discoverNextCharacteristic and readCharacteristic, are
  added.
BLEServiceImp.h:
- Prototyping.
- Change the discover process.
- Add read operation after discover returned invalid UUID.

SidLeung · 2017-02-22T18:58:51Z

@bigdinotech @eriknyquist , please review the code change.

@russmcinnis @noelpaz , this change is to make our library to behave more like an Apple Central. The capability of discovering Characteristic UUID that is not sequential increment of the Service UUID. Please use an existing Peripheral sketch, modify the Characteristic UUID so that it is different from its Service UUID, and use a Central sketch to discover the Characteristic.

eriknyquist · 2017-02-22T20:14:12Z

libraries/CurieBLE/src/internal/BLECallbacks.cpp

+                                                         params,
+                                                         data,
+                                                         length);
+    pr_debug(LOG_MODULE_BLE, "%s-%d:ret-%d", __FUNCTION__, __LINE__, ret);


pr_debug? should this be here? not sure if these prints are disabled or not for release builds

This is only for debug. For the release, the log will never output to the terminal.

Understood. I tested it out also and saw no serial output.

eriknyquist · 2017-02-22T20:19:16Z

libraries/CurieBLE/src/internal/BLEProfileManager.cpp

+    }
+    BLEServiceNodePtr node = serviceHeader->next;
+
+    while (node != NULL)


Lot of extra (slightly confusing) code in lines 524 - 540. It took me a few reads to understand what was actually happening here. This while loop could be greatly simplified, and thus easier on the eyes.....

while (node != NULL) { serviceImp = node->value; end_handle = serviceImp->endHandle(); if (handle >= start_handle && handle <= end_handle) { return serviceImp; } node = node->next; } return NULL;

I like use break instead of return. Because we may extend the code in the feature, we don't need add the code in the loop if we use the break. Just add the code at the end of the function. But if you persist in your style, I can update the code.

no, it's OK. I understand your explanation, just looking for the reasoning :)

eriknyquist · 2017-02-22T22:07:59Z