blueman-project / blueman

Blueman is a GTK+ Bluetooth Manager
GNU General Public License v3.0
1.27k stars 192 forks source link

blueman and pipewire #1511

Closed joakim-tjernlund closed 3 years ago

joakim-tjernlund commented 3 years ago

blueman:2.1.4 BlueZ: master Distribution: Gentoo Desktop environment: MATE

I get this from blueman when using pipewire pulseaudio emulation:

Exception ignored on calling ctypes callback function: <bound method PulseAudioUtils.__list_callback of <PulseAudioUtils.PulseAudioUtils object at 0x7f448b4ac100 (blueman+main+PulseAudioUtils+PulseAudioUtils at 0x5595f1962d80)>>
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/blueman/main/PulseAudioUtils.py", line 399, in __list_callback
    info["handler"](None, True)
  File "/usr/lib/python3.8/site-packages/blueman/main/PulseAudioUtils.py", line 549, in handler
    callback(data)
  File "/usr/lib/python3.8/site-packages/blueman/plugins/manager/PulseAudioProfile.py", line 66, in list_cb
    if c["proplist"]["device.string"] == device['Address']:
KeyError: 'device.string'

Connection to a HFP device works but the Profile submenu does not work/empty. I guess PW is missing some PA DBUS signaling? What is missing?

joakim-tjernlund commented 3 years ago

Wrong log:


 blueman-manager --loglevel debug
blueman-manager 01.23.13 INFO     PluginManager:86 load_plugin: ['PulseAudioProfile', 'Services', 'Notes', 'Info']
blueman-manager 01.23.13 INFO     PluginManager:157 __load_plugin: loading <class 'blueman.plugins.manager.PulseAudioProfile.PulseAudioProfile'>
blueman-manager 01.23.13 INFO     PulseAudioUtils:355 pa_context_event: 1
blueman-manager 01.23.13 INFO     PluginManager:157 __load_plugin: loading <class 'blueman.plugins.manager.Services.Services'>
blueman-manager 01.23.13 INFO     PluginManager:157 __load_plugin: loading <class 'blueman.plugins.manager.Notes.Notes'>
blueman-manager 01.23.13 INFO     PluginManager:157 __load_plugin: loading <class 'blueman.plugins.manager.Info.Info'>
blueman-manager version 2.1.4 starting
Stale PID, overwriting
blueman-manager 01.23.13 INFO     PulseAudioUtils:355 pa_context_event: 2
blueman-manager 01.23.13 INFO     PulseAudioUtils:355 pa_context_event: 3
blueman-manager 01.23.13 INFO     Manager:102 on_dbus_name_appeared: org.bluez :1.163
blueman-manager 01.23.13 DEBUG    Base:74 do_g_properties_changed: /org/bluez/hci0 {'Address': '5C:F3:70:8D:C8:08', 'AddressType': 'public', 'Name': 'jocke', 'Alias': 'jocke', 'Class': 7078148, 'Powered': True, 'Discoverable': False, 'DiscoverableTimeout': 0, 'Pairable': True, 'PairableTimeout': 0, 'Discovering': False, 'UUIDs': ['0000110e-0000-1000-8000-00805f9b34fb', '0000111f-0000-1000-8000-00805f9b34fb', '00001200-0000-1000-8000-00805f9b34fb', '0000110b-0000-1000-8000-00805f9b34fb', '00001108-0000-1000-8000-00805f9b34fb', '0000110c-0000-1000-8000-00805f9b34fb', '00001800-0000-1000-8000-00805f9b34fb', '0000110a-0000-1000-8000-00805f9b34fb', '00001801-0000-1000-8000-00805f9b34fb', '0000180a-0000-1000-8000-00805f9b34fb'], 'Modalias': 'usb:v1D6Bp0246d0538', 'Roles': ['central', 'peripheral']}
blueman-manager 01.23.13 DEBUG    DeviceList:238 set_adapter: Setting adapter to: None 
blueman-manager 01.23.13 DEBUG    ManagerToolbar:62 on_adapter_changed: toolbar adapter /org/bluez/hci0
blueman-manager 01.23.13 DEBUG    Base:74 do_g_properties_changed: /org/bluez/hci0/dev_41_00_00_00_22_DA {'Address': '41:00:00:00:22:DA', 'AddressType': 'public', 'Name': 'HS6000', 'Alias': 'HS6000', 'Class': 2360324, 'Icon': 'audio-card', 'Paired': True, 'Trusted': True, 'Blocked': False, 'LegacyPairing': False, 'Connected': True, 'UUIDs': ['0000110b-0000-1000-8000-00805f9b34fb', '0000110c-0000-1000-8000-00805f9b34fb', '0000110d-0000-1000-8000-00805f9b34fb', '0000110e-0000-1000-8000-00805f9b34fb', '0000111e-0000-1000-8000-00805f9b34fb', '00001131-0000-1000-8000-00805f9b34fb', '00001200-0000-1000-8000-00805f9b34fb'], 'Modalias': 'bluetooth:v000Fp0000d0000', 'Adapter': '/org/bluez/hci0', 'ServicesResolved': True}
blueman-manager 01.23.13 DEBUG    Base:74 do_g_properties_changed: /org/bluez/hci0/dev_D8_2F_A9_50_F6_04 {'Address': 'D8:2F:A9:50:F6:04', 'AddressType': 'public', 'Name': 'TOUCH ONE', 'Alias': 'TOUCH ONE', 'Class': 2360324, 'Icon': 'audio-card', 'Paired': True, 'Trusted': True, 'Blocked': False, 'LegacyPairing': False, 'Connected': False, 'UUIDs': ['0000110b-0000-1000-8000-00805f9b34fb', '0000110c-0000-1000-8000-00805f9b34fb', '0000110e-0000-1000-8000-00805f9b34fb', '0000111e-0000-1000-8000-00805f9b34fb', '00001200-0000-1000-8000-00805f9b34fb'], 'Modalias': 'bluetooth:v05D6p000Ad0240', 'Adapter': '/org/bluez/hci0', 'ServicesResolved': False}
blueman-manager 01.23.13 DEBUG    Base:74 do_g_properties_changed: /org/bluez/hci0/dev_FC_58_FA_1A_09_13 {'Address': 'FC:58:FA:1A:09:13', 'AddressType': 'public', 'Name': 'Urbanista Berlin', 'Alias': 'Urbanista Berlin', 'Class': 2360324, 'Icon': 'audio-card', 'Paired': True, 'Trusted': True, 'Blocked': False, 'LegacyPairing': False, 'Connected': False, 'UUIDs': ['00001108-0000-1000-8000-00805f9b34fb', '0000110b-0000-1000-8000-00805f9b34fb', '0000110c-0000-1000-8000-00805f9b34fb', '0000110d-0000-1000-8000-00805f9b34fb', '0000110e-0000-1000-8000-00805f9b34fb', '0000111e-0000-1000-8000-00805f9b34fb'], 'Adapter': '/org/bluez/hci0', 'ServicesResolved': False}
blueman-manager 01.23.13 INFO     DeviceList:280 add_device: adding new device
blueman-manager 01.23.13 INFO     DeviceList:388 do_cache  : Caching new device /org/bluez/hci0/dev_41_00_00_00_22_DA
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Trusted True
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Paired True
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Connected True
blueman-manager 01.23.13 INFO     DeviceList:388 do_cache  : Caching new device /org/bluez/hci0/dev_41_00_00_00_22_DA
blueman-manager 01.23.13 INFO     DeviceList:185 monitor_power_levels: starting monitor
blueman-manager 01.23.13 INFO     ManagerDeviceList:354 level_setup_event: animating up
blueman-manager 01.23.13 INFO     DeviceList:280 add_device: adding new device
blueman-manager 01.23.13 INFO     DeviceList:388 do_cache  : Caching new device /org/bluez/hci0/dev_D8_2F_A9_50_F6_04
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Trusted True
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Paired True
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Connected False
blueman-manager 01.23.13 INFO     DeviceList:388 do_cache  : Caching new device /org/bluez/hci0/dev_D8_2F_A9_50_F6_04
blueman-manager 01.23.13 INFO     DeviceList:280 add_device: adding new device
blueman-manager 01.23.13 INFO     DeviceList:388 do_cache  : Caching new device /org/bluez/hci0/dev_FC_58_FA_1A_09_13
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Trusted True
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Paired True
blueman-manager 01.23.13 INFO     ManagerDeviceList:289 row_update_event: Connected False
blueman-manager 01.23.13 INFO     DeviceList:388 do_cache  : Caching new device /org/bluez/hci0/dev_FC_58_FA_1A_09_13
blueman-manager 01.23.13 DEBUG    ManagerDeviceMenu:259 generate  : HS6000
blueman-manager 01.23.13 INFO     PulseAudioUtils:355 pa_context_event: 4
blueman-manager 01.23.13 INFO     PulseAudioProfile:28 on_pa_ready: connected
blueman-manager 01.23.13 DEBUG    ManagerDeviceMenu:259 generate  : HS6000
Exception ignored on calling ctypes callback function: <bound method PulseAudioUtils.__list_callback of <PulseAudioUtils.PulseAudioUtils object at 0x7f0cac884a00 (blueman+main+PulseAudioUtils+PulseAudioUtils at 0x55b0e2ae3980)>>
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/blueman/main/PulseAudioUtils.py", line 399, in __list_callback
    info["handler"](None, True)
  File "/usr/lib/python3.8/site-packages/blueman/main/PulseAudioUtils.py", line 549, in handler
    callback(data)
  File "/usr/lib/python3.8/site-packages/blueman/plugins/manager/PulseAudioProfile.py", line 66, in list_cb
    if c["proplist"]["device.string"] == device['Address']:
KeyError: 'device.string'
blueman-manager 01.23.13 INFO     PulseAudioUtils:361 <lambda>  : 1
joakim-tjernlund commented 3 years ago

So PW was missing the device.string prop. Adding that made Profile work, I could see them and change but the change was not visible to blueman until I restarted blueman so I guess PW is missing some notification prop, which ?

cschramm commented 3 years ago

Not sure what you mean with "notification prop". If it's about a pulseaudio event, blueman tries to add a card for all card events except remove but it might very well be that the driver provided by PipeWire does not match PulseAudioProfile.py#L49. I think blueman even queries pulseaudio when you open a right-click menu, so handling events should not be critical.

joakim-tjernlund commented 3 years ago

I guess a PA event that something changed so the active entry/profile changes in the menu. If I change profile from HFP to Off I can see that it did have effect but the menu selection still stays at HFP in blueman.

joakim-tjernlund commented 3 years ago

OK, so PW has card["driver"] == "bluez5" would you mind adding that to the list of drivers?

joakim-tjernlund commented 3 years ago

Do think that the "device.string" property is very PA specific or should any daemon(like native PW) also have have it? Trying to find out if this prop should only be in PWs PA emulation or everywhere.

joakim-tjernlund commented 3 years ago

Trying to work out what DBUS property card["driver"] is, can you let me know?

joakim-tjernlund commented 3 years ago

Found the card["driver"] info, patch sent to pipewire to change it to "module-bluez5-device.c" Now blueman works :)

infirit commented 3 years ago

Ok then 🤔.

@cschramm I've never seen an error like that. Is it something we need to catch/handle in blueman?

cschramm commented 3 years ago

Well, pulseaudio provides a property called device.string that contains the MAC address. The PipeWire pulseaudio emulation does not seem to have that. I did not get how you made it available, though, @joakim-tjernlund.

joakim-tjernlund commented 3 years ago

PW didn't set that prop and I sniffed it out. Then PW committed a fix to PW, easy :)

The other issue with card["driver"] is another story. BM should not relay on that, it is just informal. You have already had to add 3 different names to match against. Please find a better way.

joakim-tjernlund commented 3 years ago

Reopened w.r.t the card["driver"] matching rule

cschramm commented 3 years ago

I checked pulseaudio and found the following characteristics of the properties set by the bluetooth drivers (basically always, at least since 0.9.15):

PipeWire just added device.string in 0.3.24 (🔗). It also seems to set device.api to bluez5 and device.bus to bluetooth. It does not set bluez.path(there is api.bluez5.path instead).

So I'd say checking for a device.bus property that is set to bluetooth would be best and we can spare the driver check.

Can you confirm that the properties do get exposed in the pulseaudio API like that? Would be best to see your pacmd list-cards.

joakim-tjernlund commented 3 years ago

pacmd does not work in PW, I got this from pw-cli though:

type: PipeWire:Interface:Device/3
    properties:
        device.api = "bluez5"
        media.class = "Audio/Device"
        device.name = "bluez_card.41_00_00_00_22_DA"
        device.description = "HS6000"
        device.alias = "HS6000"
        device.icon-name = "audio-card"
        device.form-factor = "headset"
        device.string = "41:00:00:00:22:DA"
        api.bluez5.path = "/org/bluez/hci0/dev_41_00_00_00_22_DA"
        api.bluez5.address = "41:00:00:00:22:DA"
        api.bluez5.device = ""
        api.bluez5.class = "0x240404"
        api.bluez5.connection = "connected"
        bluez5.msbc-support = "true"
        factory.id = "14"
        client.id = "29"
        object.id = "51"
        device.bus = "bluetooth"

So device.bus = "bluetooth" is the common one for now at least and it is and it is an good one. matching anything with bluez* in it is not stable either, it the name of the SW stack which may change.

PW is still open for changes here, especially when it come to PA emulation

joakim-tjernlund commented 3 years ago

if APi is important to you too you could also match against device.api = "bluez*"

It is much better than "driver" anyhow

infirit commented 3 years ago

@cschramm device.bus looks promising as a replacement.

infirit commented 3 years ago

Another thing to fix is the active profile. It is not updating properly when switching, initially it is correct though.

infirit commented 3 years ago

PipeWire just added device.string in 0.3.24 (link). It also seems to set device.api to bluez5 and device.bus to bluetooth. It does not set bluez.path(there is api.bluez5.path instead).

So I'd say checking for a device.bus property that is set to bluetooth would be best and we can spare the driver check.

Can you confirm that the properties do get exposed in the pulseaudio API like that? Would be best to see your pacmd list-cards.

Unfortunately pipewire reports bluez5 for device.bus using the pulse api, even on 0.3.24.

@joakim-tjernlund if pipewire wants to be a drop in replacement they need to make sure that these things are identical. This is also the reason the active profile is not being updated even after updating to using device.bus.

edit: quick patch to make it work, pipewire needs to report the same device.bus string as pulse

diff --git a/blueman/main/PulseAudioUtils.py b/blueman/main/PulseAudioUtils.py
index 2bf65bc8..e47d91d3 100644
--- a/blueman/main/PulseAudioUtils.py
+++ b/blueman/main/PulseAudioUtils.py
@@ -30,7 +30,7 @@ if TYPE_CHECKING:
         name: str
         proplist: Dict[str, str]
         owner_module: int
-        driver: str
+        bus: str
         index: int
         profiles: List[CardProfileInfo]
         active_profile: str
@@ -98,7 +98,7 @@ PaCardInfo._fields_ = [
     ('index', c_uint32),
     ('name', c_char_p),
     ('owner_module', c_uint32),
-    ('driver', c_char_p),
+    ('bus', c_char_p),
     ('n_profiles', c_uint32),
     ('profiles', POINTER(PaCardProfileInfo)),
     ('active_profile', POINTER(PaCardProfileInfo)),
@@ -275,7 +275,7 @@ class PulseAudioUtils(GObject.GObject, metaclass=SingletonGObjectMeta):
             "name": card_info[0].name.decode("UTF-8"),
             "proplist": self.__get_proplist(card_info[0].proplist),
             "owner_module": card_info[0].owner_module,
-            "driver": card_info[0].driver.decode("UTF-8"),
+            "bus": card_info[0].bus.decode("UTF-8"),
             "index": card_info[0].index,
             "profiles": [{
                 "name": card_info[0].profiles[i].name.decode("UTF-8"),
diff --git a/blueman/plugins/manager/PulseAudioProfile.py b/blueman/plugins/manager/PulseAudioProfile.py
index 9303c04c..97bc0ef1 100644
--- a/blueman/plugins/manager/PulseAudioProfile.py
+++ b/blueman/plugins/manager/PulseAudioProfile.py
@@ -46,11 +46,7 @@ class PulseAudioProfile(ManagerPlugin, MenuItemsProvider):
         logging.debug(f"{event} {idx}")

         def get_card_cb(card: "CardInfo") -> None:
-            drivers = ("module-bluetooth-device.c",
-                       "module-bluez4-device.c",
-                       "module-bluez5-device.c")
-
-            if card["driver"] in drivers:
+            if card["bus"] in ("bluetooth", "bluez5"):
                 self.devices[card["proplist"]["device.string"]] = card
                 self.regenerate_with_device(card["proplist"]["device.string"])
infirit commented 3 years ago

Ugh, pipewires is inconsistent, for my sennheiser bt4.40 it reports bluez5 but for my android phone it is bluetooth :disappointed: .

@cschramm we will need to check if device.bus is bluetooth and bluez5 if we want to support pipewire. But then what is the point of using device.bus as we can just add bluez5 to driverlist.

infirit commented 3 years ago

https://gitlab.freedesktop.org/pipewire/pipewire/-/commit/d983c5146932d0c68308e5c9aae9870122231e0a??

We don't need that, it is what we used until now. I personally would prefer that commit got reverted because now it looks like there is an exception just for us and we are not willing to discuss alternatives which we are right now.

infirit commented 3 years ago

This is all we need. @cschramm if you want to test and commit feel free :smiley:

diff --git a/blueman/plugins/manager/PulseAudioProfile.py b/blueman/plugins/manager/PulseAudioProfile.py
index 9303c04c..b5ee8a14 100644
--- a/blueman/plugins/manager/PulseAudioProfile.py
+++ b/blueman/plugins/manager/PulseAudioProfile.py
@@ -46,11 +46,7 @@ class PulseAudioProfile(ManagerPlugin, MenuItemsProvider):
         logging.debug(f"{event} {idx}")

         def get_card_cb(card: "CardInfo") -> None:
-            drivers = ("module-bluetooth-device.c",
-                       "module-bluez4-device.c",
-                       "module-bluez5-device.c")
-
-            if card["driver"] in drivers:
+            if card["proplist"]["device.bus"] == "bluetooth":
                 self.devices[card["proplist"]["device.string"]] = card
                 self.regenerate_with_device(card["proplist"]["device.string"])
cschramm commented 3 years ago

APi [...] is much better than "driver" anyhow

I wouldn't say so.

Taking a step back, what we need is a way to detect constellations in which device.string holds a Bluetooth MAC address. We know that it's true for the three pa drivers that we match. They also set device.api to bluez and device.bus to bluetooth. I guess there's nothing that defines semantics of that as "in that case, device.string holds the Bluetooth MAC address" but it just holds.

It makes some sense that pw does not emulate a pa driver name and as long as nothing comes up that sets device.bus to bluetooth but no MAC address in device.string, we're perfectly fine with that scheme.

@infirit: Your Sennheiser comment confused me. Have you been confused there as well? :smile:

Regarding the patch I guess the proplist dictionary access fails if the the property is not set and it looks like device.bus is not guaranteed to be set. From the code it seems to be absent at least for RAOP devices.

joakim-tjernlund commented 3 years ago

Another thing to fix is the active profile. It is not updating properly when switching, initially it is correct though.

Can you bring that to PW please?

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

joakim-tjernlund commented 3 years ago

I think this needs some more attention. Right now BM needs the PWs PA emulation but BM should strive to work with pure PW too.

cschramm commented 3 years ago

You mean native PipeWire support? Why? Its PulseAudio backend seems to be a vital part of the PipeWire concept.

To wrap up what we discussed: blueman should, could and would change the matching to common characteristics not specific to PulseAudio drivers so that it matches cards from PipeWire's PulseAudio backend as well. With the commit to PipeWire there's nothing left to do, though, as they now show the PulseAudio module-bluez5-device.c driver. The comment on blueman needing that is rather impudent.

I'd also say that the commit would keep us from adding support for the PipeWire API. If we'd detect and talk to PipeWire natively, it would not make sense to also handle cards from its PulseAudio backend, so we'd need to detect and ignore those and the best way would be to either find a PipeWire driver on them or at least no driver at all but now it's set to a PulseAudio driver name.