| Message ID | 20240907204941.8006-1-vibhavp@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [BlueZ,1/2] device: Add ConnectionType property. | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T3 Title has trailing punctuation (.): "[BlueZ,1/2] device: Add ConnectionType property." |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=888046 ---Test result--- Test Summary: CheckPatch PASS 0.79 seconds GitLint FAIL 0.75 seconds BuildEll PASS 24.49 seconds BluezMake PASS 1648.94 seconds MakeCheck PASS 13.72 seconds MakeDistcheck PASS 176.37 seconds CheckValgrind PASS 250.72 seconds CheckSmatch PASS 353.18 seconds bluezmakeextell PASS 119.16 seconds IncrementalBuild PASS 2921.19 seconds ScanBuild PASS 982.22 seconds Details ############################## Test: GitLint - FAIL Desc: Run gitlint Output: [BlueZ,1/2] device: Add ConnectionType property. WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T3 Title has trailing punctuation (.): "[BlueZ,1/2] device: Add ConnectionType property." [BlueZ,2/2] org.bluez.Device: Add documentation for ConnectionType. WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T3 Title has trailing punctuation (.): "[BlueZ,2/2] org.bluez.Device: Add documentation for ConnectionType." --- Regards, Linux Bluetooth
Hi Vibhav, On Sat, Sep 7, 2024 at 4:50 PM Your Name <vibhavp@gmail.com> wrote: > > From: Vibhav Pant <vibhavp@gmail.com> > > --- > src/device.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/src/device.c b/src/device.c > index 0f18c8c7f..fa149f1d4 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -1003,6 +1003,32 @@ static gboolean dev_property_exists_class(const GDBusPropertyTable *property, > return device->class != 0; > } > > +static gboolean > +dev_property_get_connection_type(const GDBusPropertyTable *property, > + DBusMessageIter *iter, void *data) > +{ > + struct btd_device *device = data; > + const char *str; > + > + if (device->le && device->bredr) > + str = "dual"; > + else if (device->le) > + str = "le"; > + else > + str = "bredr"; > + dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &str); > + return TRUE; > +} > + > +static gboolean > +dev_property_exists_connection_type(const GDBusPropertyTable *property, > + void *data) > +{ > + struct btd_device *device = data; > + > + return device->bredr || device->le; > +} > + > static gboolean dev_property_get_class(const GDBusPropertyTable *property, > DBusMessageIter *iter, void *data) > { > @@ -3234,6 +3260,8 @@ static const GDBusPropertyTable device_properties[] = { > { "Alias", "s", dev_property_get_alias, dev_property_set_alias }, > { "Class", "u", dev_property_get_class, NULL, > dev_property_exists_class }, > + { "ConnectionType", "s", dev_property_get_connection_type, NULL, > + dev_property_exists_connection_type }, Don't really like where this is going, if we need bearer specific API like this then we need a whole new interface, besides you didn't really explain the reason why you needed this, is that to determine if there are SDP records to be fetched? We could perhaps implement something like a Bearer property alongside ConnectBearer but many of the properties of Device are sort of making sense only for one bearer. > { "Appearance", "q", dev_property_get_appearance, NULL, > dev_property_exists_appearance }, > { "Icon", "s", dev_property_get_icon, NULL, > -- > 2.46.0 > >
On Mon, Sep 9, 2024 at 9:00 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Vibhav, > > Don't really like where this is going, if we need bearer specific API > like this then we need a whole new interface, besides you didn't > really explain the reason why you needed this, is that to determine if > there are SDP records to be fetched? ConnectionType is needed only to differentiate between LE and BR/EDR devices. This is because the NT kernel creates device objects for every discovered Bluetooth device, and BR/EDR and LE devices exist under different paths/hierarchies (I suppose the equivalent would be BR/EDR devices existing under /dev/bluetooth/devices/bredr, and LE devices under /dev/bluetooth/devices/le). Essentially, BR/EDR and LE devices can be differentiated in several significant ways, and I could not find a way to reliably do that. The current code checks if AddressType is random, otherwise tries to see if the device object has org.bluez.Gatt* interfaces to determine if the connection bearer is LE. However, I understand that's not reliable, and doesn't determine if the device is connected using dual-mode. That being said, Bearer does sound like a better name for the property, and I can modify the patch to that effect. However, I don't foresee the need for a ConnectBearer property for Wine. Ultimately, my goal is to implement Bluetooth functionality with the minimum possible API surface added to BlueZ, hence the small (ish) patches.
Hi Vibhav, On Mon, Sep 9, 2024 at 12:36 PM Vibhav Pant <vibhavp@gmail.com> wrote: > > On Mon, Sep 9, 2024 at 9:00 PM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Vibhav, > > > > Don't really like where this is going, if we need bearer specific API > > like this then we need a whole new interface, besides you didn't > > really explain the reason why you needed this, is that to determine if > > there are SDP records to be fetched? > > ConnectionType is needed only to differentiate between LE and BR/EDR > devices. This is because the NT kernel creates device objects for > every discovered Bluetooth device, and BR/EDR and LE devices exist > under different paths/hierarchies (I suppose the equivalent would be > BR/EDR devices existing under /dev/bluetooth/devices/bredr, and LE > devices under /dev/bluetooth/devices/le). Essentially, BR/EDR and LE > devices can be differentiated in several significant ways, and I could > not find a way to reliably do that. The current code checks if > AddressType is random, otherwise tries to see if the device object has > org.bluez.Gatt* interfaces to determine if the connection bearer is > LE. However, I understand that's not reliable, and doesn't determine > if the device is connected using dual-mode. > > That being said, Bearer does sound like a better name for the > property, and I can modify the patch to that effect. However, I don't > foresee the need for a ConnectBearer property for Wine. Ultimately, my > goal is to implement Bluetooth functionality with the minimum possible > API surface added to BlueZ, hence the small (ish) patches. Ive left this out for now, we can revisit this if we find out that it is really necessary in the future. Btw, how is the wine support going, perhaps we can put something under doc or in the guthub wiki about the compatibility with windows. > -- > Vibhav Pant > vibhavp@gmail.com > GPG: 7ED1 D48C 513C A024 BE3A 785F E3FB 28CB 6AB5 9598
diff --git a/src/device.c b/src/device.c index 0f18c8c7f..fa149f1d4 100644 --- a/src/device.c +++ b/src/device.c @@ -1003,6 +1003,32 @@ static gboolean dev_property_exists_class(const GDBusPropertyTable *property, return device->class != 0; } +static gboolean +dev_property_get_connection_type(const GDBusPropertyTable *property, + DBusMessageIter *iter, void *data) +{ + struct btd_device *device = data; + const char *str; + + if (device->le && device->bredr) + str = "dual"; + else if (device->le) + str = "le"; + else + str = "bredr"; + dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &str); + return TRUE; +} + +static gboolean +dev_property_exists_connection_type(const GDBusPropertyTable *property, + void *data) +{ + struct btd_device *device = data; + + return device->bredr || device->le; +} + static gboolean dev_property_get_class(const GDBusPropertyTable *property, DBusMessageIter *iter, void *data) { @@ -3234,6 +3260,8 @@ static const GDBusPropertyTable device_properties[] = { { "Alias", "s", dev_property_get_alias, dev_property_set_alias }, { "Class", "u", dev_property_get_class, NULL, dev_property_exists_class }, + { "ConnectionType", "s", dev_property_get_connection_type, NULL, + dev_property_exists_connection_type }, { "Appearance", "q", dev_property_get_appearance, NULL, dev_property_exists_appearance }, { "Icon", "s", dev_property_get_icon, NULL,
From: Vibhav Pant <vibhavp@gmail.com> --- src/device.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)