Hi, On Fri, Sep 9, 2016 at 3:35 PM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > Hi Felipe, > > On Thu, Sep 8, 2016 at 6:37 PM, Felipe Ferreri Tonello > <eu@xxxxxxxxxxxxxxxxx> wrote: >> Hi Luiz, >> >> On 08/09/16 16:26, Luiz Augusto von Dentz wrote: >>> Hi Felipe, >>> >>> On Thu, Sep 8, 2016 at 5:45 PM, Felipe Ferreri Tonello >>> <eu@xxxxxxxxxxxxxxxxx> wrote: >>>> Hi Luiz, >>>> >>>> On 08/09/16 13:38, Luiz Augusto von Dentz wrote: >>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> >>>>> >>>>> The accept calback may transit the state to connected on the call itself >>>>> since most of the time it is just a matter of selecting the attributes >>>>> in case of GATT profiles. >>>>> --- >>>>> src/service.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/src/service.c b/src/service.c >>>>> index f387fc4..20a41d0 100644 >>>>> --- a/src/service.c >>>>> +++ b/src/service.c >>>>> @@ -209,7 +209,8 @@ int service_accept(struct btd_service *service) >>>>> return err; >>>>> >>>>> done: >>>>> - change_state(service, BTD_SERVICE_STATE_CONNECTING, 0); >>>>> + if (service->state == BTD_SERVICE_STATE_DISCONNECTED) >>>>> + change_state(service, BTD_SERVICE_STATE_CONNECTING, 0); >>>> >>>> This looks really hacky - I know it was like this before already. >>> >>> This is to avoid resetting the connection if the accept callback has >>> set it already. >>> >>>> Why can you change the state in two different places? What if the >>>> profile->accept doesn't change the state to connected? Will this change >>>> to connecting and then the connected state will never be set? >>> >>> Going from connected to connecting is not valid, thus if the driver >>> had changed the states there is nothing else to be done. >> >> Why do you have this done label if the driver is supposed to call >> btd_service_connecting_complete() on success? >>> >>>> I think btd_service_connecting_complete() should always be called with >>>> the same err as returned by profile->accept() >>> >>> Not sure what difference does this make? >> >> My suggestion is to handle the state based on the return value of the >> driver's callback and not let the driver to change the state itself. >> It's boilerplate code anyway. > > The driver may have to respond this asynchronously so we can't really > depend on the return alone. Applied. -- Luiz Augusto von Dentz -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
