It was reported that PulseAudio causes error messages in syslog from
dbus-daemon:
Jan 14 04:51:32 gentoo dbus-daemon[2492]: [system] Rejected send message, 2 matched rules; type="error", sender=":1.15" (uid=1000 pid=2864 comm="/usr/bin/pulseaudio --start --log-target=syslog ") interface="(unset)" member="(unset)" error name="org.bluez.MediaEndpoint1.Error.NotImplemented" requested_reply="0" destination=":1.1" (uid=0 pid=2670 comm="/usr/libexec/bluetooth/bluetoothd ")
The default policy on the system bus is to not let any method call
replies through if they have not been requested, and apparently
bluetoothd doesn't want replies to the Release() call.
This also changes the reply type from error to normal reply. The "not
implemented" error didn't make sense to me. We don't do any cleanup in
the Release() handler, probably because there's nothing to do. If there
is some cleanup that we should do, then it's a serious bug not to do it.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=104646
Use pa_assert_se() to check return value (pro forma) like everywhere else
Coverity ID: #154313
Signed-off-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
There are actually two HSP HS UUIDs. My theory is that the second one
was added, because someone was not happy with the old UUID being used
for identifying two different things (the HSP profile as a whole, and
the HS role within the HSP profile). Some headsets only use the new
UUID, and those headsets won't work if we don't recognize the new UUID.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93898
If a HFP audio gateway was connected via the ofono backend, pulse would
segfault during shutdown of the daemon. pa_bluetooth_discovery_unref()
removed the devices and transports before the ofono backend was freed.
Because the ofono backend keeps its own list of transports, transport_free()
was then called during termination of the ofono backend with an invalid
transport. Bug reported by Andrew Hlynskyi.
This patch moves the termination of the ofono and native backends before
freeing the devices.
The function can only fail if there's not enough memory available, and
if that happens, the convention in PulseAudio is to abort.
CID: 1353106, 1353108, 1353140
There were two bugs in the old logic. The first one:
If a device has two profiles, the old code would start the wait timer
when the first profile connects, but when the second profile connects,
the timer would not get stopped and the CONNECTION_CHANGED hook would
not get fired, because the code for that was inside an if block that
only gets executed when the first profile connects. As a result,
module-bluez5-device loading would always be delayed until the wait
timeout expires.
The second bug:
A crash was observed in device_start_waiting_for_profiles(). That
function is called whenever the connected profile count changes from 0
to 1. The function also has an assertion that checks that the timer is
not running when the function is called. That assertion crashed in the
following scenario with a headset that supports HSP and A2DP:
1. First HSP gets connected. The timer is started.
2. Then HSP gets disconnected for some reason. The timer is still
running.
3. Then A2DP gets connected. device_start_waiting_for_profiles() is
called, because the connected profile count changed from 0 to 1 again.
The timer is already running, so the assertion fails.
First I thought I'd remove the assertion from
device_start_waiting_for_profiles() and just restart the timer on the
second call, but then I figured that when the device returns to the
"everything disconnected" state in step 2, it would be better to stop
the timer. The purpose of the timer is to delay the notification of the
device becoming connected, but if the device becomes disconnected during
the waiting period, the notification doesn't make sense any more, and
therefore the timer doesn't make sense either.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100237
With headset=auto it is possible that AG devices are connected and handled
via the native backend when ofono is started. Because the HS role will then
be disabled in the native backend, AG devices must be disconnected and any
future connections will be handled by ofono.
This patch changes the behavior of the headset=auto switch for module-bluez5-discover.
With headset=auto now both backends will be active at the same time for the AG role and
the switching between the backends is only done for the HS role.
headset=ofono and headset=native remain unchanged.
This allows to use old HSP only headsets while running ofono and to have headset support
via pulseaudio if ofono is started with the --noplugin=hfp_ag_bluez5 option.
Add transport_set_state() that encapsulates changing the variable,
logging and firing the change hook.
I also made a cosmetic change to the corresponding BlueZ 5 log
message so that both messages have the format that I like.
A hashmap is more convenient than a linked list for storing the UUIDs,
so change the BlueZ 4 code accordingly.
Rename the BlueZ 4 UUID constants to match the BlueZ 5 naming.
The only changes to the BlueZ 5 code are the addition of one comment
and making another comment a bit clearer.
The properties_received flag affects whether the device should be
considered valid, so let's update the valid flag after setting the
properties_received flag.
There's a call to device_update_valid() anyway later when setting
the device adapters, so this change isn't strictly necessary, but
this makes it more obvious that the code is correct (and less
fragile).
The CONNECTION_CHANGED hook is used to notify the discovery module
about new and removed devices. When a bluetooth device connects, the
hook used to be called immediately when the first profile connected.
That meant that only one profile was marked as available during the
card creation, other profiles would get marked as available later.
That makes it hard for module-card-restore to restore the saved
profile, if the saved profile becomes available with some delay.
module-card-restore has a workaround for this problem, but that turned
out to interfere with module-bluetooth-policy, so the workaround will
be removed in the next patch.
The BlueZ 4 code doesn't need changes, because we use the
org.bluez.Audio interface to get a notification when all profiles are
connected.
FSF addresses used in PA sources are no longer valid and rpmlint
generates numerous warnings during packaging because of this.
This patch changes all FSF addresses to FSF web page according to
the GPL how-to: https://www.gnu.org/licenses/gpl-howto.en.html
Done automatically by sed-ing through sources.
Because the adapters reference the devices hashmap on free, we mush
free the adapters hashmap first and then the devices hashmap.
Reported-by: Alexander Patrakov <patrakov@gmail.com>
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
This patch adds a module argument "headset=ofono|native|auto" to
module-bluetooth-discover and module-bluez5-discover.
To make Arun's happy, the default is 'native' if compiled in, otherwise
'ofono'. 'Auto' will try to autoswitch depending on whether ofono is
running or not.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
This implements some autodetect if both headset backends are compiled in:
First we try to contact the oFono service, if that's not working,
then we start the native backend instead.
Likewise if the oFono service is going offline/online, we load/unload
the native backend accordingly.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
Enable both ofono and native backends to be built into the same
libbluez5-util. Never build the null backend.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
This should not have any effect on behaviour. The goal is to align
with the pattern that I think we should follow:
Object initialization:
- put() is the place to create references from other objects to the
newly created object. In this case, adding the transport to
discovery->transports was moved from new() to put, and adding the
transport to device->transports was moved from set_state() to
put().
Object destruction:
- unlink() undoes put() and removes all references from other objects
to the object being unlinked. In this case setting the
device->transports pointer to NULL was moved from set_state() to
unlink(), and setting the discovery->transports pointer to NULL was
moved from free() to unlink().
- free() undoes new(), but also calls unlink() so that object owners
don't need to remember to call unlink() before free().
This commit adds basic support for devices implementing HSP Headset
Unit, HSP Audio Gateway, HFP Handsfree Unit, HFP Audio Gateway to the
BlueZ 5 bluetooth audio devices driver module (module-bluez5-device).
There are several intertwined changes that I couldn't separate into
nicer commits. This is mostly just refactoring, but this also fixes
a bug: the old code set the device valid in parse_device_properties()
even if the device's adapter was invalid (had NULL address).
To improve the clarity of the code, I split the device_info_valid
variable into two booleans: properties_received and valid.
I added function device_update_valid() that checks all conditions that
affect the device validity. The function can then be called from any
place where something changes that potentially affects the device
validity. However, currently the only validity-affecting thing that
can change is the device adapter, so device_update_valid() is only
called from set_device_adapter().
I added the aforementioned set_device_adapter() function so that
whenever the adapter is set, the device validity gets updated
automatically.
The new properties_received variable allowed me to remove the
is_property_update function parameters.
This is a cosmetic change. There are a couple of places where we check
whether the adapter object is valid, and while checking whether the
address property is set works just fine, I find it nicer to have a
dedicated flag for the object validity. This improves maintainability
too, because if there will ever be more adapter properties that affect
the adapter validity, the places that check if the adapter is valid
don't need to be updated.
When parsing device properties, missing adapter will result in
device_info_valid being set to -1. It is then logical that if the
adapter goes missing at a later point, device_info_valid gets set to
-1 also in that situation.
The function did two things: set device_info_valid to -1 and called
device_free() for each device in the hashmap. Setting
device_info_valid to -1 was unnecessary. The main purpose of that was
to fire DEVICE_CONNECTION_CHANGED as a side effect, but that hook is
fired anyway in device_free(), as a side effect of removing all
transports. Calling device_free() can be delegated to pa_hashmap, when
freeing or emptying it.
Normally DEVICE_CONNECTION_CHANGED is fired when the first transport
becomes connected, but it may happen that the first transport becomes
connected already before the device properties have been received. In
that case the hook should be fired at the time the device properties
are received. This patch makes the hook to be fired at the right time.
At this point this doesn't make any other practical difference than
making the code more logical, but in the next patch I'll fire the
DEVICE_CONNECTION_CHANGED hook in set_device_info_valid(), and at that
point it's important that the device isn't marked valid too early,
because otherwise external code would see "valid" devices that however
don't have the adapter set.
The function was redundant, because all it did was call adapter_free()
for each adapter in the hashmap, and that can be delegated to
pa_hashmap when freeing or emptying it.
For quite some time now the device driver module doesn't work well
without the discovery module, so for the BlueZ 5 support we'll prevent
the device driver module to be loaded if the discovery module is not
loaded.
bluetoothd always send the GetManagedObjects() reply messages with the
objects array argument following an in-depth order starting from the
root. That means parents will always be known at the time their children
objects are parsed, if clients parse the objects in the same order they
appear in the array, as we do in PulseAudio.
This commit tries to protect PulseAudio in the case bluetoothd changes
that behavior for some reason. It hasn't been tested, since this
situation never occurs.
