You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by "MariuszSkamra (via GitHub)" <gi...@apache.org> on 2024/02/21 14:59:07 UTC

[PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

MariuszSkamra opened a new pull request, #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708

   This adds dedicated event listener for LE Audio events and BASE parser implementation along with unit tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#issuecomment-1964099845

   The CI is failing with false negative result


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505863413


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,297 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+/* Get the next subgroup data pointer */
+static const uint8_t *
+ble_audio_base_subgroup_next(uint8_t num_bis, const uint8_t *data,
+                             uint8_t data_len)
+{
+    uint8_t offset = 0;
+
+    for (uint8_t i = 0; i < num_bis; i++) {
+        uint8_t codec_specific_config_len;
+
+        /* BIS_index[i[k]] + Codec_Specific_Configuration_Length[i[k]] */
+        if ((data_len - offset) < 2) {
+            return NULL;
+        }
+
+        /* Skip BIS_index[i[k]] */
+        offset++;
+
+        codec_specific_config_len = data[offset];
+        offset++;
+
+        if ((data_len - offset) < codec_specific_config_len) {
+            return NULL;
+        }
+
+        offset += codec_specific_config_len;
+    }
+
+    return &data[offset];
+}
+
+int
+ble_audio_base_parse(const uint8_t *data, uint8_t data_len,
+                     struct ble_audio_base_group *group,
+                     struct ble_audio_base_iter *subgroup_iter)
+{
+    uint8_t offset = 0;
+
+    if (data == NULL) {
+        BLE_HS_LOG_ERROR("NULL data!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (group == NULL) {
+        BLE_HS_LOG_ERROR("NULL group!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /* Presentation_Delay + Num_Subgroups */
+    if (data_len < 4) {
+        return BLE_HS_EMSGSIZE;
+    }
+
+    group->presentation_delay = get_le24(data);
+    offset += 3;
+
+    group->num_subgroups = data[offset];
+    offset++;
+
+    if (group->num_subgroups < 1) {
+        BLE_HS_LOG_DEBUG("Rule 1 violation: There shall be at least one subgroup.\n");
+    }
+
+    if (subgroup_iter != NULL) {
+        subgroup_iter->data = &data[offset];
+        subgroup_iter->buf_len = data_len;
+        subgroup_iter->buf = data;
+        subgroup_iter->num_elements = group->num_subgroups;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_base_subgroup_iter(struct ble_audio_base_iter *subgroup_iter,
+                             struct ble_audio_base_subgroup *subgroup,
+                             struct ble_audio_base_iter *bis_iter)
+{
+    const uint8_t *data;
+    uint8_t data_len;
+    ptrdiff_t offset;
+    uint8_t num_subgroups;
+
+    if (subgroup_iter == NULL) {
+        BLE_HS_LOG_ERROR("NULL subgroup_iter!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (subgroup == NULL) {
+        BLE_HS_LOG_ERROR("NULL subgroup!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    data = subgroup_iter->data;
+    if (data == NULL) {
+        return BLE_HS_ENOENT;
+    }
+
+    offset = data - subgroup_iter->buf;
+    if (offset < 0 || offset > subgroup_iter->buf_len) {
+        return BLE_HS_EINVAL;
+    }
+
+    num_subgroups = subgroup_iter->num_elements;
+    if (num_subgroups == 0) {
+        /* All subgroups have been parsed */
+        return BLE_HS_ENOENT;
+    }
+
+    data_len = subgroup_iter->buf_len - offset;
+
+    /* Reset the offset */
+    offset = 0;
+
+    memset(subgroup, 0, sizeof(*subgroup));
+
+    /* Num_BIS + Codec_ID + Codec_Specific_Configuration_Length[i] */
+    if (data_len < 7) {
+        return BLE_HS_EMSGSIZE;
+    }
+
+    subgroup->num_bis = data[offset];
+    offset++;
+
+    if (subgroup->num_bis < 1) {
+        BLE_HS_LOG_DEBUG("Rule 2 violation: There shall be at least one BIS per"

Review Comment:
   same, change DEBUG to ERROR and return BLE_HS_EINVAL



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1504373045


##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"
+
+/** @brief Public Broadcast Announcement features bits */
+enum ble_audio_bcst_public_feature {

Review Comment:
   I'll got with `ble_audio_pub_bcst_announcement`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505867672


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */

Review Comment:
   agree with @KKopyscinski 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505861983


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUBLIC_BROADCAST_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                break;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            break;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+
+        (void)ble_hs_adv_parse(gap_event->ext_disc.data,
+                               gap_event->ext_disc.length_data,
+                               ble_audio_adv_parse_bcst_announcement,
+                               &data);
+
+        if (data.success) {
+            data.event.type = BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT;
+            data.event.bcst_announcement.ext_disc = &gap_event->ext_disc;
+
+            (void)ble_audio_event_listener_call(&data.event);
+        }
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                  ble_audio_event_fn *fn, void *arg)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (fn == NULL) {
+        BLE_HS_LOG_ERROR("NULL fn!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_register(
+                    &ble_audio_gap_event_listener,
+                    ble_audio_gap_event, NULL);
+            if (rc != 0) {
+                return rc;
+            }
+        }
+
+        memset(listener, 0, sizeof(*listener));
+        listener->fn = fn;
+        listener->arg = arg;
+        SLIST_INSERT_HEAD(&ble_audio_event_listener_list, listener, next);
+        rc = 0;
+    } else {
+        rc = BLE_HS_EALREADY;
+    }
+
+    return rc;
+}
+
+int
+ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /*
+     * We check if element exists on the list only for sanity to let caller
+     * know whether it registered its listener before.
+     */
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        rc = BLE_HS_ENOENT;
+    } else {
+        SLIST_REMOVE(&ble_audio_event_listener_list, listener,
+                     ble_audio_event_listener, next);
+
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_unregister(
+                    &ble_audio_gap_event_listener);
+        } else {
+            rc = 0;
+        }
+    }
+
+    return rc;
+}
+int
+ble_audio_event_listener_call(struct ble_audio_event *event)
+{
+    struct ble_audio_event_listener *evl = NULL;
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        evl->fn(event, evl->arg);
+    }
+
+    return 0;
+}
+
+/* Get the next subgroup data pointer */
+static const uint8_t *
+ble_audio_base_subgroup_next(uint8_t num_bis, const uint8_t *data,
+                             uint8_t data_len)
+{
+    uint8_t offset = 0;
+
+    for (uint8_t i = 0; i < num_bis; i++) {
+        uint8_t codec_specific_config_len;
+
+        /* BIS_index[i[k]] + Codec_Specific_Configuration_Length[i[k]] */
+        if ((data_len - offset) < 2) {
+            return NULL;
+        }
+
+        /* Skip BIS_index[i[k]] */
+        offset++;
+
+        codec_specific_config_len = data[offset];
+        offset++;
+
+        if ((data_len - offset) < codec_specific_config_len) {
+            return NULL;
+        }
+
+        offset += codec_specific_config_len;
+    }
+
+    return &data[offset];
+}
+
+int
+ble_audio_base_parse(const uint8_t *data, uint8_t data_len,
+                     struct ble_audio_base_group *group,
+                     struct ble_audio_base_iter *subgroup_iter)
+{
+    uint8_t offset = 0;
+
+    if (data == NULL) {
+        BLE_HS_LOG_ERROR("NULL data!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (group == NULL) {
+        BLE_HS_LOG_ERROR("NULL group!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /* Presentation_Delay + Num_Subgroups */
+    if (data_len < 4) {
+        return BLE_HS_EMSGSIZE;
+    }
+
+    group->presentation_delay = get_le24(data);
+    offset += 3;
+
+    group->num_subgroups = data[offset];
+    offset++;
+
+    if (group->num_subgroups < 1) {
+        BLE_HS_LOG_DEBUG("Rule 1 violation: There shall be at least one subgroup.\n");
+    }

Review Comment:
   I suggest BLE_HS_LOG_ERROR here and return as @KKopyscinski  suggested.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk merged PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505644767


##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+
+#include "host/ble_audio_common.h"
+
+/**
+ * BASE iterator

Review Comment:
   could you add here a comment explaining the common usage of this API ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1504320637


##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"

Review Comment:
   I agree, but I would like this PR to be merged first. Existing code can be moved to `host/audio/` as you suggested in subsequent PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505869762


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -23,6 +23,238 @@
 #include "host/ble_hs.h"
 #include "host/audio/ble_audio.h"
 
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_pub_bcst_announcement pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    data->success = false;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                data->success = false;
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUB_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub_announcement_data != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                /* Stop parsing */
+                data->success = false;
+                return 0;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub_announcement_data = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            /* Stop parsing */
+            data->success = false;
+            return 0;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+        int rc;
+
+        rc = ble_hs_adv_parse(gap_event->ext_disc.data,
+                              gap_event->ext_disc.length_data,
+                              ble_audio_adv_parse_bcst_announcement, &data);
+        if (rc == 0 && data.success) {
+            data.event.type = BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT;
+            data.event.bcst_announcement.ext_disc = &gap_event->ext_disc;
+
+            (void)ble_audio_event_listener_call(&data.event);
+        }
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                  ble_audio_event_fn *fn, void *arg)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (fn == NULL) {
+        BLE_HS_LOG_ERROR("NULL fn!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {

Review Comment:
   maybe to avoid nesting,
   
   if (evl) {
      return BLE_HS_EALREADY;
   }
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505881814


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */

Review Comment:
   I agree as well, and I already changed that



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1504320637


##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"

Review Comment:
   I agree, but I would like this PR to be merged first. Then we can move the existing code to `host/audio/` as you suggested in subsequent PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#issuecomment-1968459087

   Rebased


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "KKopyscinski (via GitHub)" <gi...@apache.org>.
KKopyscinski commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1504098334


##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"

Review Comment:
   I think this file should be merged into this one, not included. So I think contents of `ble_audio_common.h` should be added here and old file deleted. And `ble_audio_broadcast_source.h` should also be moved into this folder (`...include/host/audio/`), and probably renamed to something like `ble_audio_bsct_source` or `ble_audio_bcst_src`. That way we have all audio API in one place.



##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"
+
+/** @brief Public Broadcast Announcement features bits */
+enum ble_audio_bcst_public_feature {

Review Comment:
   maybe `ble_audio_pub_bcst_announce_feat`? and then the rest of functions and macros change to `ble_audio_pub_bcst_announce...`



##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */

Review Comment:
   As far as I can see, if we ever enter here, it means that this field is malformed. What if previous one wasn't, and `data->success` is already set to `true`? I'm not sure we want to report malformed advertising reports, maybe we should set `data->success = false` before we return, here and in later checks?



##########
nimble/host/audio/syscfg.yml:
##########
@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+syscfg.defs:
+
+syscfg.logs:

Review Comment:
   I think we don't need syscfg.logs here. And under defs, I'd move BLE Audio configs from `host/syscfg.yml`: `BLE_MAX_BIG` and `BLE_MAX_BIS`. And maybe ISO related configs too? I mean `BLE_ISO`, `BLE_ISO_BROADCAST_SOURCE` and so on. What do you think @andrzej-kaczmarek ? This way we'll limit bloating of host syscfg



##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"
+
+/** @brief Public Broadcast Announcement features bits */
+enum ble_audio_bcst_public_feature {
+    /** Broadcast Stream Encryption */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_ENCRYPTION = 1 << 0,
+
+    /** Standard Quality Public Broadcast Audio */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_AUDIO_SQ = 1 << 1,
+
+    /** High Quality Public Broadcast Audio */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_AUDIO_HQ = 1 << 2,
+};
+
+/** @brief Public Broadcast Announcement structure */
+struct ble_audio_bcst_public {
+    /** Public Broadcast Announcement features bitfield */
+    enum ble_audio_bcst_public_feature features;
+
+    /** Metadata length */
+    uint8_t metadata_len;
+
+    /** Metadata */
+    const uint8_t *metadata;
+};
+
+struct ble_audio_bcst_name {
+    /** Broadcast Name length */
+    uint8_t name_len;
+
+    /** Broadcast Name */
+    const char *name;
+};
+
+/**
+ * @defgroup ble_audio_events Bluetooth Low Energy Audio Events
+ * @{
+ */
+
+/** BLE Audio event: Broadcast Announcement */
+#define BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT               0
+
+/** @} */
+
+/** @brief Broadcast Announcement */
+struct ble_audio_event_bcst_announcement {
+    /** Extended advertising report */
+    const struct ble_gap_ext_disc_desc *ext_disc;
+
+    /** Broadcast ID */
+    uint32_t broadcast_id;
+
+    /** Additional service data included in Broadcast Audio Announcement */
+    const uint8_t *svc_data;
+
+    /** Additional service data length  */
+    uint16_t svc_data_len;
+
+    /** Optional Public Broadcast Announcement data */
+    struct ble_audio_bcst_public *pub;
+
+    /** Optional Broadcast Name */
+    struct ble_audio_bcst_name *name;
+};
+
+/**
+ * Represents a BLE Audio related event. When such an event occurs, the host
+ * notifies the application by passing an instance of this structure to an
+ * application-specified callback.
+ */
+struct ble_audio_event {
+    /**
+     * Indicates the type of BLE Audio event that occurred. This is one of the
+     * BLE_AUDIO_EVENT codes.
+     */
+    uint8_t type;
+
+    /**
+     * A discriminated union containing additional details concerning the event.
+     * The 'type' field indicates which member of the union is valid.
+     */
+    union {
+        /**
+         * @ref BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT
+         *
+         * Represents a received Broadcast Announcement.
+         */
+        struct ble_audio_event_bcst_announcement bcst_announcement;
+    };
+};
+
+/** Callback function type for handling BLE Audio events. */
+typedef int ble_audio_event_fn(struct ble_audio_event *event, void *arg);
+
+/**
+ * Event listener structure
+ *
+ * This should be used as an opaque structure and not modified manually.
+ */
+struct ble_audio_event_listener {
+    /** The function to call when a BLE Audio event occurs. */
+    ble_audio_event_fn *fn;
+
+    /** An optional argument to pass to the event handler function. */
+    void *arg;
+
+    /** Singly-linked list entry. */
+    SLIST_ENTRY(ble_audio_event_listener) next;
+};
+
+/**
+ * Registers listener for BLE Audio events
+ *
+ * On success listener structure will be initialized automatically and does not
+ * need to be initialized prior to calling this function. To change callback
+ * and/or argument unregister listener first and register it again.
+ *
+ * @param[in] listener          Listener structure
+ * @param[in] event_mask        Optional event mask
+ * @param[in] fn                Callback function
+ * @param[in] arg               Optional callback argument
+ *
+ * @return                      0 on success
+ *                              BLE_HS_EINVAL if no callback is specified
+ *                              BLE_HS_EALREADY if listener is already registered
+ */
+int ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                      ble_audio_event_fn *fn, void *arg);
+
+/**
+ * Unregisters listener for BLE Audio events
+ *
+ * @param[in] listener          Listener structure
+ *
+ * @return                      0 on success
+ *                              BLE_HS_ENOENT if listener was not registered
+ */
+int ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener);
+
+/**
+ * BASE iterator
+ *
+ * This should be used as an opaque structure and not modified manually.
+ */
+struct ble_audio_base_iter {
+    /** Data pointer */
+    const uint8_t *data;
+
+    /** Base length */
+    uint8_t buf_len;
+
+    /** Original BASE pointer */
+    const uint8_t *buf;
+
+    /** Remaining number of elements */
+    uint8_t num_elements;
+};
+
+/** @brief Broadcast Audio Source Endpoint Group structure */
+struct ble_audio_base_group {
+    /** Presentation Delay */
+    uint32_t presentation_delay;
+
+    /** Number of subgroups */
+    uint8_t num_subgroups;
+};
+
+/**
+ * Parse the BASE received from Basic Audio Announcement data.
+ *
+ * @param[in] data              Pointer to the BASE data buffer to parse.
+ * @param[in] data_len          Length of the BASE data buffer.
+ * @param[out] group            Group object.
+ * @param[out] subgroup_iter    Subgroup iterator object.
+ *
+ * @return                      0 on success; nonzero on failure.
+ */
+int ble_audio_base_parse(const uint8_t *data, uint8_t data_len,
+                         struct ble_audio_base_group *group,
+                         struct ble_audio_base_iter *subgroup_iter);
+
+/** @brief Broadcast Audio Source Endpoint Subgroup structure */
+struct ble_audio_base_subgroup {
+    /** Codec information for the subgroup */
+    struct ble_audio_codec_id codec_id;
+
+    /** Length of the Codec Specific Configuration for the subgroup */
+    uint8_t codec_specific_config_len;
+
+    /** Codec Specific Configuration for the subgroup */
+    const uint8_t *codec_specific_config;
+
+    /** Length of the Metadata for the subgroup */
+    uint8_t metadata_len;
+
+    /** Series of LTV structures containing Metadata */
+    const uint8_t *metadata;
+
+    /** Number of BISes in the subgroup */
+    uint8_t num_bis;
+};
+
+/**
+ * @brief Basic Audio Announcement Subgroup information
+ *
+ * @param[in] subgroup_iter     Subgroup iterator object.
+ * @param[out] subgroup         Subgroup object.
+ * @param[out] bis_iter         BIS iterator object.
+ *
+ * @return                      0 on success;
+ *                              A non-zero value on failure.
+ */
+int ble_audio_base_subgroup_iter(struct ble_audio_base_iter *subgroup_iter,
+                                 struct ble_audio_base_subgroup *subgroup,
+                                 struct ble_audio_base_iter *bis_iter);
+
+/** @brief Broadcast Audio Source Endpoint BIS structure */
+struct ble_audio_base_bis {
+    /** BIS_index value for the BIS */
+    uint8_t index;
+
+    /** Length of the Codec Specific Configuration for the BIS */
+    uint8_t codec_specific_config_len;

Review Comment:
   ditto



##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUBLIC_BROADCAST_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                break;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            break;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+
+        (void)ble_hs_adv_parse(gap_event->ext_disc.data,

Review Comment:
   I think we should not discard return code and check it. If parser returned no-zero value, it means most likely that some field reported value beyond packet length (`BLE_HS_EBADDATA`). I think in this case we should drop the report and not send event.



##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUBLIC_BROADCAST_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                break;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            break;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+
+        (void)ble_hs_adv_parse(gap_event->ext_disc.data,
+                               gap_event->ext_disc.length_data,
+                               ble_audio_adv_parse_bcst_announcement,
+                               &data);
+
+        if (data.success) {
+            data.event.type = BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT;
+            data.event.bcst_announcement.ext_disc = &gap_event->ext_disc;
+
+            (void)ble_audio_event_listener_call(&data.event);
+        }
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                  ble_audio_event_fn *fn, void *arg)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (fn == NULL) {
+        BLE_HS_LOG_ERROR("NULL fn!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_register(
+                    &ble_audio_gap_event_listener,
+                    ble_audio_gap_event, NULL);
+            if (rc != 0) {
+                return rc;
+            }
+        }
+
+        memset(listener, 0, sizeof(*listener));
+        listener->fn = fn;
+        listener->arg = arg;
+        SLIST_INSERT_HEAD(&ble_audio_event_listener_list, listener, next);
+        rc = 0;
+    } else {
+        rc = BLE_HS_EALREADY;
+    }
+
+    return rc;
+}
+
+int
+ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /*
+     * We check if element exists on the list only for sanity to let caller
+     * know whether it registered its listener before.
+     */
+

Review Comment:
   ```suggestion
   ```
   probably not needed empty line



##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUBLIC_BROADCAST_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                break;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            break;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+
+        (void)ble_hs_adv_parse(gap_event->ext_disc.data,
+                               gap_event->ext_disc.length_data,
+                               ble_audio_adv_parse_bcst_announcement,
+                               &data);
+
+        if (data.success) {
+            data.event.type = BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT;
+            data.event.bcst_announcement.ext_disc = &gap_event->ext_disc;
+
+            (void)ble_audio_event_listener_call(&data.event);
+        }
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                  ble_audio_event_fn *fn, void *arg)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (fn == NULL) {
+        BLE_HS_LOG_ERROR("NULL fn!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_register(
+                    &ble_audio_gap_event_listener,
+                    ble_audio_gap_event, NULL);
+            if (rc != 0) {
+                return rc;
+            }
+        }
+
+        memset(listener, 0, sizeof(*listener));
+        listener->fn = fn;
+        listener->arg = arg;
+        SLIST_INSERT_HEAD(&ble_audio_event_listener_list, listener, next);
+        rc = 0;
+    } else {
+        rc = BLE_HS_EALREADY;
+    }
+
+    return rc;
+}
+
+int
+ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /*
+     * We check if element exists on the list only for sanity to let caller
+     * know whether it registered its listener before.
+     */
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        rc = BLE_HS_ENOENT;
+    } else {
+        SLIST_REMOVE(&ble_audio_event_listener_list, listener,
+                     ble_audio_event_listener, next);
+
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_unregister(
+                    &ble_audio_gap_event_listener);
+        } else {
+            rc = 0;
+        }
+    }
+
+    return rc;
+}
+int
+ble_audio_event_listener_call(struct ble_audio_event *event)
+{
+    struct ble_audio_event_listener *evl = NULL;
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        evl->fn(event, evl->arg);
+    }
+
+    return 0;
+}
+
+/* Get the next subgroup data pointer */
+static const uint8_t *
+ble_audio_base_subgroup_next(uint8_t num_bis, const uint8_t *data,
+                             uint8_t data_len)
+{
+    uint8_t offset = 0;
+
+    for (uint8_t i = 0; i < num_bis; i++) {
+        uint8_t codec_specific_config_len;
+
+        /* BIS_index[i[k]] + Codec_Specific_Configuration_Length[i[k]] */
+        if ((data_len - offset) < 2) {
+            return NULL;
+        }
+
+        /* Skip BIS_index[i[k]] */
+        offset++;
+
+        codec_specific_config_len = data[offset];
+        offset++;
+
+        if ((data_len - offset) < codec_specific_config_len) {
+            return NULL;
+        }
+
+        offset += codec_specific_config_len;
+    }
+
+    return &data[offset];
+}
+
+int
+ble_audio_base_parse(const uint8_t *data, uint8_t data_len,
+                     struct ble_audio_base_group *group,
+                     struct ble_audio_base_iter *subgroup_iter)
+{
+    uint8_t offset = 0;
+
+    if (data == NULL) {
+        BLE_HS_LOG_ERROR("NULL data!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (group == NULL) {
+        BLE_HS_LOG_ERROR("NULL group!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /* Presentation_Delay + Num_Subgroups */
+    if (data_len < 4) {
+        return BLE_HS_EMSGSIZE;
+    }
+
+    group->presentation_delay = get_le24(data);
+    offset += 3;
+
+    group->num_subgroups = data[offset];
+    offset++;
+
+    if (group->num_subgroups < 1) {
+        BLE_HS_LOG_DEBUG("Rule 1 violation: There shall be at least one subgroup.\n");
+    }

Review 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?
   ```suggestion
       if (group->num_subgroups < 1) {
           BLE_HS_LOG_ERROR("Invalid BASE: no subgroups!\n");
           return BLE_HS_EINVAL;
       }
   ```



##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"
+
+/** @brief Public Broadcast Announcement features bits */
+enum ble_audio_bcst_public_feature {
+    /** Broadcast Stream Encryption */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_ENCRYPTION = 1 << 0,
+
+    /** Standard Quality Public Broadcast Audio */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_AUDIO_SQ = 1 << 1,
+
+    /** High Quality Public Broadcast Audio */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_AUDIO_HQ = 1 << 2,
+};
+
+/** @brief Public Broadcast Announcement structure */
+struct ble_audio_bcst_public {
+    /** Public Broadcast Announcement features bitfield */
+    enum ble_audio_bcst_public_feature features;
+
+    /** Metadata length */
+    uint8_t metadata_len;
+
+    /** Metadata */
+    const uint8_t *metadata;
+};
+
+struct ble_audio_bcst_name {
+    /** Broadcast Name length */
+    uint8_t name_len;
+
+    /** Broadcast Name */
+    const char *name;
+};
+
+/**
+ * @defgroup ble_audio_events Bluetooth Low Energy Audio Events
+ * @{
+ */
+
+/** BLE Audio event: Broadcast Announcement */
+#define BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT               0
+
+/** @} */
+
+/** @brief Broadcast Announcement */
+struct ble_audio_event_bcst_announcement {
+    /** Extended advertising report */
+    const struct ble_gap_ext_disc_desc *ext_disc;
+
+    /** Broadcast ID */
+    uint32_t broadcast_id;
+
+    /** Additional service data included in Broadcast Audio Announcement */
+    const uint8_t *svc_data;
+
+    /** Additional service data length  */
+    uint16_t svc_data_len;
+
+    /** Optional Public Broadcast Announcement data */
+    struct ble_audio_bcst_public *pub;

Review Comment:
   ```suggestion
       struct ble_audio_pub_bcst_announce *pub_announce_data;
   ```
   I think it's not too long and bit more descriptive.



##########
nimble/host/audio/include/host/audio/ble_audio.h:
##########
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_BLE_AUDIO_
+#define H_BLE_AUDIO_
+
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include "host/ble_audio_common.h"
+
+/** @brief Public Broadcast Announcement features bits */
+enum ble_audio_bcst_public_feature {
+    /** Broadcast Stream Encryption */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_ENCRYPTION = 1 << 0,
+
+    /** Standard Quality Public Broadcast Audio */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_AUDIO_SQ = 1 << 1,
+
+    /** High Quality Public Broadcast Audio */
+    BLE_AUDIO_BCST_PUBLIC_FEATURE_AUDIO_HQ = 1 << 2,
+};
+
+/** @brief Public Broadcast Announcement structure */
+struct ble_audio_bcst_public {
+    /** Public Broadcast Announcement features bitfield */
+    enum ble_audio_bcst_public_feature features;
+
+    /** Metadata length */
+    uint8_t metadata_len;
+
+    /** Metadata */
+    const uint8_t *metadata;
+};
+
+struct ble_audio_bcst_name {
+    /** Broadcast Name length */
+    uint8_t name_len;
+
+    /** Broadcast Name */
+    const char *name;
+};
+
+/**
+ * @defgroup ble_audio_events Bluetooth Low Energy Audio Events
+ * @{
+ */
+
+/** BLE Audio event: Broadcast Announcement */
+#define BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT               0
+
+/** @} */
+
+/** @brief Broadcast Announcement */
+struct ble_audio_event_bcst_announcement {
+    /** Extended advertising report */
+    const struct ble_gap_ext_disc_desc *ext_disc;
+
+    /** Broadcast ID */
+    uint32_t broadcast_id;
+
+    /** Additional service data included in Broadcast Audio Announcement */
+    const uint8_t *svc_data;
+
+    /** Additional service data length  */
+    uint16_t svc_data_len;
+
+    /** Optional Public Broadcast Announcement data */
+    struct ble_audio_bcst_public *pub;
+
+    /** Optional Broadcast Name */
+    struct ble_audio_bcst_name *name;
+};
+
+/**
+ * Represents a BLE Audio related event. When such an event occurs, the host
+ * notifies the application by passing an instance of this structure to an
+ * application-specified callback.
+ */
+struct ble_audio_event {
+    /**
+     * Indicates the type of BLE Audio event that occurred. This is one of the
+     * BLE_AUDIO_EVENT codes.
+     */
+    uint8_t type;
+
+    /**
+     * A discriminated union containing additional details concerning the event.
+     * The 'type' field indicates which member of the union is valid.
+     */
+    union {
+        /**
+         * @ref BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT
+         *
+         * Represents a received Broadcast Announcement.
+         */
+        struct ble_audio_event_bcst_announcement bcst_announcement;
+    };
+};
+
+/** Callback function type for handling BLE Audio events. */
+typedef int ble_audio_event_fn(struct ble_audio_event *event, void *arg);
+
+/**
+ * Event listener structure
+ *
+ * This should be used as an opaque structure and not modified manually.
+ */
+struct ble_audio_event_listener {
+    /** The function to call when a BLE Audio event occurs. */
+    ble_audio_event_fn *fn;
+
+    /** An optional argument to pass to the event handler function. */
+    void *arg;
+
+    /** Singly-linked list entry. */
+    SLIST_ENTRY(ble_audio_event_listener) next;
+};
+
+/**
+ * Registers listener for BLE Audio events
+ *
+ * On success listener structure will be initialized automatically and does not
+ * need to be initialized prior to calling this function. To change callback
+ * and/or argument unregister listener first and register it again.
+ *
+ * @param[in] listener          Listener structure
+ * @param[in] event_mask        Optional event mask
+ * @param[in] fn                Callback function
+ * @param[in] arg               Optional callback argument
+ *
+ * @return                      0 on success
+ *                              BLE_HS_EINVAL if no callback is specified
+ *                              BLE_HS_EALREADY if listener is already registered
+ */
+int ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                      ble_audio_event_fn *fn, void *arg);
+
+/**
+ * Unregisters listener for BLE Audio events
+ *
+ * @param[in] listener          Listener structure
+ *
+ * @return                      0 on success
+ *                              BLE_HS_ENOENT if listener was not registered
+ */
+int ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener);
+
+/**
+ * BASE iterator
+ *
+ * This should be used as an opaque structure and not modified manually.
+ */
+struct ble_audio_base_iter {
+    /** Data pointer */
+    const uint8_t *data;
+
+    /** Base length */
+    uint8_t buf_len;
+
+    /** Original BASE pointer */
+    const uint8_t *buf;
+
+    /** Remaining number of elements */
+    uint8_t num_elements;
+};
+
+/** @brief Broadcast Audio Source Endpoint Group structure */
+struct ble_audio_base_group {
+    /** Presentation Delay */
+    uint32_t presentation_delay;
+
+    /** Number of subgroups */
+    uint8_t num_subgroups;
+};
+
+/**
+ * Parse the BASE received from Basic Audio Announcement data.
+ *
+ * @param[in] data              Pointer to the BASE data buffer to parse.
+ * @param[in] data_len          Length of the BASE data buffer.
+ * @param[out] group            Group object.
+ * @param[out] subgroup_iter    Subgroup iterator object.
+ *
+ * @return                      0 on success; nonzero on failure.
+ */
+int ble_audio_base_parse(const uint8_t *data, uint8_t data_len,
+                         struct ble_audio_base_group *group,
+                         struct ble_audio_base_iter *subgroup_iter);
+
+/** @brief Broadcast Audio Source Endpoint Subgroup structure */
+struct ble_audio_base_subgroup {
+    /** Codec information for the subgroup */
+    struct ble_audio_codec_id codec_id;
+
+    /** Length of the Codec Specific Configuration for the subgroup */
+    uint8_t codec_specific_config_len;
+
+    /** Codec Specific Configuration for the subgroup */
+    const uint8_t *codec_specific_config;

Review Comment:
   ```suggestion
       /** Length of the Codec Specific Configuration for the subgroup */
       uint8_t codec_spec_config_len;
   
       /** Codec Specific Configuration for the subgroup */
       const uint8_t *codec_spec_config;
   ```
   maybe worth to make field name shorter, whenever we can



##########
nimble/host/include/host/ble_audio_common.h:
##########
@@ -47,6 +47,7 @@
                                                       __VA_ARGS__)
 
 #define BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID                 0x1852
+#define BLE_BROADCAST_PUBLIC_BROADCAST_ANNOUNCEMENT_SVC_UUID      0x1856

Review Comment:
   Shorter = better?
   ```suggestion
   #define BLE_BROADCAST_PUB_BCST_ANNOUNCEMENT_SVC_UUID      0x1856
   ```
   and probably doxygen comment with full name. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1504349919


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUBLIC_BROADCAST_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                break;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            break;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+
+        (void)ble_hs_adv_parse(gap_event->ext_disc.data,
+                               gap_event->ext_disc.length_data,
+                               ble_audio_adv_parse_bcst_announcement,
+                               &data);
+
+        if (data.success) {
+            data.event.type = BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT;
+            data.event.bcst_announcement.ext_disc = &gap_event->ext_disc;
+
+            (void)ble_audio_event_listener_call(&data.event);
+        }
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                  ble_audio_event_fn *fn, void *arg)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (fn == NULL) {
+        BLE_HS_LOG_ERROR("NULL fn!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_register(
+                    &ble_audio_gap_event_listener,
+                    ble_audio_gap_event, NULL);
+            if (rc != 0) {
+                return rc;
+            }
+        }
+
+        memset(listener, 0, sizeof(*listener));
+        listener->fn = fn;
+        listener->arg = arg;
+        SLIST_INSERT_HEAD(&ble_audio_event_listener_list, listener, next);
+        rc = 0;
+    } else {
+        rc = BLE_HS_EALREADY;
+    }
+
+    return rc;
+}
+
+int
+ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /*
+     * We check if element exists on the list only for sanity to let caller
+     * know whether it registered its listener before.
+     */
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        rc = BLE_HS_ENOENT;
+    } else {
+        SLIST_REMOVE(&ble_audio_event_listener_list, listener,
+                     ble_audio_event_listener, next);
+
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_unregister(
+                    &ble_audio_gap_event_listener);
+        } else {
+            rc = 0;
+        }
+    }
+
+    return rc;
+}
+int
+ble_audio_event_listener_call(struct ble_audio_event *event)
+{
+    struct ble_audio_event_listener *evl = NULL;
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        evl->fn(event, evl->arg);
+    }
+
+    return 0;
+}
+
+/* Get the next subgroup data pointer */
+static const uint8_t *
+ble_audio_base_subgroup_next(uint8_t num_bis, const uint8_t *data,
+                             uint8_t data_len)
+{
+    uint8_t offset = 0;
+
+    for (uint8_t i = 0; i < num_bis; i++) {
+        uint8_t codec_specific_config_len;
+
+        /* BIS_index[i[k]] + Codec_Specific_Configuration_Length[i[k]] */
+        if ((data_len - offset) < 2) {
+            return NULL;
+        }
+
+        /* Skip BIS_index[i[k]] */
+        offset++;
+
+        codec_specific_config_len = data[offset];
+        offset++;
+
+        if ((data_len - offset) < codec_specific_config_len) {
+            return NULL;
+        }
+
+        offset += codec_specific_config_len;
+    }
+
+    return &data[offset];
+}
+
+int
+ble_audio_base_parse(const uint8_t *data, uint8_t data_len,
+                     struct ble_audio_base_group *group,
+                     struct ble_audio_base_iter *subgroup_iter)
+{
+    uint8_t offset = 0;
+
+    if (data == NULL) {
+        BLE_HS_LOG_ERROR("NULL data!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (group == NULL) {
+        BLE_HS_LOG_ERROR("NULL group!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /* Presentation_Delay + Num_Subgroups */
+    if (data_len < 4) {
+        return BLE_HS_EMSGSIZE;
+    }
+
+    group->presentation_delay = get_le24(data);
+    offset += 3;
+
+    group->num_subgroups = data[offset];
+    offset++;
+
+    if (group->num_subgroups < 1) {
+        BLE_HS_LOG_DEBUG("Rule 1 violation: There shall be at least one subgroup.\n");
+    }

Review 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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "MariuszSkamra (via GitHub)" <gi...@apache.org>.
MariuszSkamra commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1504351794


##########
nimble/host/audio/syscfg.yml:
##########
@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+syscfg.defs:
+
+syscfg.logs:

Review Comment:
   Once the code will be moved to `host/syscfg.yml` in subsequent PR, I'll place the configs as you suggested.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505864132


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -0,0 +1,524 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stddef.h>
+
+#include "host/ble_hs.h"
+#include "host/audio/ble_audio.h"
+
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_bcst_public pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUBLIC_BROADCAST_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                break;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            break;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+
+        (void)ble_hs_adv_parse(gap_event->ext_disc.data,
+                               gap_event->ext_disc.length_data,
+                               ble_audio_adv_parse_bcst_announcement,
+                               &data);
+
+        if (data.success) {
+            data.event.type = BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT;
+            data.event.bcst_announcement.ext_disc = &gap_event->ext_disc;
+
+            (void)ble_audio_event_listener_call(&data.event);
+        }
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                  ble_audio_event_fn *fn, void *arg)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (fn == NULL) {
+        BLE_HS_LOG_ERROR("NULL fn!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_register(
+                    &ble_audio_gap_event_listener,
+                    ble_audio_gap_event, NULL);
+            if (rc != 0) {
+                return rc;
+            }
+        }
+
+        memset(listener, 0, sizeof(*listener));
+        listener->fn = fn;
+        listener->arg = arg;
+        SLIST_INSERT_HEAD(&ble_audio_event_listener_list, listener, next);
+        rc = 0;
+    } else {
+        rc = BLE_HS_EALREADY;
+    }
+
+    return rc;
+}
+
+int
+ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /*
+     * We check if element exists on the list only for sanity to let caller
+     * know whether it registered its listener before.
+     */
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        rc = BLE_HS_ENOENT;
+    } else {
+        SLIST_REMOVE(&ble_audio_event_listener_list, listener,
+                     ble_audio_event_listener, next);
+
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_unregister(
+                    &ble_audio_gap_event_listener);
+        } else {
+            rc = 0;
+        }
+    }
+
+    return rc;
+}
+int
+ble_audio_event_listener_call(struct ble_audio_event *event)
+{
+    struct ble_audio_event_listener *evl = NULL;
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        evl->fn(event, evl->arg);
+    }
+
+    return 0;
+}
+
+/* Get the next subgroup data pointer */
+static const uint8_t *
+ble_audio_base_subgroup_next(uint8_t num_bis, const uint8_t *data,
+                             uint8_t data_len)
+{
+    uint8_t offset = 0;
+
+    for (uint8_t i = 0; i < num_bis; i++) {
+        uint8_t codec_specific_config_len;
+
+        /* BIS_index[i[k]] + Codec_Specific_Configuration_Length[i[k]] */
+        if ((data_len - offset) < 2) {
+            return NULL;
+        }
+
+        /* Skip BIS_index[i[k]] */
+        offset++;
+
+        codec_specific_config_len = data[offset];
+        offset++;
+
+        if ((data_len - offset) < codec_specific_config_len) {
+            return NULL;
+        }
+
+        offset += codec_specific_config_len;
+    }
+
+    return &data[offset];
+}
+
+int
+ble_audio_base_parse(const uint8_t *data, uint8_t data_len,
+                     struct ble_audio_base_group *group,
+                     struct ble_audio_base_iter *subgroup_iter)
+{
+    uint8_t offset = 0;
+
+    if (data == NULL) {
+        BLE_HS_LOG_ERROR("NULL data!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (group == NULL) {
+        BLE_HS_LOG_ERROR("NULL group!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /* Presentation_Delay + Num_Subgroups */
+    if (data_len < 4) {
+        return BLE_HS_EMSGSIZE;
+    }
+
+    group->presentation_delay = get_le24(data);
+    offset += 3;
+
+    group->num_subgroups = data[offset];
+    offset++;
+
+    if (group->num_subgroups < 1) {
+        BLE_HS_LOG_DEBUG("Rule 1 violation: There shall be at least one subgroup.\n");
+    }

Review Comment:
   I like @MariuszSkamra  error message better, so just change DEBUG to ERROR here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] nimble/audio: Add LE Audio event listener and BASE parser [mynewt-nimble]

Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on code in PR #1708:
URL: https://github.com/apache/mynewt-nimble/pull/1708#discussion_r1505870441


##########
nimble/host/audio/src/ble_audio.c:
##########
@@ -23,6 +23,238 @@
 #include "host/ble_hs.h"
 #include "host/audio/ble_audio.h"
 
+#include "ble_audio_priv.h"
+
+static struct ble_gap_event_listener ble_audio_gap_event_listener;
+static SLIST_HEAD(, ble_audio_event_listener) ble_audio_event_listener_list =
+    SLIST_HEAD_INITIALIZER(ble_audio_event_listener_list);
+
+struct ble_audio_adv_parse_bcst_announcement_data {
+    struct ble_audio_event event;
+    struct ble_audio_pub_bcst_announcement pub;
+    struct ble_audio_bcst_name name;
+    bool success;
+};
+
+static int
+ble_audio_adv_parse_bcst_announcement(const struct ble_hs_adv_field *field,
+                                      void *user_data)
+{
+    struct ble_audio_adv_parse_bcst_announcement_data *data = user_data;
+    struct ble_audio_event_bcst_announcement *event;
+    const uint8_t value_len = field->length - sizeof(field->length);
+    ble_uuid16_t uuid16 = BLE_UUID16_INIT(0);
+    uint8_t offset = 0;
+
+    event = &data->event.bcst_announcement;
+
+    data->success = false;
+
+    switch (field->type) {
+    case BLE_HS_ADV_TYPE_SVC_DATA_UUID16:
+        if (value_len < 2) {
+            break;
+        }
+
+        uuid16.value = get_le16(&field->value[offset]);
+        offset += 2;
+
+        switch (uuid16.value) {
+        case BLE_BROADCAST_AUDIO_ANNOUNCEMENT_SVC_UUID:
+            if ((value_len - offset) < 3) {
+                /* Stop parsing */
+                data->success = false;
+                return 0;
+            }
+
+            event->broadcast_id = get_le24(&field->value[offset]);
+            offset += 3;
+
+            if (value_len > offset) {
+                event->svc_data = &field->value[offset];
+                event->svc_data_len = value_len - offset;
+            }
+
+            data->success = true;
+            break;
+
+        case BLE_BROADCAST_PUB_ANNOUNCEMENT_SVC_UUID:
+            if (event->pub_announcement_data != NULL) {
+                /* Ignore */
+                break;
+            }
+
+            if ((value_len - offset) < 2) {
+                /* Stop parsing */
+                data->success = false;
+                return 0;
+            }
+
+            data->pub.features = field->value[offset++];
+            data->pub.metadata_len = field->value[offset++];
+
+            if ((value_len - offset) < data->pub.metadata_len) {
+                break;
+            }
+
+            data->pub.metadata = &field->value[offset];
+
+            event->pub_announcement_data = &data->pub;
+            break;
+
+        default:
+            break;
+        }
+
+        break;
+
+    case BLE_HS_ADV_TYPE_BROADCAST_NAME:
+        if (event->name != NULL) {
+            /* Ignore */
+            break;
+        }
+
+        if (value_len < 4 || value_len > 32) {
+            /* Stop parsing */
+            data->success = false;
+            return 0;
+        }
+
+        data->name.name = (char *)field->value;
+        data->name.name_len = value_len;
+
+        event->name = &data->name;
+        break;
+
+    default:
+        break;
+    }
+
+    /* Continue parsing */
+    return BLE_HS_ENOENT;
+}
+
+static int
+ble_audio_gap_event(struct ble_gap_event *gap_event, void *arg)
+{
+    switch (gap_event->type) {
+    case BLE_GAP_EVENT_EXT_DISC: {
+        struct ble_audio_adv_parse_bcst_announcement_data data = { 0 };
+        int rc;
+
+        rc = ble_hs_adv_parse(gap_event->ext_disc.data,
+                              gap_event->ext_disc.length_data,
+                              ble_audio_adv_parse_bcst_announcement, &data);
+        if (rc == 0 && data.success) {
+            data.event.type = BLE_AUDIO_EVENT_BCST_ANNOUNCEMENT;
+            data.event.bcst_announcement.ext_disc = &gap_event->ext_disc;
+
+            (void)ble_audio_event_listener_call(&data.event);
+        }
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+int
+ble_audio_event_listener_register(struct ble_audio_event_listener *listener,
+                                  ble_audio_event_fn *fn, void *arg)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    if (fn == NULL) {
+        BLE_HS_LOG_ERROR("NULL fn!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        if (SLIST_EMPTY(&ble_audio_event_listener_list)) {
+            rc = ble_gap_event_listener_register(
+                    &ble_audio_gap_event_listener,
+                    ble_audio_gap_event, NULL);
+            if (rc != 0) {
+                return rc;
+            }
+        }
+
+        memset(listener, 0, sizeof(*listener));
+        listener->fn = fn;
+        listener->arg = arg;
+        SLIST_INSERT_HEAD(&ble_audio_event_listener_list, listener, next);
+        rc = 0;
+    } else {
+        rc = BLE_HS_EALREADY;
+    }
+
+    return rc;
+}
+
+int
+ble_audio_event_listener_unregister(struct ble_audio_event_listener *listener)
+{
+    struct ble_audio_event_listener *evl = NULL;
+    int rc;
+
+    if (listener == NULL) {
+        BLE_HS_LOG_ERROR("NULL listener!\n");
+        return BLE_HS_EINVAL;
+    }
+
+    /* We check if element exists on the list only for sanity to let caller
+     * know whether it registered its listener before.
+     */
+    SLIST_FOREACH(evl, &ble_audio_event_listener_list, next) {
+        if (evl == listener) {
+            break;
+        }
+    }
+
+    if (!evl) {
+        rc = BLE_HS_ENOENT;

Review Comment:
   maybe just `return BLE_HS_ENOENT;`
   
   to avoid nesting.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org