Bluetooth: CSIP: Add support for dynamically setting set size #86826
Conversation
Thalley
force-pushed
the
csip_dyn_set_size
branch
4 times, most recently
from
a46bcc8 to
2e25620
Compare
Thalley
force-pushed
the
csip_dyn_set_size
branch
from
2e25620 to
9df9c97
Compare
|
#AutoPTS run zephyr nrf53 CSIS/SR/SGGIT/CHA/BV-02-C |
|
Scheduled PR #86826 (comment), board: nrf53, estimated start time: 15:34:56, test case count: 1, estimated duration: 0:10:34 Test cases to be runCSIS/SR/SGGIT/CHA/BV-02-C |
zephyrbot
added
Release Notes
zephyrbot
requested review from
aescolar,
asbjornsabo,
babrsn,
Casper-Bonde-Bose,
fabiobaltieri,
fredrikdanebjer,
jhedberg,
jthm-ot,
kartben,
larsgk,
MariuszSkamra,
niym-ot,
pin-zephyr and
sjanc
|
AutoPTS Bot results: Successful tests (1)CSIS CSIS/SR/SGGIT/CHA/BV-02-C PASS |
Thalley
force-pushed
the
csip_dyn_set_size
branch
3 times, most recently
from
5e3045a to
59a228f
Compare
Thalley
force-pushed
the
csip_dyn_set_size
branch
from
59a228f to
e3c45d5
Compare
Thalley
force-pushed
the
csip_dyn_set_size
branch
from
e3c45d5 to
f7aaddd
Compare
| @@ -237,6 +238,70 @@ int bt_csip_set_member_sirk(struct bt_csip_set_member_svc_inst *svc_inst, | |||
| int bt_csip_set_member_get_sirk(struct bt_csip_set_member_svc_inst *svc_inst, | |||
There was a problem hiding this comment.
The new _get_info() gets the SIRK (and other info). Is the _get_sirk() then needed anymore? Should it be deprecated and removed?
There was a problem hiding this comment.
#86991 exists for that :)
Thalley
force-pushed
the
csip_dyn_set_size
branch
from
f7aaddd to
2faf535
Compare
There was a problem hiding this comment.
I have some general questions about this.
Isn't it strange that the rank is not notifiable when we can change it? I know that the spec. says that it cannot be notifiable - but maybe that should be changed? Is there a behaviour requirement that the service has to be rediscovered when the set size change has been notified?
The other thing is: The set member can now self-change the set size, but there is no guarantee that all set members will successfully update its set size. I did see that you made it very clear in the api description that it's the callers responsibility to ensure that all set members update it - and that is fair enough. However, I think its quite likely that there will be real-life situations where they aren't synced, due to bugs and whatnot. If this happens, can catastrophes happen as a side-effect?
| return -ENOEXEC; | ||
| } | ||
|
|
||
| rank = shell_strtoul(argv[1], 0, &err); |
There was a problem hiding this comment.
Shouldn't argv[1] be argv[2]? You are already using argv[1] for size
Thalley
force-pushed
the
csip_dyn_set_size
branch
from
2faf535 to
327a895
Compare
It is strange IMO, and I'm planning on filing an erratum for this.
That is correct :) However, the same can be said about the calls to
That really depend on the set coordinators. If a set coordinator connect to 3 devices and 2 of them share the same rank, it's hard to tell what will happen. Ditto if 2 devices claim that the set has 3 device, and the last only claims that there's 2. |
The set size can now be dynamically set and notified. The rank is added as a argument in the case that changing the set size, also affects the device's rank, as ranks in a coordinated set needs to be continuous. The set coordinator implementation has been updated to support receiving the new set size, and providing this information to the upper layers. This commit adds a babblesim test for the new API, as well as a shell command. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Thalley
force-pushed
the
csip_dyn_set_size
branch
from
327a895 to
c447dae
Compare
The set size can now be dynamically set and notified. The rank is added as a argument in the case that changing the set size, also affects the device's rank, as ranks in a coordinated set needs to be continuous.
fixes #57470