Message ID | 20210312165017.31829-1-frederic.danis@collabora.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [Bluez,v2] adapter: Fix discovery trigger for 0 second delay | expand |
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 >
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
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.
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
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); }