Bluetooth: Mesh: Op agg mdl coex #64088
Conversation
Andrewpini
commented
Oct 18, 2023
•
Andrewpini
commented
- Refactors the implementation of the Opcode Aggregator models to allow them to coexist on the same device.
- Adds coexistence Bsim test for Opcode Aggregator models. The test verifies that the Opcode Aggregator server and client can be present and functional when operating on the same device.
- Adds coexistence Bsim test for Opcode Aggregator models using loopback. The test verifies that the Opcode Aggregator server and client can send messages to each other on the loopback interface.
Andrewpini
requested review from
jhedberg,
Vudentz,
wopu-ot,
PavelVPV and
LingaoM
as code owners
| @@ -1506,8 +1506,10 @@ int bt_mesh_model_send(struct bt_mesh_model *model, struct bt_mesh_msg_ctx *ctx, | |||
| struct net_buf_simple *msg, | |||
| const struct bt_mesh_send_cb *cb, void *cb_data) | |||
| { | |||
| if (IS_ENABLED(CONFIG_BT_MESH_OP_AGG) && bt_mesh_op_agg_accept(ctx)) { | |||
| return bt_mesh_op_agg_send(model, ctx, msg, cb); | |||
| if (IS_ENABLED(CONFIG_BT_MESH_OP_AGG_SRV) && bt_mesh_op_agg_srv_accept(ctx, msg)) { | |||
There was a problem hiding this comment.
Why does server checking go first?
There was a problem hiding this comment.
It just made most sense to me. The aggregation process for the server is fully dependent on internal behavoir and should under normal circumstances complete in a very short time. The client aggregation however is completly dependent on how the user utilize the API. I wanted to avoid a situation where starting a client sequence on the device would block any incoming sequences to the server, since doing it the other way around would put the status responses in the ongoing aggregated sequence buffer rather than in the response buffer. Ideally (as discussed outside this PR) we should have a way to tag access messages to know if the message belongs to a agg sequence or a agg response (or non of them). Then we could remove the else if and make the decision based on the tag. But this would require major rework and potentially change API, so I think this is the best solution for now if we don't want to spend much more time fixing this properly.
There was a problem hiding this comment.
Ideally (as discussed outside this PR) we should have a way to tag access messages to know if the message belongs to a agg sequence or a agg response (or non of them).
I need to look deeply but first thing that comes to me is to add tag field into bt_mesh_msg_ctx. I think we also need to look at this in the context of response delay randomization. Currently we don't have any API for sending a response message. If we introduce it, it could also be used to solve this issue.
There was a problem hiding this comment.
Should we create a separate task for this?
Andrewpini
force-pushed
the
NCSDK-23391_OpAgg_cli_and_srv_on_same_dev
branch
from
e9a8864
to
f243f21
Compare
| @@ -1506,8 +1506,10 @@ int bt_mesh_model_send(struct bt_mesh_model *model, struct bt_mesh_msg_ctx *ctx, | |||
| struct net_buf_simple *msg, | |||
| const struct bt_mesh_send_cb *cb, void *cb_data) | |||
| { | |||
| if (IS_ENABLED(CONFIG_BT_MESH_OP_AGG) && bt_mesh_op_agg_accept(ctx)) { | |||
| return bt_mesh_op_agg_send(model, ctx, msg, cb); | |||
| if (IS_ENABLED(CONFIG_BT_MESH_OP_AGG_SRV) && bt_mesh_op_agg_srv_accept(ctx, msg)) { | |||
There was a problem hiding this comment.
Ideally (as discussed outside this PR) we should have a way to tag access messages to know if the message belongs to a agg sequence or a agg response (or non of them).
I need to look deeply but first thing that comes to me is to add tag field into bt_mesh_msg_ctx. I think we also need to look at this in the context of response delay randomization. Currently we don't have any API for sending a response message. If we introduce it, it could also be used to solve this issue.
Refactors the implementation of the Opcode Aggregator models to allow them to coexist on the same device. Signed-off-by: Anders Storrø <[email protected]>
Adds coexistence Bsim test for Opcode Aggregator models. The test verifies that the Opcode Aggregator server and client can be present and functional when operating on the same device. Signed-off-by: Anders Storrø <[email protected]>
Adds coexistence Bsim test for Opcode Aggregator models using loopback. The test verifies that the Opcode Aggregator server and client can send messages to each other on the loopback interface. Signed-off-by: Anders Storrø <[email protected]>
Andrewpini
force-pushed
the
NCSDK-23391_OpAgg_cli_and_srv_on_same_dev
branch
from
f243f21
to
9a020c4
Compare
