[PATCH Bluez v2] adapter: Fix discovery trigger for 0 second delay

[PATCH Bluez v2] adapter: Fix discovery trigger for 0 second delay

[Frédéric Danis]

When calling `StartDiscovery` the effective start can take around 10 ms or
up to 700 ms.
g_timeout_add_seconds() call doesn't ensure the time for the first call of
the timer if the delay is less or equal to 1 second.
---
v2: Fix issue founs by CI

 src/adapter.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index cc0849f99..3078ce1a8 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1797,6 +1797,13 @@ static void trigger_start_discovery(struct btd_adapter *adapter, guint delay)
 	if (!btd_adapter_get_powered(adapter))
 		return;
 
+	if (!delay) {
+		adapter->discovery_idle_timeout = g_idle_add(
+						start_discovery_timeout,
+						adapter);
+		return;
+	}
+
 	adapter->discovery_idle_timeout = g_timeout_add_seconds(delay,
 					start_discovery_timeout, adapter);
 }
-- 
2.18.0

Re: [PATCH Bluez v2] adapter: Fix discovery trigger for 0 second delay

[Luiz Augusto von Dentz]

Hi Frédéric,

On Fri, Mar 12, 2021 at 8:53 AM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> When calling `StartDiscovery` the effective start can take around 10 ms or
> up to 700 ms.
> g_timeout_add_seconds() call doesn't ensure the time for the first call of
> the timer if the delay is less or equal to 1 second.

Interesting, I always thought that 0 would be handle just as idle and
not round up to the next timeout.

> ---
> v2: Fix issue founs by CI
>
>  src/adapter.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index cc0849f99..3078ce1a8 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1797,6 +1797,13 @@ static void trigger_start_discovery(struct btd_adapter *adapter, guint delay)
>         if (!btd_adapter_get_powered(adapter))
>                 return;
>
> +       if (!delay) {
> +               adapter->discovery_idle_timeout = g_idle_add(
> +                                               start_discovery_timeout,
> +                                               adapter);
> +               return;
> +       }
> +
>         adapter->discovery_idle_timeout = g_timeout_add_seconds(delay,
>                                         start_discovery_timeout, adapter);

Maybe we should have a wrapper function for g_timeout_add_seconds
since I suspect there might be other instances of
g_timeout_add_seconds with 0 delay.

>  }
> --
> 2.18.0
>


-- 
Luiz Augusto von Dentz

Re: [PATCH Bluez v2] adapter: Fix discovery trigger for 0 second delay

[Frédéric Danis]

Hi Luiz,

On 12/03/2021 19:28, Luiz Augusto von Dentz wrote:
> Hi Frédéric,
>
> On Fri, Mar 12, 2021 at 8:53 AM Frédéric Danis
> <frederic.danis@collabora.com> wrote:
>> When calling `StartDiscovery` the effective start can take around 10 ms or
>> up to 700 ms.
>> g_timeout_add_seconds() call doesn't ensure the time for the first call of
>> the timer if the delay is less or equal to 1 second.
> Interesting, I always thought that 0 would be handle just as idle and
> not round up to the next timeout.
>
>> ---
>> v2: Fix issue founs by CI
>>
>>   src/adapter.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/src/adapter.c b/src/adapter.c
>> index cc0849f99..3078ce1a8 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -1797,6 +1797,13 @@ static void trigger_start_discovery(struct btd_adapter *adapter, guint delay)
>>          if (!btd_adapter_get_powered(adapter))
>>                  return;
>>
>> +       if (!delay) {
>> +               adapter->discovery_idle_timeout = g_idle_add(
>> +                                               start_discovery_timeout,
>> +                                               adapter);
>> +               return;
>> +       }
>> +
>>          adapter->discovery_idle_timeout = g_timeout_add_seconds(delay,
>>                                          start_discovery_timeout, adapter);
> Maybe we should have a wrapper function for g_timeout_add_seconds
> since I suspect there might be other instances of
> g_timeout_add_seconds with 0 delay.

Ok
Is adding a timeout_add_seconds() function to src/shared/timeout.h the 
right place?

Fred

Re: [PATCH Bluez v2] adapter: Fix discovery trigger for 0 second delay

[Luiz Augusto von Dentz]

Hi Frédéric,

On Fri, Mar 12, 2021 at 11:09 AM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> Hi Luiz,
>
> On 12/03/2021 19:28, Luiz Augusto von Dentz wrote:
> > Hi Frédéric,
> >
> > On Fri, Mar 12, 2021 at 8:53 AM Frédéric Danis
> > <frederic.danis@collabora.com> wrote:
> >> When calling `StartDiscovery` the effective start can take around 10 ms or
> >> up to 700 ms.
> >> g_timeout_add_seconds() call doesn't ensure the time for the first call of
> >> the timer if the delay is less or equal to 1 second.
> > Interesting, I always thought that 0 would be handle just as idle and
> > not round up to the next timeout.
> >
> >> ---
> >> v2: Fix issue founs by CI
> >>
> >>   src/adapter.c | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/src/adapter.c b/src/adapter.c
> >> index cc0849f99..3078ce1a8 100644
> >> --- a/src/adapter.c
> >> +++ b/src/adapter.c
> >> @@ -1797,6 +1797,13 @@ static void trigger_start_discovery(struct btd_adapter *adapter, guint delay)
> >>          if (!btd_adapter_get_powered(adapter))
> >>                  return;
> >>
> >> +       if (!delay) {
> >> +               adapter->discovery_idle_timeout = g_idle_add(
> >> +                                               start_discovery_timeout,
> >> +                                               adapter);
> >> +               return;
> >> +       }
> >> +
> >>          adapter->discovery_idle_timeout = g_timeout_add_seconds(delay,
> >>                                          start_discovery_timeout, adapter);
> > Maybe we should have a wrapper function for g_timeout_add_seconds
> > since I suspect there might be other instances of
> > g_timeout_add_seconds with 0 delay.
>
> Ok
> Is adding a timeout_add_seconds() function to src/shared/timeout.h the
> right place?

Yep, that should be the right place.

-- 
Luiz Augusto von Dentz

RE: [Bluez,v2] adapter: Fix discovery trigger for 0 second delay

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

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=447189

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth