[RFC] add a bunch of hackish (but working) observer role functions

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

[RFC] add a bunch of hackish (but working) observer role functions #155

Open floe opened 7 years ago

floe commented 7 years ago

Amazing library, saved me a lot of work when trying to get the BBC micro:bit up and running for a student. Thanks!

That being said, I'm currently looking into broadcast-based mesh applications, and I need observer support for these (not a full-scale central role, but at least receiving advertisements). I hacked together a couple of control functions and wanted your opinion on

if this an addition to the library you'd want to integrate (maybe difficult, I guess the nRF8001 doesn't support scanning at all)
if you have any suggestions how to properly handle the scan results - I'm currently thinking to just have a uint8_t* getScanResult() method that's supposed to be called after poll() and will return the most recent result, or NULL.

Best, Florian

sandeepmistry commented 7 years ago

Hey @floe,

Thanks for taking the time to submit this PR!

For compatibility reasons, I'd prefer to keep this library peripheral only.

However, moving forward it would be great to great a dual central and peripheral mode library for the nRF51/52 based on the CurieBLE API: https://github.com/01org/corelibs-arduino101/tree/master/libraries/CurieBLE

Thoughts? I don't have time to start this at the moment though ...

floe commented 7 years ago

Ah, didn't know about CurieBLE yet, I'll have a look. For the moment, I'll probably go with my hackish-but-works solution... no time for a proper solution, either ;-)

BTW, PR #120 is related in terms of API, isn't it?

floe commented 7 years ago

P.S. The alternative would be to create a dedicated fork of your library that only supports the nRF5x series. Would probably be less work than rebuilding everything to fit the CurieBLE API?

sandeepmistry commented 7 years ago

P.S. The alternative would be to create a dedicated fork of your library that only supports the nRF5x series. Would probably be less work than rebuilding everything to fit the CurieBLE API?

Yes, that's a good way to start. We can always split it out into a new repo later.

floe commented 6 years ago

Came back to this after a couple of months, PR is now in a state where it's working fine for my specific use case. My suggestion would be to review this under the assumption that we'll get rid of the nRF8001 support sooner or later.

sandeepmistry commented 6 years ago

My suggestion would be to review this under the assumption that we'll get rid of the nRF8001 support sooner or later.

No plans to remove it at this time :)

floe commented 6 years ago

Well, removing is probably too harsh, but how about splitting your library into a legacy branch that still supports the nRF8001, and a new branch that supports nRF51/52?

sandeepmistry commented 6 years ago

New branch sounds ok short term for development. However, to make it compatible with the Arduino IDE's library manager a new repo would need to be created.

cheeta1 commented 6 years ago

hi @floe can you provide the example of observer role in you master branch?

floe commented 6 years ago

@cheeta1 sure, no problem, see examples/observer/ in my master branch. It's a bit kludgy, but it demonstrates simultaneous sending and receiving of advertisements (i.e. broadcaster and observer in one).

floe commented 6 years ago

@sandeepmistry if I rename my fork to, let's say, BLEPeripheralObserver, would that work?

sandeepmistry commented 6 years ago

if I rename my fork to, let's say, BLEPeripheralObserver, would that work?

@floe works for me