2011-11-11 21:08:38

by Alon Bar-Lev

[permalink] [raw]
Subject: Out of tree plugins

Hello,

Reading the tread about Sixaxis [1], and actually want to use this device...

I could not figure out the end conclusion... as discussion was
terminated in the middle.

Bluez is already implemented in modular way.

However, there is no sense in having the ability to be modular and not
expose it to out of tree modules.

And if there is out of tree modular support, features like new devices
may be added without upstream involvement.

I tried to compile the echo, wiiremote, sixaxis and most other, and
found that plugins may be compiled using the simple command:

gcc \
-shared -fPIC -fpic -o a.out $f \
$(pkg-config --cflags glib-2.0) $(pkg-config --cflags dbus-1)
$(pkg-config --cflags bluez) \
'-DVERSION="1"' -I../gdbus -Iextra_include

While /usr/include/bluetooth has whatever bluez installs.
And while in extra_include the following files:
---
adapter.h
btio.h
device.h
hcid.h
log.h
manager.h
oob.h
plugin.h
sdpd.h
sdp-xml.h
storage.h
textfile.h
---

Note: Not sure about the requirement of gdbus.h if dbus-glib is available.

Of course src/bluetooth.ver needs to be updated to export the above
headers' symbols to make it work.

After doing that, most of in-tree plugins may be split out of
bluetoothd, and plugins as wiiremote, sixaxis may
be maintained in its own repository and release cycle.

Thoughts?
Alon Bar-Lev.

[1] http://thread.gmane.org/gmane.linux.bluez.kernel/15308


2011-11-16 14:48:22

by Antonio Ospite

[permalink] [raw]
Subject: Re: Out of tree plugins

On Fri, 11 Nov 2011 21:42:21 -0200
Lucas De Marchi <[email protected]> wrote:

> On Fri, Nov 11, 2011 at 8:25 PM, Alon Bar-Lev <[email protected]> wrote:
> > Hello,
> >
> > On Fri, Nov 11, 2011 at 11:59 PM, Lucas De Marchi
> > <[email protected]> wrote:
> >> Hi Alon
> >>
> >> On Fri, Nov 11, 2011 at 7:08 PM, Alon Bar-Lev <[email protected]> wrote:
> >>> Hello,
> >>>
> >>> Reading the tread about Sixaxis [1], and actually want to use this device...
> >>>
> >>> I could not figure out the end conclusion... as discussion was
> >>> terminated in the middle.
> >>>

Alon, you can look at here to see what direction the discussion was
taking:

http://thread.gmane.org/gmane.linux.bluez.kernel/16540

a proper review of this proposal by BlueZ developer is still needed.

[...]
> The goal here is not to be able to build out of tree, but just as
> module or builtin.
>

I can see Luca's argument here, AFAICS in BlueZ modularity of plugins is
meant for object size reasons and for linking dependencies (think to
udev for the sixaxis/plystation-peripheral plugin); plugins are
still bound to the symbols provided by a matching bluetoothd, BlueZ
devs do not want to keep a “super stable” public API for out-of-tree
plugins, having the possibility of out-of-tree plugins is considered
just a side effect of modularity here with no promises at all.

Sorry if I skipped other parts of the discussion, not enough time.

Regards,
Antonio

--
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?


Attachments:
(No filename) (1.65 kB)
(No filename) (198.00 B)
Download all attachments

2011-11-12 11:03:48

by Alon Bar-Lev

[permalink] [raw]
Subject: Re: Out of tree plugins

On Sat, Nov 12, 2011 at 2:29 AM, Alon Bar-Lev <[email protected]> wrote:
> On Sat, Nov 12, 2011 at 2:17 AM, Lucas De Marchi
> <[email protected]> wrote:
>> ( e.g. in the mail thread you mentioned, dbus_connection_unref() and
>> dbus_bus_get() are not right. If you need those you should link
>> against d-bus instead when building as an external module )
>
> This is needed in order to get the address of the bluetooth controller.
> I am no expert in this library so maybe there is another way to do so.
>
> The code currently uses:
> ---
> struct btd_device *adapter_get_device(DBusConnection *conn,
>                                struct btd_adapter *adapter, const
> char *address);
> ---
>
> Can this be somehow be replaced with:
> ---
> struct btd_device *adapter_find_device(struct btd_adapter *adapter,
> const char *dest);
> ---

I could not find how to do this, as the device should be created, and
all creation funcitons needs dbus connection.
How do you suggest to replace the adapter_get_device() to something
else that does not need dbus?

Thanks,
Alon.

2011-11-12 00:29:05

by Alon Bar-Lev

[permalink] [raw]
Subject: Re: Out of tree plugins

On Sat, Nov 12, 2011 at 2:17 AM, Lucas De Marchi
<[email protected]> wrote:
> ( e.g. in the mail thread you mentioned, dbus_connection_unref() and
> dbus_bus_get() are not right. If you need those you should link
> against d-bus instead when building as an external module )

This is needed in order to get the address of the bluetooth controller.
I am no expert in this library so maybe there is another way to do so.

The code currently uses:
---
struct btd_device *adapter_get_device(DBusConnection *conn,
struct btd_adapter *adapter, const
char *address);
---

Can this be somehow be replaced with:
---
struct btd_device *adapter_find_device(struct btd_adapter *adapter,
const char *dest);
---

Alon

2011-11-12 00:17:47

by Lucas De Marchi

[permalink] [raw]
Subject: Re: Out of tree plugins

On Fri, Nov 11, 2011 at 9:49 PM, Alon Bar-Lev <[email protected]> wrote:
> On Sat, Nov 12, 2011 at 1:42 AM, Lucas De Marchi
> <[email protected]> wrote:
>>> If you do not expose the plugin.h (install it as part of bluez package
>>> installation),
>>> it means that all plugins must be in tree.
>>> Or you require people to checkout bluez sources and compile against
>>> it, this is none standard approach.
>>> Same for other header files which may be used by out of tree plugins.
>>
>> The goal here is not to be able to build out of tree, but just as
>> module or builtin.
>
> <snip>
>
>> Then this is a bug and should be treated as such. In ConnMan/oFono we
>> prefix the functions and then we use a glob in the linker script. I
>> think we don't have that in bluez because there aren't that many
>> external plugins.
>
> Well, so there will be no out of tree plugins.
> So what is the process to get complex module such as sixaxis into tree?

Resolve the issue with a few symbols that should be exported.

Maybe we would need to start using prefixes like ofono/connman do.
Just patching the version script IMO is not a good thing

( e.g. in the mail thread you mentioned, dbus_connection_unref() and
dbus_bus_get() are not right. If you need those you should link
against d-bus instead when building as an external module )


>
> Adding the udev dependency to bluetoothd is ugly.
> Bloating the bluetoothd with a lot of more code is ugly.
> So at the very least the symbol issues should be resolved, so the plugin can
> be built into its own .so.

Correct. Patches for that would be appreciated.



Regards,
Lucas De Marchi

2011-11-11 23:49:14

by Alon Bar-Lev

[permalink] [raw]
Subject: Re: Out of tree plugins

On Sat, Nov 12, 2011 at 1:42 AM, Lucas De Marchi
<[email protected]> wrote:
>> If you do not expose the plugin.h (install it as part of bluez package
>> installation),
>> it means that all plugins must be in tree.
>> Or you require people to checkout bluez sources and compile against
>> it, this is none standard approach.
>> Same for other header files which may be used by out of tree plugins.
>
> The goal here is not to be able to build out of tree, but just as
> module or builtin.

<snip>

> Then this is a bug and should be treated as such. In ConnMan/oFono we
> prefix the functions and then we use a glob in the linker script. I
> think we don't have that in bluez because there aren't that many
> external plugins.

Well, so there will be no out of tree plugins.
So what is the process to get complex module such as sixaxis into tree?

Adding the udev dependency to bluetoothd is ugly.
Bloating the bluetoothd with a lot of more code is ugly.
So at the very least the symbol issues should be resolved, so the plugin can
be built into its own .so.

Alon.

2011-11-11 23:42:21

by Lucas De Marchi

[permalink] [raw]
Subject: Re: Out of tree plugins

On Fri, Nov 11, 2011 at 8:25 PM, Alon Bar-Lev <[email protected]> wrote:
> Hello,
>
> On Fri, Nov 11, 2011 at 11:59 PM, Lucas De Marchi
> <[email protected]> wrote:
>> Hi Alon
>>
>> On Fri, Nov 11, 2011 at 7:08 PM, Alon Bar-Lev <[email protected]> wrote:
>>> Hello,
>>>
>>> Reading the tread about Sixaxis [1], and actually want to use this device...
>>>
>>> I could not figure out the end conclusion... as discussion was
>>> terminated in the middle.
>>>
>>> Bluez is already implemented in modular way.
>>>
>>> However, there is no sense in having the ability to be modular and not
>>> expose it to out of tree modules.
>>
>> It does make sense. It means you can have it builtin or as a .so that
>> will be loaded at runtime.
>>
>
> If you do not expose the plugin.h (install it as part of bluez package
> installation),
> it means that all plugins must be in tree.
> Or you require people to checkout bluez sources and compile against
> it, this is none standard approach.
> Same for other header files which may be used by out of tree plugins.

The goal here is not to be able to build out of tree, but just as
module or builtin.

>
>>> And if there is out of tree modular support, features like new devices
>>> may be added without upstream involvement.
>>
>> Why do you want that?
>>
>
> This provides the flexibility to maintain new features within its own
> release cycle.
> If I have a special device, why should all people be effected by the plugin?

If it will affect people, it should have a config option to enable/disable it.


> Why people should beg to revbump and change the way bluez is built on
> distributions?

Because it depends in the exact version of core that is running.

> For example, having bluez-wiiremote, bluez-sixaxis packages will
> effect only people which actually use this hardware, and bugs/features
> may be pushed much faster to users.
> Isn't this a win-win situation?
> Do you prefer to handle all devices and functionality in tree?
>
>>> I tried to compile the echo, wiiremote, sixaxis and most other, and
>>> found that plugins may be compiled using the simple command:
>>>
> <snip>
>
>>
>> All the symbols needed by plugins should be covered by bluetooth.ver
>
> Not exactly.
>
> Example:
> ---
> $ gcc -pthread -shared -fPIC -fpic -o a.out sixaxis.c $(pkg-config
> --cflags glib-2.0) $(pkg-config --cflags dbus-1) $(pkg-config --cflags
> bluez) $(pkg-config --libs glib-2.0) $(pkg-config --libs dbus-1)
> '-DVERSION="1"' -ludev -ldl -I../gdbus -II -Wl,--no-undefined
> /usr/sbin/bluetoothd
> /tmp/ccSJ9537.o: In function `create_sixaxis_association':
> sixaxis.c:(.text+0x48): undefined reference to `str2ba'
> sixaxis.c:(.text+0x5b): undefined reference to `adapter_get_address'
> sixaxis.c:(.text+0x6e): undefined reference to `ba2str'
> sixaxis.c:(.text+0x85): undefined reference to `write_device_name'
> sixaxis.c:(.text+0x94): undefined reference to `record_from_string'
> sixaxis.c:(.text+0xb2): undefined reference to `store_record'
> sixaxis.c:(.text+0xbe): undefined reference to `sdp_record_free'
> sixaxis.c:(.text+0xf6): undefined reference to `store_device_id'
> sixaxis.c:(.text+0x118): undefined reference to `write_trust'
> sixaxis.c:(.text+0x184): undefined reference to `adapter_get_device'
> sixaxis.c:(.text+0x1d5): undefined reference to `device_set_temporary'
> sixaxis.c:(.text+0x1e8): undefined reference to `device_set_name'
> /tmp/ccSJ9537.o: In function `sixpair':
> sixaxis.c:(.text+0x7a0): undefined reference to `adapter_get_address'
> sixaxis.c:(.text+0x7b3): undefined reference to `ba2str'
> /tmp/ccSJ9537.o: In function `handle_device_plug':
> sixaxis.c:(.text+0xf4e): undefined reference to `manager_get_default_adapter'
> collect2: ld returned 1 exit status

Then this is a bug and should be treated as such. In ConnMan/oFono we
prefix the functions and then we use a glob in the linker script. I
think we don't have that in bluez because there aren't that many
external plugins.


> ---
>
>>> After doing that, most of in-tree plugins may be split out of
>>> bluetoothd, and plugins as wiiremote, sixaxis may
>>> be maintained in its own repository and release cycle.

No, they would need to match the exact version of bluez. It's just
like the external kernel modules that must be rebuilt/fixed for each
kernel.

>>
>> The internal interface is subject to change. And it does change (make
>> a `git log' in these header files to see).
>
> That's is understandable.
> I guess that in time it will get more stabilized.
>
>>>
>>> Thoughts?
>>
>>
>> Yes, it's relatively easy to compile a plugin out of tree, but you
>> assume the burden to maintain and you are on your own.
>>
>
> In order to do so:
>
> 1. internal headers should be installed.
> For example to /usr/include/bluetooth/plugin, in order to avoid
> conflict with external API.

I don't see any benefits in doing it.

>
> 2. More symbols should be exported from bluetoothd, to match the headers at [1].

This is true. If such a plugin is allowed to be built externally, we
should export the necessary symbols.


Regards,
Lucas De Marchi

2011-11-11 22:25:57

by Alon Bar-Lev

[permalink] [raw]
Subject: Re: Out of tree plugins

Hello,

On Fri, Nov 11, 2011 at 11:59 PM, Lucas De Marchi
<[email protected]> wrote:
> Hi Alon
>
> On Fri, Nov 11, 2011 at 7:08 PM, Alon Bar-Lev <[email protected]> wrote:
>> Hello,
>>
>> Reading the tread about Sixaxis [1], and actually want to use this device...
>>
>> I could not figure out the end conclusion... as discussion was
>> terminated in the middle.
>>
>> Bluez is already implemented in modular way.
>>
>> However, there is no sense in having the ability to be modular and not
>> expose it to out of tree modules.
>
> It does make sense. It means you can have it builtin or as a .so that
> will be loaded at runtime.
>

If you do not expose the plugin.h (install it as part of bluez package
installation),
it means that all plugins must be in tree.
Or you require people to checkout bluez sources and compile against
it, this is none standard approach.
Same for other header files which may be used by out of tree plugins.

>> And if there is out of tree modular support, features like new devices
>> may be added without upstream involvement.
>
> Why do you want that?
>

This provides the flexibility to maintain new features within its own
release cycle.
If I have a special device, why should all people be effected by the plugin?
Why people should beg to revbump and change the way bluez is built on
distributions?
For example, having bluez-wiiremote, bluez-sixaxis packages will
effect only people which actually use this hardware, and bugs/features
may be pushed much faster to users.
Isn't this a win-win situation?
Do you prefer to handle all devices and functionality in tree?

>> I tried to compile the echo, wiiremote, sixaxis and most other, and
>> found that plugins may be compiled using the simple command:
>>
<snip>

>
> All the symbols needed by plugins should be covered by bluetooth.ver

Not exactly.

Example:
---
$ gcc -pthread -shared -fPIC -fpic -o a.out sixaxis.c $(pkg-config
--cflags glib-2.0) $(pkg-config --cflags dbus-1) $(pkg-config --cflags
bluez) $(pkg-config --libs glib-2.0) $(pkg-config --libs dbus-1)
'-DVERSION="1"' -ludev -ldl -I../gdbus -II -Wl,--no-undefined
/usr/sbin/bluetoothd
/tmp/ccSJ9537.o: In function `create_sixaxis_association':
sixaxis.c:(.text+0x48): undefined reference to `str2ba'
sixaxis.c:(.text+0x5b): undefined reference to `adapter_get_address'
sixaxis.c:(.text+0x6e): undefined reference to `ba2str'
sixaxis.c:(.text+0x85): undefined reference to `write_device_name'
sixaxis.c:(.text+0x94): undefined reference to `record_from_string'
sixaxis.c:(.text+0xb2): undefined reference to `store_record'
sixaxis.c:(.text+0xbe): undefined reference to `sdp_record_free'
sixaxis.c:(.text+0xf6): undefined reference to `store_device_id'
sixaxis.c:(.text+0x118): undefined reference to `write_trust'
sixaxis.c:(.text+0x184): undefined reference to `adapter_get_device'
sixaxis.c:(.text+0x1d5): undefined reference to `device_set_temporary'
sixaxis.c:(.text+0x1e8): undefined reference to `device_set_name'
/tmp/ccSJ9537.o: In function `sixpair':
sixaxis.c:(.text+0x7a0): undefined reference to `adapter_get_address'
sixaxis.c:(.text+0x7b3): undefined reference to `ba2str'
/tmp/ccSJ9537.o: In function `handle_device_plug':
sixaxis.c:(.text+0xf4e): undefined reference to `manager_get_default_adapter'
collect2: ld returned 1 exit status
---

>> After doing that, most of in-tree plugins may be split out of
>> bluetoothd, and plugins as wiiremote, sixaxis may
>> be maintained in its own repository and release cycle.
>
> The internal interface is subject to change. And it does change (make
> a `git log' in these header files to see).

That's is understandable.
I guess that in time it will get more stabilized.

>>
>> Thoughts?
>
>
> Yes, it's relatively easy to compile a plugin out of tree, but you
> assume the burden to maintain and you are on your own.
>

In order to do so:

1. internal headers should be installed.
For example to /usr/include/bluetooth/plugin, in order to avoid
conflict with external API.

2. More symbols should be exported from bluetoothd, to match the headers at [1].

This will allow an external package to be built and installed.


Alon

2011-11-11 21:59:20

by Lucas De Marchi

[permalink] [raw]
Subject: Re: Out of tree plugins

Hi Alon

On Fri, Nov 11, 2011 at 7:08 PM, Alon Bar-Lev <[email protected]> wrote:
> Hello,
>
> Reading the tread about Sixaxis [1], and actually want to use this device...
>
> I could not figure out the end conclusion... as discussion was
> terminated in the middle.
>
> Bluez is already implemented in modular way.
>
> However, there is no sense in having the ability to be modular and not
> expose it to out of tree modules.

It does make sense. It means you can have it builtin or as a .so that
will be loaded at runtime.


>
> And if there is out of tree modular support, features like new devices
> may be added without upstream involvement.

Why do you want that?

>
> I tried to compile the echo, wiiremote, sixaxis and most other, and
> found that plugins may be compiled using the simple command:
>
> gcc \
> ? ?-shared -fPIC -fpic -o a.out $f \
> ? $(pkg-config --cflags glib-2.0) $(pkg-config --cflags dbus-1)
> $(pkg-config --cflags bluez) \
> ? '-DVERSION="1"' -I../gdbus -Iextra_include
>
> While /usr/include/bluetooth has whatever bluez installs.
> And while in extra_include the following files:
> ---
> adapter.h
> btio.h
> device.h
> hcid.h
> log.h
> manager.h
> oob.h
> plugin.h
> sdpd.h
> sdp-xml.h
> storage.h
> textfile.h
> ---
>
> Note: Not sure about the requirement of gdbus.h if dbus-glib is available.
>
> Of course src/bluetooth.ver needs to be updated to export the above
> headers' symbols to make it work.

All the symbols needed by plugins should be covered by bluetooth.ver

>
> After doing that, most of in-tree plugins may be split out of
> bluetoothd, and plugins as wiiremote, sixaxis may
> be maintained in its own repository and release cycle.

The internal interface is subject to change. And it does change (make
a `git log' in these header files to see).

>
> Thoughts?


Yes, it's relatively easy to compile a plugin out of tree, but you
assume the burden to maintain and you are on your own.



Regards,
Lucas De Marchi