search

sandeepmistry / arduino-BLEPeripheral

An Arduino library for creating custom BLE peripherals with Nordic Semiconductor's nRF8001 or nR51822.
MIT License
462 stars 179 forks source link

clarify BLEPeripheral constructor pin required, but hardcoded/ignored… #130

Closed jacobrosenthal closed 7 years ago

jacobrosenthal commented 7 years ago

… for some boards

Happy to argue verbiage

sandeepmistry commented 7 years ago

Maybe we should just remove the pin stuff, and add a comment about being able to customize them for nRF8001.

I added a default args in https://github.com/sandeepmistry/arduino-BLEPeripheral/commit/6e758dc7de3e95b62527f37612c8374a9cb4a3b2 to match CurieBLE.

jacobrosenthal commented 7 years ago

With those new (old) defaults already in, and nrf8001 style boards going out of favor and nrf51/52 boards the current standard I see just removing the comments from examples entirely and adding a readme footnote?

//   Adafruit Bluefruit LE   10, 2, 9
//   Blend                    9, 8, UNUSED
//   Blend Micro              6, 7, 4
//   RBL BLE Shield           9, 8, UNUSED

The default constructor is currently filled by these pins, and thus we've lost the 9, 8, UNUSED configuration

#if defined(NRF51) || defined(NRF52) || defined(__RFduino__)
  #define BLE_DEFAULT_REQ   -1
  #define BLE_DEFAULT_RDY   -1
  #define BLE_DEFAULT_RST   -1
#elif defined(BLEND_MICRO)
  #define BLE_DEFAULT_REQ   6
  #define BLE_DEFAULT_RDY   7
  #define BLE_DEFAULT_RST   4
#else
  #define BLE_DEFAULT_REQ   10
  #define BLE_DEFAULT_RDY   2
  #define BLE_DEFAULT_RST   9
#endif

BLEND_MICRO isnt defined anywhere in arduino-nRF5, Looks like its coming from Redbear board definitions, https://github.com/RedBearLab/Blend/tree/master/Arduino/hardware/blend/avr/variants

#define BLEND

and

#define BLEND_MICRO
#define BLEND_MICRO_16MHZ

And implemented in their nrf8001 library as https://github.com/RedBearLab/nRF8001/blob/fd3d32212fae78ef717bceea9c5c87a493d9b29d/src/RBL_nRF8001.h#L31-L37

#if defined(BLEND_MICRO)
#define DEFAULT_REQN    6
#define DEFAULT_RDYN    7
#else
#define DEFAULT_REQN    9
#define DEFAULT_RDYN    8
#endif

NOTE I dont see a #define there for RBL BLE Shield nor can I find it elsewhere, I think were just going to have to lose it.

I also can cant find any information on their reset pin, doesnt seem to be hardcoded, they have an accessor function that takes a reset pin as argument, but none of their examples call it: https://github.com/RedBearLab/nRF8001/blob/fd3d32212fae78ef717bceea9c5c87a493d9b29d/src/RBL_nRF8001.cpp#L244

Im just going to cargo cult the BLEND_MICRO reset pin in our definitions for BLEND like:

#elif defined(BLEND)
  #define BLE_DEFAULT_REQ   9
  #define BLE_DEFAULT_RDY   8
  #define BLE_DEFAULT_RST   4

Updated pr soon

jacobrosenthal commented 7 years ago

Needs testing and review still

sandeepmistry commented 7 years ago

I think this is good to merge once we sort out https://github.com/sandeepmistry/arduino-BLEPeripheral/pull/130#discussion_r106556493