Kconfig default on BT_ACL_RX_COUNT can be 1, but range is 2-64

[ulfalizer]

The BT_ACL_RX_COUNT Kconfig symbol has range 2 64 and default BT_CTLR_RX_BUFFERS if BT_CTLR.

BT_CTLR_RX_BUFFERS defaults to 1 for tests/bluetooth/init/prj_controller.conf, triggering a warning about the default lying outside the range (which is turned into an error).

I'm not sure what the proper fix is here. I'll whitelist the warning for now (in scripts/kconfig/kconfig.py).

[ulfalizer]

CC @carlescufi @jhedberg

[jhedberg]

I think the proper fix is to set the minimum to 1, however some basic pairing & GATT tests should be done before merging such a patch. with a config that results in a single buffer.

[carlescufi]

Does not work with a single buffer.
We must set the range of BT_CTLR_RX_BUFFERS to 2..N

[cvinayak]

@carlescufi controller will operate with 1 buffer, and makes sense to allow that, as LESC is a one time thing in most products life. We can save on a controller buffer.
I will remove the line
default BT_CTLR_RX_BUFFERS if BT_CTLR

[ulfalizer]

@cvinayak
@carlescufi did some testing on IRC where it seemed to break (he's away now though):

< carlesc> jhe: with one packet, [bt] [ERR] smp_timeout: SMP Timeout
< carlesc> jhe: with 2 packets it works, but I get [bt] [ERR] bt_id_add: Failed to add IRK to controller
< carlesc> anyway one packet is not enough
< jhe> hmm, ok
< jhe> so the right fix might be to increase the controller minimum
< carlesc> yep
< carlesc> will close the issue with this

[carlescufi]

@ulfalizer removing the line:

default BT_CTLR_RX_BUFFERS if BT_CTLR

will allow the range of BT_ACL_RX_COUNT to be 2..n and should therefore be a good fix.

[jhedberg]

With a combined build it makes little sense to have the host out of sync with the controller. So I don't think it's a good idea to remove default BT_CTLR_RX_BUFFERS if BT_CTLR. @carlescufi when you hit the issue with LE SC I suppose you had to force 1 buffer in your configuration? I.e. it didn't come from any default? In that case maybe it's ok to reduce the host minimum to 1 after all and assume that whoever changes the defaults knows what they're doing. The other option would be to have two ranges, with the one starting with 2 depending on LE SC. However this second option feels a bit less clean to me, since there could be other condidtions besides LE SC that'll require more than one buffer.

[carlescufi]

when you hit the issue with LE SC I suppose you had to force 1 buffer in your configuration?

yes

maybe it's ok to reduce the host minimum to 1 after all and assume that whoever changes the defaults knows what they're doin

Agreed, with a comment. I will do that in a PR.

[carlescufi]

that whoever changes the defaults knows what they're doing

The problem is that if one enablesBT_HCI_ACL_FLOW_CONTROL in a combined build, then the default BT_ACL_RX_COUNT is 1 because the default BT_CTLR_RX_BUFFERS is 1.

@jhedberg maybe we shouldn't allow BT_HCI_ACL_FLOW_CONTROL in combined builds then?