2021-03-12 16:53:31

by Frédéric Danis

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

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


2021-03-12 18:32:14

by Luiz Augusto von Dentz

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

Hi Frédéric,

On Fri, Mar 12, 2021 at 8:53 AM Frédéric Danis
<[email protected]> 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

2021-03-12 19:12:38

by Frédéric Danis

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

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
> <[email protected]> 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

2021-03-12 19:42:05

by Luiz Augusto von Dentz

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

Hi Frédéric,

On Fri, Mar 12, 2021 at 11:09 AM Frédéric Danis
<[email protected]> 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
> > <[email protected]> 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

2021-03-15 22:40:26

by bluez.test.bot

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

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