intel-iot-devkit / tinyb

TinyB exposes the BLE GATT API for C++, Java and other languages, using BlueZ over DBus.
MIT License
255 stars 114 forks source link

Generated dbus code doesn't match org.bluez.xml #101

Open impala454 opened 7 years ago

impala454 commented 7 years ago

I'm attempting some modifications to add newer services like LEAdvertisement1. I noticed when modifying the existing org.bluez.xml file, then generating the code via gdbus-codegen --interface-prefix org.bluez --generate-c-code generated-code --c-generate-object-manager org.bluez.xml that the code generated was slightly different in many places. The most common occurence was gchar being generated instead of the GBytes in the tinyb generated-code.h and generated-code.c. This obviously breaks tinyb as its code is built on top of this generated code. I tried several variations on the gdbus-codgen command to no avail.

Any thoughts or could someone share what gdbus-codegen command they used? My version is 2.48.2 just like the one in the repo.

petreeftime commented 7 years ago

Yes. I used a modified version of gdbus which I didn't get to upstream, as it needed some extra changes to not break compatibility of other programs. At the time I found a bug report in which the gdbus maintainers rejected a complaint that gchar * is not a good format, so I put in patch on the backlog and never got around to doing it. I don't have the patch anymore, but it was not difficult to hack it. If you need any help let me know and I'll look into it.

impala454 commented 7 years ago

I'm actually working on it a little in my spare time right now. Having a bit of issue getting GDBus.Error:org.bluez.Error.InvalidArguments: Invalid Length when attempting to call gatt_characteristic1_call_write_value_sync() from BluetoothGattCharacteristic::write_value. Not sure why it's being such a pain as it seemed to be nearly a drop in replacement of GBytes to gchar. I'll clean it up on the branch I'm working on and commit so you can see it.

impala454 commented 7 years ago

See my fork here

petreeftime commented 7 years ago

gchar* doesn't work because BlueZ transfers binary data over DBus, which might contain zeroes. Since gchar * doesn't actually contain length information, the transfer function stops when it finds the first zero, and you will end up with this error: GDBus.Error:org.bluez.Error.InvalidArguments: Invalid Length.

Instead, you have to use GBytes and in generated function itself, you'll need blocks that marshall the GBytes from/into DBus byte arrays, and not strings.

I suggest you just manually merge the generated-code.c you get with the old one, keeping the use of GBytes and just adding the org.bluez.LEAdvertisement1 interface.

impala454 commented 7 years ago

Bummer! I did find that I could add the argument --annotate "org.bluez.GattCharacteristic1.WriteValue()[value]" org.gtk.GDBus.C.ForceGVariant true to the code generation and it would actually change the troublesome gchar* value into a GVariant*. I'm trying to see if I can leverage that to somehow keep the generated code stable.

impala454 commented 7 years ago

I did end up having success with this! Added annotations to the dbus profile xml file <annotation name="org.gtk.GDBus.C.ForceGVariant" value="true"/> to force GVariants and replaced the code. Now the generated code works! I'll still need to make the changes to GattDescriptor but glad to have solved it. Thanks for your help!

vkolotov commented 7 years ago

Good stuff @impala454, I never got to this, would that mean that it is possible to extend tinyb for this? https://github.com/intel-iot-devkit/tinyb/issues/80

vkolotov commented 7 years ago

BTW, do you have this as well? https://github.com/intel-iot-devkit/tinyb/issues/69

Something tells me that there is something wrong with org.bluez.xml

impala454 commented 7 years ago

Hey @vkolotov yep on the Adapter.SetDiscoveryFilter. The reason I wanted the code generator to work is because I wanted to update the interface xml to the latest BlueZ. I've also tinkered with OpenHAB so would love to help them out. I will remove the generated code and have CMake generate it automatically when it's done. Hopefully I'll have a PR in by the end of the week.

As far as the java stuff, not really sure. I'll reply on #69 with what little I know about it.

petreeftime commented 7 years ago

@impala454 Awesome find. Should make it a lot easier to update TinyB, and it should be possible to remove the generated-code.c altogether if this works reliably.

vkolotov commented 7 years ago

@impala454 @petreeftime good stuff guys, the plugin for openhab will be ready soon, SetDiscoveryFilter would help a lot.

impala454 commented 7 years ago

@petreeftime is it worth doing a PR on the completed C++ side, or would you rather wait until the corresponding java code is written by someone else? For what it's worth, I'll likely be helping out a lot with the C++ side in the near future. What are your thoughts on splitting this library out into a C++ only library, and someone else building the Java side which depends on it? That may get better traction to bring in more help. I notice someone else just submitted a PR for functionality only on the java side.

petreeftime commented 7 years ago

I am fine with having C++ only functionality.

impala454 commented 7 years ago

Sorry I've been a little slow on this but likely over the next couple of months I'll be working on this. I will probably pear it back to C++ only. If it needs to be a separate fork that's ok just let me know.

vkolotov commented 6 years ago

Hey @impala454 I'm happy to do java part if you are still keen on doing dbus-codegen and some c++ parts of it. Pls guide me what to do as I'm not a big expert in c++. Particularly, what command did you use to run dbus-codegen?

impala454 commented 6 years ago

I'm really sorry I've just been swamped at my day job and the project I thought would be using this got put off. I may have time at some point but for now, I believe my fork as is incorporates the automatic dbus code generation.

sgothel commented 4 years ago

Hi @impala454 and @vkolotov - thank you for your discussion and work. I have merged your work here https://github.com/sgothel/tinyb

Some of the unref/free I have added, but I am not a glib expert, so I might have missed some parts? Just in case you still work with this library, maybe you like to review and merge back.

I may also review lbt4 and some other binding tools later, but so far I like the low footprint and no dependencies here.

Thank you.