2009-08-24 18:26:42

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: sniff mode

Hi,

It is just me or hci_conn_enter_active_mode will never do what its
name suggests. This check will always succeed:

if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
goto timer;

But in hci_mode_change_evt whatever mode different than HCI_CM_ACTIVE
reset power_save to zero:


if (!test_and_clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
if (conn->mode == HCI_CM_ACTIVE)
conn->power_save = 1;
else
conn->power_save = 0;
}

So it seems that if mode is SNIFF power_save is 0, so this check will
only fail if mode is in fact HCI_CM_ACTIVE, and then latter on we got
this:


if (!test_and_set_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
struct hci_cp_exit_sniff_mode cp;
cp.handle = cpu_to_le16(conn->handle);
hci_send_cmd(hdev, HCI_OP_EXIT_SNIFF_MODE, sizeof(cp), &cp);
}

I was expecting that only when mode is HCI_CM_SNIFF it would send
HCI_OP_EXIT_SNIFF_MODE, or Im missing something?


--
Luiz Augusto von Dentz
Engenheiro de Computa??o


2009-08-26 18:04:51

by Marcel Holtmann

[permalink] [raw]
Subject: Re: sniff mode

Hi Ville,

> >>>> It is just me or hci_conn_enter_active_mode will never do what its
> >>>> name suggests. This check will always succeed:
> >>>>
> >>>> if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> >>>> goto timer;
> >>> so conn->power_save is used to disable automatic sniff mode for incoming
> >>> connections. Mainly HID since they manager it by themselves.
> >> Hmm, but this seems to be done for any device regardless of its type.
> >> So for example we start sending/receiving data to a headset it will
> >> triggers hci_conn_enter_active_mode which I thought would exit sniff
> >> mode, right? (yep, I have seem some headset that doesn't leave sniff
> >> mode by their own).
> >
> > I said this before. We need a socket option for L2CAP and RFCOMM that
> > can trigger leaving sniff mode even if the remote device initiated the
> > connection.
> >
>
> I guess I need to take this on my TODO list. Or is some one already
> working on this?

go ahead. I am not working on it and I don't know of anybody else who
has plans for it.

Regards

Marcel



2009-08-26 07:21:38

by Ville Tervo

[permalink] [raw]
Subject: Re: sniff mode

ext Marcel Holtmann wrote:
> Hi Luiz,
>
>>>> It is just me or hci_conn_enter_active_mode will never do what its
>>>> name suggests. This check will always succeed:
>>>>
>>>> if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
>>>> goto timer;
>>> so conn->power_save is used to disable automatic sniff mode for incoming
>>> connections. Mainly HID since they manager it by themselves.
>> Hmm, but this seems to be done for any device regardless of its type.
>> So for example we start sending/receiving data to a headset it will
>> triggers hci_conn_enter_active_mode which I thought would exit sniff
>> mode, right? (yep, I have seem some headset that doesn't leave sniff
>> mode by their own).
>
> I said this before. We need a socket option for L2CAP and RFCOMM that
> can trigger leaving sniff mode even if the remote device initiated the
> connection.
>

I guess I need to take this on my TODO list. Or is some one already
working on this?

--
Ville

2009-08-24 21:12:31

by Marcel Holtmann

[permalink] [raw]
Subject: Re: sniff mode

Hi Luiz,

> >> It really seems the opposite, like we were disabling the automatic
> >> active mode since this make us to always hit goto timer; while in
> >> sniff mode. Perhaps this is meant for when remote device has been
> >> controlling modes, which sound strange to me since hci_mode_change_evt
> >> is called even when the we actively change the mode, right?
> >> (hci_conn_idle does that)
> >
> > I am not following.
>
> Perhaps it is just me, but I don't see any attempt to detect which
> part has initiated the connection, so regardless of who has been the
> initiator, on any mode change different than active we will reset the
> power_save to 0 which will prevent any attempt to leave sniff mode.

actually it is not based on who initiated the connection, it is based on
who initiated the sniff mode:

if (conn->mode == HCI_CM_ACTIVE)
conn->power_save = 1;
else
conn->power_save = 0;

If we are back to active mode, then reset power_save and trigger
automatic sniff-mode else disable it.

Regards

Marcel



2009-08-24 20:58:22

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: sniff mode

Hi Marcel,

On Mon, Aug 24, 2009 at 4:33 PM, Marcel Holtmann<[email protected]> wrote=
:
>> It really seems the opposite, like we were disabling the automatic
>> active mode since this make us to always hit goto timer; while in
>> sniff mode. Perhaps this is meant for when remote device has been
>> controlling modes, which sound strange to me since hci_mode_change_evt
>> is called even when the we actively change the mode, right?
>> (hci_conn_idle does that)
>
> I am not following.

Perhaps it is just me, but I don't see any attempt to detect which
part has initiated the connection, so regardless of who has been the
initiator, on any mode change different than active we will reset the
power_save to 0 which will prevent any attempt to leave sniff mode.

--=20
Luiz Augusto von Dentz
Engenheiro de Computa=E7=E3o

2009-08-24 19:33:48

by Marcel Holtmann

[permalink] [raw]
Subject: Re: sniff mode

Hi Luiz,

> >> It is just me or hci_conn_enter_active_mode will never do what its
> >> name suggests. This check will always succeed:
> >>
> >> if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> >> goto timer;
> >
> > so conn->power_save is used to disable automatic sniff mode for incoming
> > connections. Mainly HID since they manager it by themselves.
>
> Hmm, but this seems to be done for any device regardless of its type.
> So for example we start sending/receiving data to a headset it will
> triggers hci_conn_enter_active_mode which I thought would exit sniff
> mode, right? (yep, I have seem some headset that doesn't leave sniff
> mode by their own).

I said this before. We need a socket option for L2CAP and RFCOMM that
can trigger leaving sniff mode even if the remote device initiated the
connection.

> >> But in hci_mode_change_evt whatever mode different than HCI_CM_ACTIVE
> >> reset power_save to zero:
> >>
> >>
> >> if (!test_and_clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
> >> if (conn->mode == HCI_CM_ACTIVE)
> >> conn->power_save = 1;
> >> else
> >> conn->power_save = 0;
> >> }
> >
> > And again, we are not setting the sniff mode. We disable automatic sniff
> > mode feature.
>
> It really seems the opposite, like we were disabling the automatic
> active mode since this make us to always hit goto timer; while in
> sniff mode. Perhaps this is meant for when remote device has been
> controlling modes, which sound strange to me since hci_mode_change_evt
> is called even when the we actively change the mode, right?
> (hci_conn_idle does that)

I am not following.

Regards

Marcel



2009-08-24 19:26:30

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: sniff mode

Hi Marcel,

On Mon, Aug 24, 2009 at 3:40 PM, Marcel Holtmann<[email protected]> wrote=
:
> Hi Luiz,
>
>> It is just me or hci_conn_enter_active_mode will never do what its
>> name suggests. This check will always succeed:
>>
>> =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto timer;
>
> so conn->power_save is used to disable automatic sniff mode for incoming
> connections. Mainly HID since they manager it by themselves.

Hmm, but this seems to be done for any device regardless of its type.
So for example we start sending/receiving data to a headset it will
triggers hci_conn_enter_active_mode which I thought would exit sniff
mode, right? (yep, I have seem some headset that doesn't leave sniff
mode by their own).

>> But in hci_mode_change_evt whatever mode different than HCI_CM_ACTIVE
>> reset power_save to zero:
>>
>>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!test_and_clear_bit(HCI_CONN_MODE_CHANGE=
_PEND, &conn->pend)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (conn->mode =3D=3D HCI_CM=
_ACTIVE)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->power_=
save =3D 1;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->power_=
save =3D 0;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>
> And again, we are not setting the sniff mode. We disable automatic sniff
> mode feature.

It really seems the opposite, like we were disabling the automatic
active mode since this make us to always hit goto timer; while in
sniff mode. Perhaps this is meant for when remote device has been
controlling modes, which sound strange to me since hci_mode_change_evt
is called even when the we actively change the mode, right?
(hci_conn_idle does that)

--=20
Luiz Augusto von Dentz
Engenheiro de Computa=E7=E3o

2009-08-24 18:40:57

by Marcel Holtmann

[permalink] [raw]
Subject: Re: sniff mode

Hi Luiz,

> It is just me or hci_conn_enter_active_mode will never do what its
> name suggests. This check will always succeed:
>
> if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> goto timer;

so conn->power_save is used to disable automatic sniff mode for incoming
connections. Mainly HID since they manager it by themselves.

> But in hci_mode_change_evt whatever mode different than HCI_CM_ACTIVE
> reset power_save to zero:
>
>
> if (!test_and_clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
> if (conn->mode == HCI_CM_ACTIVE)
> conn->power_save = 1;
> else
> conn->power_save = 0;
> }

And again, we are not setting the sniff mode. We disable automatic sniff
mode feature.

> So it seems that if mode is SNIFF power_save is 0, so this check will
> only fail if mode is in fact HCI_CM_ACTIVE, and then latter on we got
> this:
>
>
> if (!test_and_set_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
> struct hci_cp_exit_sniff_mode cp;
> cp.handle = cpu_to_le16(conn->handle);
> hci_send_cmd(hdev, HCI_OP_EXIT_SNIFF_MODE, sizeof(cp), &cp);
> }
>
> I was expecting that only when mode is HCI_CM_SNIFF it would send
> HCI_OP_EXIT_SNIFF_MODE, or Im missing something?

This reads as if NO mode change is pending, then exit from sniff mode.

Regards

Marcel