Jira #788: BLE Peripheral cannot discover Central attributes, Git #382. #410
Conversation
|
@bigdinotech @eriknyquist , please review the code change. @noelpaz @russmcinnis, this fix has been unit tested, please performance verification testing. Please note the additional of test sketches to exercise the fix. |
| License along with this library; if not, write to the Free Software | ||
| Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA | ||
| */ | ||
|
|
There was a problem hiding this comment.
Sketch code looks good, it's pretty readable. However I have no idea what this sketch is doing, there are not really any comments to explain it.
Can you please follow the style of our other example sketches (here is a good example: https://github.com/01org/corelibs-arduino101/blob/master/libraries/CurieIMU/examples/Accelerometer/Accelerometer.ino)
Specifically, 1) put the main part of the license at the bottom of the file, with a short 1-line reference at the top. Just copy the style of existing sketches, here. And 2), add a short comment at the top, explaining what the sketch does. Again, just copy the style of existing examples.
|
@eriknyquist , please check out the changes per requested. Thanks. |
There was a problem hiding this comment.
CentralDouble.ino looks great, excellent description, and the license is in the right place.
But it looks like the other files don't have these updates-- can you do the same thing for the rest of the example sketches here please? e.g. add a good description, like the one in CentralDouble.ino, and move the bulk of the license to the bottom of the file.
SidLeung
force-pushed
the
788
branch
2 times, most recently
from
b3eb626 to
b7d406e
Compare
|
@eriknyquist , please check out the additional changes to the sketches under the Central and Peripheral. They should be in the Arduino style with additional header block comments. |
|
What about discoveratperipheral.ino and profileatcentral.ino? |
|
@eriknyquist , you are correct... The sketches in the test directory are from various sources. And, according to @noelpaz , Arduino will not release them as their official CoreLibs release. @calvinatintel , @yashaswini-hanji , will not allow me to put anything into the release without proper licensing. Thus, any sketches without licensing, they will be removed. |
|
These two sketches do not match the style of the others; there is no description (so I do not know what they are supposed to do), and the full license is at the top, unlike the others. Please fix these things. |
|
profileatcentral.ino doesn't even have a license at all. Just make all the example sketches look the same, with a description and license. That's all. |
|
@eriknyquist , scanned the example sketches under the CurieBLE/example directory. They have proper licensing and format. For those sketches that do not have, and I can't locate, the licensing header, they are removed. Please review. |
|
@eriknyquist , fixed the last sketch licensing block issue. Please review. |
| Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA | ||
| */ | ||
| /* | ||
| * Sketch: BatteryMonitor_Central.ino |
There was a problem hiding this comment.
oops-- wrong name here (BatteryMonitor_Central.ino)
| Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA | ||
| */ | ||
| /* | ||
| * Sketch: BatteryMonitor_Central.ino |
There was a problem hiding this comment.
Wrong name again? BatteryMonitor_Central.ino?
| Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA | ||
| */ | ||
| /* | ||
| * Sketch: BatteryMonitor_Central.ino |
There was a problem hiding this comment.
Our old friend BatteryMonitor_Central.ino popping up again... 😄
|
|
||
|
|
||
| /* | ||
| Arduino BLE Peripheral Double read/write test example |
There was a problem hiding this comment.
Wrong example sketch name here (I think?)
|
|
||
|
|
||
| /* | ||
| Arduino BLE Peripheral LED example |
There was a problem hiding this comment.
The name here doesn't seem to match description of example sketch at the top....
|
|
||
|
|
||
| /* | ||
| Arduino BLE Peripheral LED example |
There was a problem hiding this comment.
Wrong-looking name again
|
@eriknyquist , licensing correction made. |
|
@SidLeung please see my comments about sketches wrongly named as "BatteryMonitor_Central.ino" |
|
@eriknyquist , corrected sketch reference name in comment block. |
| For more information: https://developer.bluetooth.org/gatt/services/Pages/ServicesHome.aspx | ||
| */ | ||
|
|
||
| * Sketch: led.ino |
|
@eriknyquist , sketch name corrected. Thanks. |
|
@yashaswini-hanji , please cherry-pick this PR to merge with main trunk. Thanks. |
1. Add test sketches discoveratperipheral.ino and profileatcentral.ino
2. Modify BLEDeviceManager.cpp to add central to profile buffer in peripheral
role.
|
@yashaswini-hanji , have cherry-picked this PR on top latest 01org/master. Would you please merge? Thanks. |
|
Changes merged to base branch. |
role.