nimble/audio: Add LE Audio event listener and BASE parser #1708
Conversation
MariuszSkamra
force-pushed
the
audio_base_parser
branch
4 times, most recently
from
b0e7d6e
to
8cb3124
Compare
MariuszSkamra
force-pushed
the
audio_base_parser
branch
from
8cb3124
to
f810b0f
Compare
|
The CI is failing with false negative result |
| uint16_t svc_data_len; | ||
|
|
||
| /** Optional Public Broadcast Announcement data */ | ||
| struct ble_audio_bcst_public *pub; |
There was a problem hiding this comment.
| struct ble_audio_bcst_public *pub; | |
| struct ble_audio_pub_bcst_announce *pub_announce_data; |
I think it's not too long and bit more descriptive.
nimble/host/audio/src/ble_audio.c
Outdated
| if (group->num_subgroups < 1) { | ||
| BLE_HS_LOG_DEBUG("Rule 1 violation: There shall be at least one subgroup.\n"); | ||
| } |
There was a problem hiding this comment.
We know the rules, but it imho looks a bit weird :) And don't we want to return early here, like in other errors? I think BASE with no subgroups as just as bad as empty one - both don't follow spec. Maybe something like this?
| if (group->num_subgroups < 1) { | |
| BLE_HS_LOG_DEBUG("Rule 1 violation: There shall be at least one subgroup.\n"); | |
| } | |
| if (group->num_subgroups < 1) { | |
| BLE_HS_LOG_ERROR("Invalid BASE: no subgroups!\n"); | |
| return BLE_HS_EINVAL; | |
| } |
There was a problem hiding this comment.
Yeah, I was considering that, probably you're right.
I was thinking about this as an Advertising data that display to the user - when you parse e.g. Complete Name, Service Data and Appearance. When you fail to parse Service data I think you still want to report the Complete Name and Appearance.
Let's wait for 3rd eye pair to look at this.
There was a problem hiding this comment.
I suggest BLE_HS_LOG_ERROR here and return as @KKopyscinski suggested.
There was a problem hiding this comment.
I like @MariuszSkamra error message better, so just change DEBUG to ERROR here.
MariuszSkamra
force-pushed
the
audio_base_parser
branch
from
f810b0f
to
49e25fe
Compare
MariuszSkamra
force-pushed
the
audio_base_parser
branch
2 times, most recently
from
d551ad2
to
82ad759
Compare
|
Rebased |
MariuszSkamra
force-pushed
the
audio_base_parser
branch
from
82ad759
to
14eed27
Compare
nimble/host/audio/src/ble_audio.c
Outdated
| offset++; | ||
|
|
||
| if (subgroup->num_bis < 1) { | ||
| BLE_HS_LOG_DEBUG("Rule 2 violation: There shall be at least one BIS per" |
There was a problem hiding this comment.
same, change DEBUG to ERROR and return BLE_HS_EINVAL
nimble/host/audio/src/ble_audio.c
Outdated
| } | ||
| } | ||
|
|
||
| if (!evl) { |
There was a problem hiding this comment.
maybe to avoid nesting,
if (evl) {
return BLE_HS_EALREADY;
}
nimble/host/audio/src/ble_audio.c
Outdated
| } | ||
|
|
||
| if (!evl) { | ||
| rc = BLE_HS_ENOENT; |
There was a problem hiding this comment.
maybe just return BLE_HS_ENOENT;
to avoid nesting.
This adds BASE parser implementation along with unit tests.
This adds dedicated event listener for LE Audio events.
MariuszSkamra
force-pushed
the
audio_base_parser
branch
from
14eed27
to
3bf3835
Compare
This adds dedicated event listener for LE Audio events and BASE parser implementation along with unit tests.