Hi,
On Mon, Sep 10, 2018 at 9:56 AM Łukasz Rymanowski <
lukasz.rymanow...@codecoup.pl> wrote:
> Hi
> On Fri, 7 Sep 2018 at 21:12, Andrzej Kaczmarek <
> andrzej.kaczma...@codecoup.pl> wrote:
>
> > Hi Chris,
> >
> > On Fri, Sep 7, 2018 at 8:00 PM Christopher Collins <ch...@runtime.io>
> > wrote:
> >
> > > Hello all,
> > >
> > > TL;DR: Proposal for registration of GAP event listeners in the NimBLE
> > > host. Currently, GAP event callbacks are specified by the code which
> > > creates a connection. This proposal allows code to listen for GAP
> > > events without creating a connection.
> > >
> > > The NimBLE host allows the application to associate callbacks with GAP
> > > events. Callbacks are configured *per connection*. When the
> > > application initiates a connection, it specifies a single callback to
> > > be executed for all GAP events involving the connection. Similarly,
> > > when the application starts advertising, it specifies the callback to
> > > use with the connection that may result.
> > >
> > > This design provides some degree of modularity. If a package
> > > independently creates a connection, it can handle all the relevant GAP
> > > events without involving other packages in the system. However, there
> > > is a type of modularity that this does not provide: *per event*
> > > callbacks. If a package doesn't want to initiate any GAP events, but
> > > it wants to be notified every time a connection is established (for
> > > example), there is no way to achieve this. The problem here is that
> > > there is only a single callback associated with each connection. What
> > > we really need is a list of callbacks that all get called whenever a
> > > GAP event occurs.
> > >
> > > My proposal is to add the following to the NimBLE host API:
> > >
> > > struct ble_gap_listener {
> > > /*** Public. */
> > > ble_gap_event_fn *fn;
> > > void *arg;
> > >
> > > /*** Internal. */
> > > STAILQ_ENTRY(ble_gap_listener) next;
> > > };
> > >
> > > /**
> > > * @brief Registers a BLE GAP listener.
> > > *
> > > * When a GAP event occurs, callbacks are executed in the following
> > > * order:
> > > * 1. Connection-specific GAP event callback.
> > > * 2. Each registered listener, in the order they were registered.
> > > *
> > > * @param listener The listener to register.
> > > */
> > > void ble_gap_register(struct ble_gap_listener *listener);
> > >
> > > /**
> > > * @brief Unregisters a BLE GAP listener.
> > > *
> > > * @param listener The listener to unregister.
> > > *
> > > * @return 0 on success;
> > > * BLE_HS_ENOENT if the listener is
> > > * not registered.
> > > */
> > > int ble_gap_unregister(struct ble_gap_listener *listener);
> > >
> > > Initially, I thought this functionality could be achieved with a new
> > > package implemented on top of the host (call it `blemux`).
> > > Unfortunately, I think there are some issues that make such an approach
> > > unwieldy for the user. I imagined such a package would be used as
> > > follows:
> > >
> > > 1. The `blemux` package exposes a GAP callback (call it
> > > `blemux_gap_event`).
> > > 2. Elsewhere in the sytem, for each GAP function call, the caller
> > > specifies `blemux_gap_event` as the callback.
> > > 3. Each package that is interested in GAP events registers one or
> > > more listeners via `blemux_register()`.
> > > 4. When a GAP event occurs, `blemux_gap_event` is executed. This
> > > callback executes every registered listener.
> > >
> > > The problem is that packages have no guarantee that the app is using
> > > blemux. A package can depend on `blemux` and can register listeners,
> > > but it really only works if every other BLE-aware package in the system
> > > also uses `blemux`. If any package doesn't use `blemux`, then the GAP
> > > procedures that it initiates won't be reported to the `blemux`
> > > listeners. A secondary problem is that this just feels like a clumsy
> > > API. It is confusing and error prone to need to specify
> > > `blemux_gap_event` as the GAP event callback.
> > >
> > > So, I think this functionality should be implemented directly in the
> > > host.
> > >
> >
> >
> Like the idea. We could use it for mesh as well - for now we have dedicated
> API for Mesh to register for GAP events.
> With this change we could get rid of it.
>
Indeed. Also we can simplify existing services.
> > Sounds exactly like something I wrote some time ago, so I can only agree
> > with above :-)
> > Typical scenario here (and what I was doing) is that external package,
> like
> > service, wants to be notified about connection or characteristic
> > subscription state. Currently we do this by exposing extra API from
> package
> > which app needs to call from GAP callbacks in order to everything work
> > properly. It works, but it's really clunky. Hooks provided by host are
> imo
> > good way to handle this.
> >
> > I did not finish my service so forgot to send a PR with NimBLE changes,
> but
> > here they are for reference - I can create PR if they look ok.
> >
> >
> https://github.com/andrzej-kaczmarek/apache-mynewt-nimble/commit/6cc902821e6d65fbd5f72fa89e7219915afca38d
>
>
> Looks it does what it should, however ble_gap_register() sounds like a
> better name for API to me imho.
>
"Hooks" were just temporary name. I renamed everything to "event listeners"
and pushed this as a PR: https://github.com/apache/mynewt-nimble/pull/185
> >
> > The API is pretty much the same as proposed. The difference is that
> > listener struct is opaque and fields are initialized when listener is
> > registered.
> > Notified events are the ones which are notifications, i.e. do not require
> > interaction from app (so e.g. pairing event does not call listeners since
> > it does not make sense to do so).
> >
> > > Thoughts?
>
> >
> > > Thanks,
> > > Chris
> > >
> >
> >
> Best
> Łukasz
>
Best,
Andrzej
>
>
> > Best,
> > Andrzej
> >
>
