Opening API

[philips77]

Hi,
As you may have seen, most of the methods in the BleManager are package protected and final. My intention was to allow the final user to define the their manager's API without bringing all other methods with it. For example, the MySuperCoolManager extends BleManager would have a method enableFancyLED(), bond(), etc. and internal methods, like writeNotification would not be visible from outside. There are 2 methods exposed: connect(..) and disconnect(..), but not more.
The question is: is it a good idea, or does it make more troubles than good?
There are 3 solutions:

1. It's good as it is. Package protected and final.
2. All should be made public or at least not final.
3. I may also expose some methods, like reading RSSI or bonding only.

As I'm thinking now, making them non-final is the best idea. Some methods could be made public by overriding them without actually changing functionality.

CC: @eliotstock, @maragues-kolibree, @sztomek, @balazsnemeth, @balazsnemeth, ...

[eliotstock]

Thanks for asking! I was just about to ask about this myself. My vote would be for both 2 and 3.

I found myself wanting access to these things in my BleManager subclass:

• The readRssi() method
• The createBond() method
• The Context passed in to the constructor, which I need in order to go and get some singletons in my app.

[ghost]

Our implementation follows 1)

On the one hand, it helps in keeping BT implementation details contained in 1 class, and we can expose operations with semantic meaning in our domain. On the other hand, it does end up becoming a God class. Our BleManager implementation is ~550 lines with very little tests (see #45)

Implementing 2) would helping in having smaller, domain-specific Ble classes if we were able to keep a single queue request. As I'm thinking it right now, we'd have a singleton BleManager instance and per-feature wrappers over it. This'd make smaller classes with easier testing.

SensorBleManager @Inject constructor(private val bleManager) {
    fun enableVibration(){
        bleManager.writeCharacteristic(...).enqueue()
    }
}

Which methods are you considering to make non-final? What would be the goal? Is it to facilitate testing or are there other reasons?

[philips77]

Let's leave the testability to #45.

I was thinking about definaling methods that return Requests, e.g. writeCharacteristic(...), etc.
This may be useful if the BleManager instance is to be used as a generic BLE controller, not as a device API. Currently, to expose such method, you need to add some prefix/sufix to the method name, which isn't very big deal, but makes it uglier. I found it useful in 2 cases, as far as I remember, where I wanted to call those methods from outside of manager. In that case I had a TestImpl, which just had the logic of which ble operations should be done. I didn't want to include it in BleManager for some reason.

Regarding having smaller domain-specific classes, I was rather thinking about allowing to add "modules" to the manager, responsible for some part of the logic. For example, DFU module, Battery module, etc.
Currently, what we do in nRF Toolbox, is just extending the BleManager with for example Battery logic, and then extending this in other profiles. I'm not yet sure the module approach is feasible, would it increase or decrease testability, but at least would split the class logic.

So far I'm (almost) convinced to make request operations non-final. Then exposing readRssi or createBond is straightforward.