Friendly ping on this series. On Tue, May 5, 2020 at 11:46 AM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > Friendly ping on this series. > > On Fri, Apr 17, 2020 at 5:03 PM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: >> >> On Fri, Apr 17, 2020 at 4:58 PM Luiz Augusto von Dentz >> <luiz.dentz@xxxxxxxxx> wrote: >> > >> > Hi Alain, >> > >> > On Fri, Apr 17, 2020 at 11:22 AM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: >> > > >> > > On Fri, Apr 17, 2020 at 2:16 PM Luiz Augusto von Dentz >> > > <luiz.dentz@xxxxxxxxx> wrote: >> > > > >> > > > Hi Alain, >> > > > >> > > > On Thu, Apr 16, 2020 at 5:34 PM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: >> > > > > >> > > > > HI Luiz, >> > > > > >> > > > > On Thu, Apr 16, 2020 at 8:29 PM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: >> > > > >> >> > > > >> Hi Alain, >> > > > >> >> > > > >> On Thu, Apr 16, 2020 at 1:32 PM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: >> > > > >> > >> > > > >> > Friendly ping on this series. >> > > > >> > >> > > > >> > >> > > > >> > On Fri, Apr 3, 2020 at 11:33 AM Alain Michaud <alainm@xxxxxxxxxxxx> wrote: >> > > > >> > > >> > > > >> > > This change adds an adapter Api to report the controller's >> > > > >> > > widebandspeech enabled state. >> > > > >> >> > > > >> I wonder if this shouldn't be queried over SCO socket, or simple fail >> > > > >> with BT_VOICE when using BT_VOICE_TRANSPARENT which is how what is >> > > > >> normally used when using mSBC. >> > > > > >> > > > > I think there is value in both. >> > > > >> > > > Can you expand on that? I think this might generate confusion if the >> > > > property indicates support for it but HFP implementation don't support >> > > > it, since the later is usually implemented as a external profile so we >> > > > don't have the features it may support, or perhaps the intention here >> > > > is to actually indicate when it is in use? >> > > >> > > This is a signal that the adapter supports it and has everything >> > > enabled to support it. Driver indicated it supports it and erroneous >> > > data reporting was enabled. The profile has it's own state which may >> > > indicate if msbc will be used, but this will be on a per connection >> > > basis and is independent from this adapter property. >> > > >> > > The value in this property is to support diagnostic UX about >> > > controller capabilities/state and also allow profiles that are >> > > implemented outside of bluetoothd to see which codec it can attempt to >> > > negotiate with the device. >> > >> > For diagnosic I think we would be better off with some dedicated >> > interface to query this, as for the later the information we are >> > giving does not actually tell anything about the codec support, which >> > was part of my original argument that for the likes of HFP and other >> > profiles using it it might not be enough and they still need to use >> > BT_VOICE in order to enable the use of custom codecs, if you take >> > ofono for example it does implement support for wideband speech >> > already and it would completely disregard this property which can give >> > the false impression that wideband speech cannot be activated when in >> > fact it can, it just don't have erroneous data reporting enable, so >> > perhaps we should indicate the actual adapter feature (e.g. >> > ErrnoneousDataReporting) not the profile feature here, so the profile >> > implementation can check weather this would disable use of wideband >> > speech or not, futhermore we should probably report the errors back to >> > the SCO socket or is that just for diagnostic and cannot be used to >> > adjust the streaming? >> >> My original patch actually had this MGMT feature called erroneous data >> reporting and Marcel argued against it. If you both agree, then I'm >> happy to rename all of this to erroneous data reporting. We'd still >> need some way for the driver support to be messaged some other way >> though. see: >> >> if (id->driver_info & BTUSB_WIDEBAND_SPEECH) >> set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks); >> >> >> > > > >> > > > >> > > > >> >> > > > >> >> > > > >> > > --- >> > > > >> > > >> > > > >> > > doc/adapter-api.txt | 8 ++++++++ >> > > > >> > > 1 file changed, 8 insertions(+) >> > > > >> > > >> > > > >> > > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt >> > > > >> > > index acae032d9..d8865e795 100644 >> > > > >> > > --- a/doc/adapter-api.txt >> > > > >> > > +++ b/doc/adapter-api.txt >> > > > >> > > @@ -326,3 +326,11 @@ Properties string Address [readonly] >> > > > >> > > >> > > > >> > > Local Device ID information in modalias format >> > > > >> > > used by the kernel and udev. >> > > > >> > > + >> > > > >> > > + boolean WidebandSpeechEnabled [readonly] >> > > > >> > > + >> > > > >> > > + Returns true if the adapter's wideband speech feature is >> > > > >> > > + supported and enabled. >> > > > >> >> > > > >> There seems to be some extra empty lines here. >> > > > > >> > > > > ACK, will fix. >> > > > >> >> > > > >> >> > > > >> > > + >> > > > >> > > + >> > > > >> > > + >> > > > >> > > -- >> > > > >> > > 2.26.0.292.g33ef6b2f38-goog >> > > > >> > > >> > > > >> >> > > > >> >> > > > >> >> > > > >> -- >> > > > >> Luiz Augusto von Dentz >> > > > >> > > > >> > > > >> > > > -- >> > > > Luiz Augusto von Dentz >> > >> > >> > >> > -- >> > Luiz Augusto von Dentz