bluetooth: id: fix adv sets with same id use different RPA #68892
Conversation
zephyrbot
added
area: Bluetooth Host
zephyrbot
requested review from
alwa-nordic,
hermabe,
jhedberg,
jori-nordic,
sjanc,
Thalley and
theob-pro
niym-ot
force-pushed
the
Issue_Id_64625_rpa_diff_for_adv_sets_with_same_id
branch
from
7d3726f to
0c723a7
Compare
|
@niym-ot modified your PR text to use the proper way of linking PRs with issues (see https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests) |
|
Could you add a bsim test for this functionality? |
niym-ot
force-pushed
the
Issue_Id_64625_rpa_diff_for_adv_sets_with_same_id
branch
from
0c723a7 to
a6ee98b
Compare
|
Can we add a new test instead of modifying an old one? I think it's much easier to navigate single-purpose tests with a clearly stated test purpose. |
niym-ot
force-pushed
the
Issue_Id_64625_rpa_diff_for_adv_sets_with_same_id
branch
from
a6ee98b to
d1fc0aa
Compare
niym-ot
force-pushed
the
Issue_Id_64625_rpa_diff_for_adv_sets_with_same_id
branch
2 times, most recently
from
2d14654 to
6a52b14
Compare
| @@ -56,15 +56,16 @@ static void validate_rpa_addr_generated_for_adv_sets(void) | |||
| FAIL("RPA not same for adv sets with same id and RPA sharing enabled\n"); | |||
| } | |||
| } | |||
| if (bt_addr_le_eq(&adv_set_data[0].old_addr, &adv_set_data[3].old_addr)) { | |||
| if (bt_addr_le_eq(&adv_set_data[0].old_addr, &adv_set_data[2].old_addr)) { | |||
There was a problem hiding this comment.
Please move the refactors of other tests to a separate PR. You can tag it trivial.
You can keep the rename in this one.
| create_adv(&adv_set[i], ID_2, true); | ||
| break; | ||
| case ADV_SET_INDEX_4: | ||
| create_adv(&adv_set[i], ID_2, false); |
There was a problem hiding this comment.
Could you post a follow-up PR where you add another id w/ two other adv sets, that return false for the first one and true for the second one? Ie started in reverse order so the iterator hits the false first.
There was a problem hiding this comment.
clarify: this comment is not a blocker.
There was a problem hiding this comment.
Noted. Will create a pull request for the same.
niym-ot
force-pushed
the
Issue_Id_64625_rpa_diff_for_adv_sets_with_same_id
branch
2 times, most recently
from
7d3092f to
d96a160
Compare
| @@ -110,11 +111,10 @@ void start_advertising(void) | |||
|
|
|||
| for (int i = 0; i < CONFIG_BT_EXT_ADV_MAX_ADV_SET; i++) { | |||
|
|
|||
| if (i != ADV_SET_INDEX_THREE) { | |||
| /* Create advertising set 1 and 2 with same id */ | |||
| /* Create first 2 advertising sets with one id and remaining with another id */ | |||
There was a problem hiding this comment.
please revert changes to this test
The fix is to check if any of the adv set's rpa expired callback returns false, then all adv sets continues with the old RPA. Note: Fix is applicable only to adv sets which shares the same id. Signed-off-by: Nithin Ramesh Myliattil <[email protected]>
niym-ot
force-pushed
the
Issue_Id_64625_rpa_diff_for_adv_sets_with_same_id
branch
from
d96a160 to
d6a000b
Compare
The fix is to check if any of the adv set's rpa expired callback returns false, then all adv sets continues with the old RPA.
Note: Fix is applicable only to adv sets which shares the same id.
fixes #64625