Bluetooth: Audio: Ignore RFU VOCS location #51037
Conversation
szymon-czapracki
requested review from
Thalley,
Vudentz,
jhedberg and
sjanc
as code owners
zephyrbot
requested review from
Casper-Bonde-Bose,
MariuszSkamra and
asbjornsabo
szymon-czapracki
force-pushed
the
vocs_location_fix
branch
from
7ba8d03 to
c2e3732
Compare
szymon-czapracki
requested review from
MariuszSkamra
and removed request for
Casper-Bonde-Bose,
Thalley,
Vudentz,
asbjornsabo,
jhedberg and
sjanc
MariuszSkamra
left a comment
MariuszSkamra
left a comment
There was a problem hiding this comment.
LGTM, however would be nice to avoid magic numbers ;)
subsys/bluetooth/audio/vocs.c
Outdated
There was a problem hiding this comment.
You don't need this one I guess
MariuszSkamra
requested review from
Casper-Bonde-Bose,
Thalley,
Vudentz,
asbjornsabo,
jhedberg and
sjanc
szymon-czapracki
force-pushed
the
vocs_location_fix
branch
from
c2e3732 to
9a1c7a9
Compare
Thalley
left a comment
Thalley
left a comment
There was a problem hiding this comment.
Some comments; not necessarily anything that needs changing, but responses on them would be nice
szymon-czapracki
force-pushed
the
vocs_location_fix
branch
from
9a1c7a9 to
a5bce5e
Compare
szymon-czapracki
requested review from
Thalley
and removed request for
Vudentz
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
zephyrbot
requested review from
Casper-Bonde-Bose,
asbjornsabo,
fredrikdanebjer,
jhedberg and
sjanc
szymon-czapracki
force-pushed
the
vocs_location_fix
branch
from
3715214 to
bab73d4
Compare
szymon-czapracki
force-pushed
the
vocs_location_fix
branch
from
bab73d4 to
ada3fb7
Compare
subsys/bluetooth/audio/vocs.c
Outdated
There was a problem hiding this comment.
If the RFU locations is really what we want, we should have that as a define as well (or instead).
There was a problem hiding this comment.
@Thalley nah, everything that is not defined in enum bt_audio_location can be considered as RFU, thus I'd not define RFU values explicitly
There was a problem hiding this comment.
I'm not suggesting that we in audio.h defined the RFU values, but IMO it makes more sense to use a #define for the RFU values instead of a local variable.
There was a problem hiding this comment.
@Thalley I understand now, makes sense indeed :)
There was a problem hiding this comment.
@MariuszSkamra @Thalley I'm not sure if I'm defining this macro now correctly. Is this the proper way?
szymon-czapracki
force-pushed
the
vocs_location_fix
branch
2 times, most recently
from
0b8caba to
ccbc09e
Compare
This commit changes VOCS set location behavior. When an RFU location is written, VOCS ignores it. Signed-off-by: Szymon Czapracki <[email protected]>
szymon-czapracki
force-pushed
the
vocs_location_fix
branch
from
ccbc09e to
9939aeb
Compare
|
|
||
| #define VALID_VOCS_OPCODE(opcode) ((opcode) == BT_VOCS_OPCODE_SET_OFFSET) | ||
|
|
||
| #define BT_AUDIO_LOCATION_RFU (~BT_AUDIO_LOCATION_ANY) |
There was a problem hiding this comment.
Minor: We should probably avoid using the BT prefix for macros that only exist in specifics source files
| #define BT_AUDIO_LOCATION_RFU (~BT_AUDIO_LOCATION_ANY) | |
| #define AUDIO_LOCATION_RFU (~BT_AUDIO_LOCATION_ANY) |
The alternative is to move this macro to audio_internal; quite possibly it's something we may use for other modules as well. Either way works :)
5 participants
This changes VOCS set location behavior.
When an RFU location is written, VOCS ignores it.
This issue was found while executing VOCS/SR/SPE/BI-01-C test case.