[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257584462
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -708,6 +711,47 @@ struct ble_gap_event {
 uint8_t tx_phy;
 uint8_t rx_phy;
 } phy_updated;
+#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+/**
+ * Represents an periodic advertising sync established during discovery
+ * procedure.  Valid for the following event types:
+ * o BLE_GAP_EVENT_PERIODIC_ADV_SYNC_ESTAB
+ */
+struct {
+uint8_t status;
+uint16_t sync_handle;
+uint8_t sid;
+uint8_t adv_addr_type;
+uint8_t adv_addr[6];
+uint8_t adv_phy;
+uint16_t per_adv_ival;
+uint8_t adv_clk_accuracy;
+} per_adv_sync_estab;
+
 
 Review comment:
   please remove empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257625928
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -28,8 +28,6 @@
 #include "ble_hs_priv.h"
 #include "ble_hs_dbg_priv.h"
 
-_Static_assert(sizeof (struct hci_data_hdr) == BLE_HCI_DATA_HDR_SZ,
 
 Review comment:
   Why this is removed? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257595528
 
 

 ##
 File path: nimble/host/mesh/src/prov.c
 ##
 @@ -449,7 +451,7 @@ static int prov_send_adv(struct os_mbuf *msg)
 #if (MYNEWT_VAL(BLE_MESH_PB_GATT))
 static int prov_send_gatt(struct os_mbuf *msg)
 {
-   if (!link.conn_handle) {
+   if (link.conn_handle == BLE_HS_CONN_HANDLE_NONE) {
 
 Review comment:
   Looks like this branch is not rebased. Please rebase as there should not be 
any changes in mesh code in this PR.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257586646
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -708,6 +711,47 @@ struct ble_gap_event {
 uint8_t tx_phy;
 uint8_t rx_phy;
 } phy_updated;
+#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+/**
+ * Represents an periodic advertising sync established during discovery
+ * procedure.  Valid for the following event types:
+ * o BLE_GAP_EVENT_PERIODIC_ADV_SYNC_ESTAB
+ */
+struct {
+uint8_t status;
+uint16_t sync_handle;
+uint8_t sid;
+uint8_t adv_addr_type;
+uint8_t adv_addr[6];
+uint8_t adv_phy;
+uint16_t per_adv_ival;
+uint8_t adv_clk_accuracy;
+} per_adv_sync_estab;
+
+
+/**
+ * Represents a Periodic advertising report received during discovery
+ * procedure.  Valid for the following event types:
+ * o BLE_GAP_EVENT_PERIODIC_DISC
+ */
+struct {
+uint16_t sync_handle;
+uint8_t tx_power;
+int8_t rssi;
+uint8_t _unused;
 
 Review comment:
   Let's removed unused from the API for now as in Bluetooth 5.0 it is always 
0xff and has no meaning for the application. When it will be defined in future 
specification it will change name for sure and then we decide how to add it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257622012
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_ENABLE);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_enable(1, instance, buf, 
sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_op = BLE_GAP_OP_S_PERIODIC_ADV;
+
+ble_hs_unlock();
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
+struct os_mbuf **data)
+{
+
+/* In that case we always fit all data in single HCI command */
+#if MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE) <= BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN
+
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE)];
+uint16_t len = OS_MBUF_PKTLEN(*data);
+int rc;
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, opcode);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_data(
+instance,
+BLE_HCI_LE_SET_DATA_OPER_COMPLETE,
+*data, len,
+buf, sizeof(buf));
+
+if (rc) {
+return rc;
+}
+
+os_mbuf_adj(*data, MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE));
+*data = os_mbuf_trim_front(*data);
+
+return ble_hs_hci_cmd_tx_empty_ack(opcode, buf,
+BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN + len);
+#else
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN];

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257590465
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -913,6 +966,28 @@ int ble_gap_ext_adv_stop(uint8_t instance);
 int ble_gap_ext_adv_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_rsp_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_remove(uint8_t instance);
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+/* Periodic Advertising */
+int ble_gap_periodic_adv_configure(uint8_t instance, const struct 
ble_gap_periodic_adv_params *params);
+int ble_gap_periodic_adv_start(uint8_t instance);
+int ble_gap_periodic_adv_stop(uint8_t instance);
+int ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data);
+int ble_gap_periodic_adv_create_sync (uint8_t filter_policy,
+ uint8_tadv_sid, uint8_t adv_add_type,
+ const uint8_t *adv_addr, uint16_t skip,
+ uint16_t sync_timeout, uint8_t unused);
 
 Review comment:
   let's don't expose `unsed`  it in the yet and set this in 0x00 in HCI 
command as per BT 5.0 specification.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257635790
 
 

 ##
 File path: porting/nimble/include/syscfg/syscfg.h
 ##
 @@ -389,6 +389,10 @@
 #define MYNEWT_VAL_BLE_MAX_CONNECTIONS (1)
 #endif
 
+#ifndef MYNEWT_VAL_BLE_MAX_PERIODIC_SCANNING_SM
+#define MYNEWT_VAL_BLE_MAX_PERIODIC_SCANNING_SM (1)
 
 Review comment:
   Please set it to 0


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257596832
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -128,6 +129,13 @@ struct ble_gap_master_state {
 uint8_t limited:1;
 } disc;
 };
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+/* Counts how many times create_sync were called without
+ * receiving sync established event
+ */
+uint16_tpending_create_sync;
 
 Review comment:
   comment does not match how `pending_create_sync` is used as this seems to be 
a flag only. Please also change type of it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257623756
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_cmd.c
 ##
 @@ -1605,6 +1605,167 @@ ble_hs_hci_cmd_build_le_ext_adv_remove(uint8_t handle,
 
 return 0;
 }
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+int
+ble_hs_hci_cmd_build_le_periodic_adv_params(uint8_t handle,
+   const struct hci_periodic_adv_params 
*params,
+   uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN);
+
+if (handle > 0xEF){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (params->max_interval > 0x || params->max_interval < 6){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (params->min_interval > 0x || params->min_interval < 6){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = handle;
+
+put_le16([1], params->min_interval);
+put_le16([3], params->max_interval);
+put_le16([5], params->properties);
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_enable(uint8_t enable,
+   uint8_t handle,
+   uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN);
+
+if (enable > 1){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = enable;
+cmd[1] = handle;
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_data(uint8_t handle,
+ uint8_t operation,
+ struct os_mbuf *data,
+ uint8_t data_len,
+ uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= 3 + data_len);
+
+if (!data){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (!data->om_data){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (handle > 0xEF){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = handle;
+cmd[1] = operation;
+cmd[2] = data_len;
+os_mbuf_copydata(data, 0, data_len, cmd + 3);
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_create_sync(uint8_t filter_policy,
+uint8_t adv_sid,
+uint8_t adv_add_type,
+const uint8_t *adv_addr,
+uint16_t skip,
+uint16_t sync_timeout, uint8_t unused,
+uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_PERIODIC_ADV_CREATE_SYNC_LEN);
+
+if (filter_policy > 1 || adv_add_type > 1){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (adv_sid > 0x0f){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (skip > 0x1f3 || sync_timeout > 0x4000 || sync_timeout < 0x0A){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+cmd[0] = filter_policy;
+cmd[1] = adv_sid;
+cmd[2] = adv_add_type;
+memcpy([3], adv_addr, 6);
+put_le16([9], skip);
+put_le16([11], sync_timeout);
+cmd[13] = unused;
 
 Review comment:
   use 0x00 for not instead `unused`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257595895
 
 

 ##
 File path: nimble/host/mesh/src/proxy.c
 ##
 @@ -417,7 +417,7 @@ void bt_mesh_proxy_beacon_send(struct bt_mesh_subnet *sub)
}
 
for (i = 0; i < ARRAY_SIZE(clients); i++) {
-   if (clients[i].conn_handle) {
+   if (clients[i].conn_handle != BLE_HS_CONN_HANDLE_NONE) {
 
 Review comment:
   this file should also by untouched. Please rebase.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257619010
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_ENABLE);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_enable(1, instance, buf, 
sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_op = BLE_GAP_OP_S_PERIODIC_ADV;
+
+ble_hs_unlock();
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
+struct os_mbuf **data)
+{
+
+/* In that case we always fit all data in single HCI command */
+#if MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE) <= BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN
+
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE)];
+uint16_t len = OS_MBUF_PKTLEN(*data);
+int rc;
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, opcode);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_data(
+instance,
+BLE_HCI_LE_SET_DATA_OPER_COMPLETE,
+*data, len,
+buf, sizeof(buf));
+
+if (rc) {
+return rc;
+}
+
+os_mbuf_adj(*data, MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE));
+*data = os_mbuf_trim_front(*data);
+
+return ble_hs_hci_cmd_tx_empty_ack(opcode, buf,
+BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN + len);
+#else
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN];

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257628078
 
 

 ##
 File path: nimble/host/src/ble_hs_periodic_disc.c
 ##
 @@ -0,0 +1,233 @@
+/*
+ * 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 
+#include 
+#include "syscfg/syscfg.h"
+#include "os/os.h"
+#include "host/ble_hs_id.h"
+#include "ble_hs_priv.h"
+
+static SLIST_HEAD(, ble_hs_periodic_sync) ble_hs_periodic_sync_handles;
+static struct os_mempool ble_hs_periodic_sync_pool;
+
+static os_membuf_t ble_hs_psync_elem_mem[
+OS_MEMPOOL_SIZE(MYNEWT_VAL(BLE_MAX_PERIODIC_SCANNING_SM),
+sizeof (struct ble_hs_periodic_sync))
+];
+
+int
+ble_hs_periodic_sync_can_alloc(void)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return 0;
+#endif
+
+return (ble_hs_periodic_sync_pool.mp_num_free >= 1);
+}
+
+struct ble_hs_periodic_sync *
+ble_hs_periodic_sync_alloc()
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return NULL;
+#endif
+
+struct ble_hs_periodic_sync *psync;
+
+psync = os_memblock_get(_hs_periodic_sync_pool);
+if (psync == NULL) {
+goto err;
+}
+memset(psync, 0, sizeof *psync);
+
+return psync;
+
+err:
+ble_hs_periodic_sync_free(psync);
+return NULL;
+}
+
+
+
+void
+ble_hs_periodic_sync_free(struct ble_hs_periodic_sync *psync)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return;
+#endif
+
+int rc;
+if (psync == NULL) {
+return;
+}
+
+#if MYNEWT_VAL(BLE_HS_DEBUG)
+memset(psync, 0xff, sizeof *psync);
+#endif
+rc = os_memblock_put(_hs_periodic_sync_pool, psync);
+BLE_HS_DBG_ASSERT_EVAL(rc == 0);
+}
+
+void
+ble_hs_periodic_sync_insert(struct ble_hs_periodic_sync *psync)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return;
+#endif
+
+BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
+
+BLE_HS_DBG_ASSERT_EVAL(
+   ble_hs_periodic_sync_find(psync->sync_handle) == NULL);
+
+SLIST_INSERT_HEAD(_hs_periodic_sync_handles, psync, bhc_next);
+}
+
+void
+ble_hs_periodic_sync_remove(struct ble_hs_periodic_sync *psync)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return;
+#endif
+
+BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
+
+SLIST_REMOVE(_hs_periodic_sync_handles,
+ psync,
+ ble_hs_periodic_sync,
+ bhc_next);
+}
+
+struct ble_hs_periodic_sync *
+ble_hs_periodic_sync_find(uint16_t sync_handle)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return NULL;
+#endif
+
+struct ble_hs_periodic_sync *psync;
+
+BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
+
+SLIST_FOREACH(psync, _hs_periodic_sync_handles, bhc_next) {
+if (psync->sync_handle == sync_handle) {
+return psync;
+}
+}
+
+return NULL;
+}
+
+struct ble_hs_periodic_sync *
+ble_hs_periodic_sync_find_assert(uint16_t sync_handle)
+{
+struct ble_hs_periodic_sync *psync;
+
+psync = ble_hs_periodic_sync_find(sync_handle);
+BLE_HS_DBG_ASSERT(psync != NULL);
+
+return psync;
+}
+
+struct ble_hs_periodic_sync *
+ble_hs_periodic_sync_find_by_adv_addr(const ble_addr_t *addr)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return NULL;
+#endif
+
+struct ble_hs_periodic_sync *psync;
+
+BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
+
+if (!addr) {
+return NULL;
+}
+
+SLIST_FOREACH(psync, _hs_periodic_sync_handles, bhc_next) {
+if (ble_addr_cmp(>advertiser_addr, addr) == 0) {
+return psync;
+}
+}
+
+return NULL;
+}
+
+struct ble_hs_periodic_sync *
+ble_hs_periodic_sync_find_by_adv_sid(uint16_t sid)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return 0;
+#endif
+
+struct ble_hs_periodic_sync *psync;
+
+BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
+
+SLIST_FOREACH(psync, _hs_periodic_sync_handles, bhc_next) {
+if (psync->adv_sid == sid) {
+return psync;
+}
+}
+
+return NULL;
+}
+
+int
+ble_hs_periodic_sync_exists(uint16_t sync_handle)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return 0;
+#endif
+return ble_hs_periodic_sync_find(sync_handle) != NULL;
+}
+
+/**
+ * Retrieves the first periodic 

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257612951
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1353,15 +1477,15 @@ ble_gap_rx_conn_complete(struct hci_le_conn_complete 
*evt, uint8_t instance)
 
 STATS_INC(ble_gap_stats, rx_conn_complete);
 
-/* in that case *only* status field is valid so we determine role
 
 Review comment:
   There is a lot of changes from here down to the end of the file which are 
not relevant. Please remove them from this patch. It is already big patch and 
let's don't make it bigger.
   Could you please prepare instead a patch with a coding style fixes instead?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257617713
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
 
 Review comment:
   not needed empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257625038
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_cmd.c
 ##
 @@ -1605,6 +1605,167 @@ ble_hs_hci_cmd_build_le_ext_adv_remove(uint8_t handle,
 
 return 0;
 }
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+int
+ble_hs_hci_cmd_build_le_periodic_adv_params(uint8_t handle,
+   const struct hci_periodic_adv_params 
*params,
+   uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN);
+
+if (handle > 0xEF){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (params->max_interval > 0x || params->max_interval < 6){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (params->min_interval > 0x || params->min_interval < 6){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = handle;
+
+put_le16([1], params->min_interval);
+put_le16([3], params->max_interval);
+put_le16([5], params->properties);
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_enable(uint8_t enable,
+   uint8_t handle,
+   uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN);
+
+if (enable > 1){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = enable;
+cmd[1] = handle;
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_data(uint8_t handle,
+ uint8_t operation,
+ struct os_mbuf *data,
+ uint8_t data_len,
+ uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= 3 + data_len);
+
+if (!data){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (!data->om_data){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (handle > 0xEF){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = handle;
+cmd[1] = operation;
+cmd[2] = data_len;
+os_mbuf_copydata(data, 0, data_len, cmd + 3);
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_create_sync(uint8_t filter_policy,
+uint8_t adv_sid,
+uint8_t adv_add_type,
+const uint8_t *adv_addr,
+uint16_t skip,
+uint16_t sync_timeout, uint8_t unused,
+uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_PERIODIC_ADV_CREATE_SYNC_LEN);
+
+if (filter_policy > 1 || adv_add_type > 1){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (adv_sid > 0x0f){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (skip > 0x1f3 || sync_timeout > 0x4000 || sync_timeout < 0x0A){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+cmd[0] = filter_policy;
+cmd[1] = adv_sid;
+cmd[2] = adv_add_type;
+memcpy([3], adv_addr, 6);
+put_le16([9], skip);
+put_le16([11], sync_timeout);
+cmd[13] = unused;
+
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_terminate_sync(uint16_t sync_handle,
+   uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_PERIODIC_ADV_TERM_SYNC_LEN);
+
+if (sync_handle > 0xEF){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = sync_handle;
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_add_dev_to_periodic_adv_list(
+uint8_t adv_add_type,
+const uint8_t *adv_addr,
+uint8_t adv_sid,
+uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_ADD_DEV_TO_PERIODIC_ADV_LIST_LEN);
+
+if (adv_add_type > 1 || adv_sid > 0x0f){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+
 
 Review comment:
   remove empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257617433
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
 
 Review comment:
   as above


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257606400
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -182,11 +194,14 @@ struct ble_gap_snapshot {
 void *cb_arg;
 };
 
-static SLIST_HEAD(ble_gap_hook_list, ble_gap_event_listener) 
ble_gap_event_listener_list;
 
 Review comment:
   I believe that from this place down to line 1336 changes are not relevant. 
Please remove them.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257607496
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1315,6 +1333,112 @@ ble_gap_rx_adv_set_terminated(struct 
hci_le_adv_set_terminated *evt)
 }
 #endif
 
+/* Periodic adv events */
+#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+void
+ble_gap_rx_peroidic_adv_sync_estab(
+struct hci_le_subev_periodic_adv_sync_estab *evt)
+{
+struct ble_gap_event event;
+struct ble_gap_master_state state;
+struct ble_hs_periodic_sync * psync;
+
+memset(, 0, sizeof event);
+
+/* There must be memory for psync as the check was done when creating
+ * sync
+ */
+psync = ble_hs_periodic_sync_alloc();
+BLE_HS_DBG_ASSERT(psync != NULL);
 
 Review comment:
   It is good oto have BLE_HS_DBG_ASSERT here but if it is not enabled we 
should also handle out of memory also without it. Please return when `psync` is 
NULL


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257633933
 
 

 ##
 File path: nimble/host/src/ble_hs_stop.c
 ##
 @@ -71,8 +111,8 @@ ble_hs_stop_terminate_next_conn(void)
 
 handle = ble_hs_atomic_first_conn_handle();
 if (handle == BLE_HS_CONN_HANDLE_NONE) {
-/* No open connections.  Signal completion of the stop procedure. */
-ble_hs_stop_done(0);
+/* No open connections.  Check for any active periodic sync */
+ble_hs_stop_terminate_next_periodic_sync();
 
 Review comment:
Can we rename it to `ble_terminate_periodic_sync() ` which would return `0` 
on success and some error otherwise.
   Then I would move this function to `ble_hs_stop`  before host start close 
all the connections.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257636645
 
 

 ##
 File path: nimble/syscfg.yml
 ##
 @@ -34,6 +34,10 @@ syscfg.defs:
 BLE_MAX_CONNECTIONS:
 description: 'The maximum number of concurrent connections.'
 value: 1
+BLE_MAX_PERIODIC_SCANNING_SM:
+description: >
+The maximum number of concurrent periodic scanning state machines.
 
 Review comment:
   Could we add here that it is the maximum number of periodic sync which can 
be created?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257616398
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
 
 Review comment:
   comment does not match. BTW. Maybe we could have defines for default ones?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257620884
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_ENABLE);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_enable(1, instance, buf, 
sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_op = BLE_GAP_OP_S_PERIODIC_ADV;
+
+ble_hs_unlock();
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
+struct os_mbuf **data)
+{
+
+/* In that case we always fit all data in single HCI command */
+#if MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE) <= BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN
+
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE)];
+uint16_t len = OS_MBUF_PKTLEN(*data);
+int rc;
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, opcode);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_data(
+instance,
+BLE_HCI_LE_SET_DATA_OPER_COMPLETE,
+*data, len,
+buf, sizeof(buf));
+
+if (rc) {
+return rc;
+}
+
+os_mbuf_adj(*data, MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE));
+*data = os_mbuf_trim_front(*data);
+
+return ble_hs_hci_cmd_tx_empty_ack(opcode, buf,
+BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN + len);
+#else
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN];

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257617527
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
 
 Review comment:
   space before {


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257617850
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_ENABLE);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_enable(1, instance, buf, 
sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_op = BLE_GAP_OP_S_PERIODIC_ADV;
+
+ble_hs_unlock();
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
+struct os_mbuf **data)
+{
+
 
 Review comment:
   not needed empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257627439
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -631,7 +709,8 @@ ble_hs_hci_evt_le_scan_timeout(uint8_t subevent, uint8_t 
*data, int len)
 }
 
 static int
-ble_hs_hci_evt_le_adv_set_terminated(uint8_t subevent, uint8_t *data, int len)
 
 Review comment:
   changes not relevant here and below in this file


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257617349
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
 
 Review comment:
   space before `{`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257619204
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_ENABLE);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_enable(1, instance, buf, 
sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_op = BLE_GAP_OP_S_PERIODIC_ADV;
+
+ble_hs_unlock();
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
+struct os_mbuf **data)
+{
+
+/* In that case we always fit all data in single HCI command */
+#if MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE) <= BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN
+
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE)];
+uint16_t len = OS_MBUF_PKTLEN(*data);
+int rc;
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, opcode);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_data(
+instance,
+BLE_HCI_LE_SET_DATA_OPER_COMPLETE,
+*data, len,
+buf, sizeof(buf));
+
+if (rc) {
+return rc;
+}
+
+os_mbuf_adj(*data, MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE));
+*data = os_mbuf_trim_front(*data);
+
+return ble_hs_hci_cmd_tx_empty_ack(opcode, buf,
+BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN + len);
+#else
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN];

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257635662
 
 

 ##
 File path: porting/nimble/include/syscfg/syscfg.h
 ##
 @@ -378,7 +378,7 @@
 
 /*** nimble */
 #ifndef MYNEWT_VAL_BLE_EXT_ADV
-#define MYNEWT_VAL_BLE_EXT_ADV (0)
+#define MYNEWT_VAL_BLE_EXT_ADV (1)
 
 Review comment:
   Please don't change  default configuration for port.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257635867
 
 

 ##
 File path: porting/nimble/include/syscfg/syscfg.h
 ##
 @@ -418,6 +422,11 @@
 #define MYNEWT_VAL_BLE_ATT_PREFERRED_MTU (256)
 #endif
 
+
+#ifndef MYNEWT_VAL_BLE_PERIODIC_ADV
+#define MYNEWT_VAL_BLE_PERIODIC_ADV (1)
 
 Review comment:
   please set it to 0


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257628177
 
 

 ##
 File path: nimble/host/src/ble_hs_periodic_disc.c
 ##
 @@ -0,0 +1,233 @@
+/*
+ * 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 
+#include 
+#include "syscfg/syscfg.h"
+#include "os/os.h"
+#include "host/ble_hs_id.h"
+#include "ble_hs_priv.h"
+
+static SLIST_HEAD(, ble_hs_periodic_sync) ble_hs_periodic_sync_handles;
+static struct os_mempool ble_hs_periodic_sync_pool;
+
+static os_membuf_t ble_hs_psync_elem_mem[
+OS_MEMPOOL_SIZE(MYNEWT_VAL(BLE_MAX_PERIODIC_SCANNING_SM),
+sizeof (struct ble_hs_periodic_sync))
+];
+
+int
+ble_hs_periodic_sync_can_alloc(void)
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return 0;
+#endif
+
+return (ble_hs_periodic_sync_pool.mp_num_free >= 1);
+}
+
+struct ble_hs_periodic_sync *
+ble_hs_periodic_sync_alloc()
+{
+#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
+return NULL;
+#endif
+
+struct ble_hs_periodic_sync *psync;
+
+psync = os_memblock_get(_hs_periodic_sync_pool);
+if (psync == NULL) {
+goto err;
+}
+memset(psync, 0, sizeof *psync);
+
+return psync;
+
+err:
+ble_hs_periodic_sync_free(psync);
+return NULL;
+}
+
+
 
 Review comment:
   please remove 2 empty lines


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257602282
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -155,6 +163,10 @@ struct ble_gap_slave_state {
 unsigned int directed:1;
 unsigned int legacy_pdu:1;
 unsigned int rnd_addr_set:1;
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+unsigned int per_configured:1;
+uint8_t   per_op;
 
 Review comment:
   I think `per_op` should be removed. Instead `op` should be used with a code 
`BLE_GAP_OP_S_ADV`
   Having this and `per_configured` should be sufficient.
   
   BTW.  just a nitpick, can we change `per_configured` to 
`periodic_configured`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257629476
 
 

 ##
 File path: nimble/host/src/ble_hs_periodic_disc_priv.h
 ##
 @@ -0,0 +1,63 @@
+/*
+ * 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_HS_PERIODIC_DISC_
+#define H_BLE_HS_PERIODIC_DISC_
+
+#include 
+#include "os/queue.h"
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct ble_hs_periodic_sync {
+SLIST_ENTRY(ble_hs_periodic_sync) bhc_next;
+uint16_t   sync_handle;
+uint8_tadv_sid;
+uint8_tadvertiser_addr_type;
+ble_addr_t advertiser_addr;
+uint8_tadvertiser_phy;
+uint16_t   periodic_adv_itvl;
+uint8_tadvertiser_clock_accuracy;
+};
+
+
 
 Review comment:
   please remove 2 empty lines


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257618851
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_ENABLE);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_enable(1, instance, buf, 
sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_op = BLE_GAP_OP_S_PERIODIC_ADV;
+
+ble_hs_unlock();
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
+struct os_mbuf **data)
+{
+
+/* In that case we always fit all data in single HCI command */
+#if MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE) <= BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN
+
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE)];
+uint16_t len = OS_MBUF_PKTLEN(*data);
+int rc;
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, opcode);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_data(
+instance,
+BLE_HCI_LE_SET_DATA_OPER_COMPLETE,
+*data, len,
+buf, sizeof(buf));
+
+if (rc) {
+return rc;
+}
+
+os_mbuf_adj(*data, MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE));
+*data = os_mbuf_trim_front(*data);
+
+return ble_hs_hci_cmd_tx_empty_ack(opcode, buf,
+BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN + len);
+#else
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN];

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257623578
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_cmd.c
 ##
 @@ -1605,6 +1605,167 @@ ble_hs_hci_cmd_build_le_ext_adv_remove(uint8_t handle,
 
 return 0;
 }
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+int
+ble_hs_hci_cmd_build_le_periodic_adv_params(uint8_t handle,
+   const struct hci_periodic_adv_params 
*params,
+   uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN);
+
+if (handle > 0xEF){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (params->max_interval > 0x || params->max_interval < 6){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (params->min_interval > 0x || params->min_interval < 6){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = handle;
+
+put_le16([1], params->min_interval);
+put_le16([3], params->max_interval);
+put_le16([5], params->properties);
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_enable(uint8_t enable,
+   uint8_t handle,
+   uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN);
+
+if (enable > 1){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = enable;
+cmd[1] = handle;
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_data(uint8_t handle,
+ uint8_t operation,
+ struct os_mbuf *data,
+ uint8_t data_len,
+ uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= 3 + data_len);
+
+if (!data){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (!data->om_data){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (handle > 0xEF){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+
+cmd[0] = handle;
+cmd[1] = operation;
+cmd[2] = data_len;
+os_mbuf_copydata(data, 0, data_len, cmd + 3);
+
+return 0;
+}
+
+int
+ble_hs_hci_cmd_build_le_periodic_adv_create_sync(uint8_t filter_policy,
+uint8_t adv_sid,
+uint8_t adv_add_type,
+const uint8_t *adv_addr,
+uint16_t skip,
+uint16_t sync_timeout, uint8_t unused,
+uint8_t *cmd, int cmd_len)
+{
+BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_PERIODIC_ADV_CREATE_SYNC_LEN);
+
+if (filter_policy > 1 || adv_add_type > 1){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (adv_sid > 0x0f){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+if (skip > 0x1f3 || sync_timeout > 0x4000 || sync_timeout < 0x0A){
+return BLE_ERR_INV_HCI_CMD_PARMS;
+}
+cmd[0] = filter_policy;
+cmd[1] = adv_sid;
+cmd[2] = adv_add_type;
+memcpy([3], adv_addr, 6);
+put_le16([9], skip);
+put_le16([11], sync_timeout);
+cmd[13] = unused;
+
 
 Review comment:
   remove empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257584500
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -708,6 +711,47 @@ struct ble_gap_event {
 uint8_t tx_phy;
 uint8_t rx_phy;
 } phy_updated;
+#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+/**
+ * Represents an periodic advertising sync established during discovery
+ * procedure.  Valid for the following event types:
+ * o BLE_GAP_EVENT_PERIODIC_ADV_SYNC_ESTAB
+ */
+struct {
+uint8_t status;
+uint16_t sync_handle;
+uint8_t sid;
+uint8_t adv_addr_type;
+uint8_t adv_addr[6];
+uint8_t adv_phy;
+uint16_t per_adv_ival;
+uint8_t adv_clk_accuracy;
+} per_adv_sync_estab;
+
+
+/**
+ * Represents a Periodic advertising report received during discovery
+ * procedure.  Valid for the following event types:
+ * o BLE_GAP_EVENT_PERIODIC_DISC
+ */
+struct {
+uint16_t sync_handle;
+uint8_t tx_power;
+int8_t rssi;
+uint8_t _unused;
+uint8_t data_status;
+uint8_t data_length;
+uint8_t *data;
+} per_disc;
+/**
 
 Review comment:
   please add empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-02-18 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r257618690
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2919,13 +3049,533 @@ ble_gap_ext_adv_remove(uint8_t instance)
 return 0;
 }
 
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_periodic_adv_params_tx(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+
+{
+struct hci_periodic_adv_params hci_adv_params;
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_PARAMS_LEN];
+int rc;
+
+memset(_adv_params, 0, sizeof(hci_adv_params));
+
+if (params->include_tx_power) {
+hci_adv_params.properties |= 
BLE_HCI_LE_SET_PERIODIC_ADV_PROP_INC_TX_PWR;
+}
+
+/* Fill optional fields if application did not specify them. */
+if (params->itvl_min == 0 && params->itvl_max == 0) {
+hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+} else {
+hci_adv_params.min_interval = params->itvl_min;
+hci_adv_params.max_interval = params->itvl_max;
+}
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_params(instance, 
_adv_params, buf,
+sizeof(buf));
+if (rc != 0) {
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(
+BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_PARAMS), buf,
+sizeof(buf));
+
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_params_validate(
+const struct ble_gap_periodic_adv_params *params)
+{
+if (!params) {
+return BLE_HS_EINVAL;
+}
+
+if (params->itvl_min > 0x || params->itvl_min < 6){
+return BLE_HS_EINVAL;
+}
+if (params->itvl_max > 0x || params->itvl_max < 6){
+return BLE_HS_EINVAL;
+}
+return 0;
+
+}
+
+int
+ble_gap_periodic_adv_configure(uint8_t instance,
+const struct ble_gap_periodic_adv_params *params)
+{
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_validate(params);
+if (rc) {
+return rc;
+}
+
+ble_hs_lock();
+
+/* The corresponding extended advertising instance should be configured */
+if (!ble_gap_slave[instance].configured){
+ble_hs_unlock();
+return ENOMEM;
+}
+
+/* Periodic advertising shall not be configured while it is already
+ * running.
+ * Bluetooth Core Specification, Section 7.8.61
+ */
+if (ble_gap_slave[instance].per_op == BLE_GAP_OP_S_PERIODIC_ADV){
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+rc = ble_gap_periodic_adv_params_tx(instance, params);
+if (rc) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_configured = 1;
+
+ble_hs_unlock();
+
+return 0;
+}
+
+int
+ble_gap_periodic_adv_start(uint8_t instance)
+{
+uint8_t buf[BLE_HCI_LE_SET_PERIODIC_ADV_ENABLE_LEN];
+uint16_t opcode;
+int rc;
+
+if (instance >= BLE_ADV_INSTANCES) {
+return BLE_HS_EINVAL;
+}
+
+ble_hs_lock();
+
+/* Periodic advertising cannot start unless it is configured before */
+if (!ble_gap_slave[instance].per_configured) {
+ble_hs_unlock();
+return BLE_HS_EINVAL;
+}
+
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PERIODIC_ADV_ENABLE);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_enable(1, instance, buf, 
sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
+if (rc != 0) {
+ble_hs_unlock();
+return rc;
+}
+
+ble_gap_slave[instance].per_op = BLE_GAP_OP_S_PERIODIC_ADV;
+
+ble_hs_unlock();
+return 0;
+}
+
+static int
+ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
+struct os_mbuf **data)
+{
+
+/* In that case we always fit all data in single HCI command */
+#if MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE) <= BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN
+
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE)];
+uint16_t len = OS_MBUF_PKTLEN(*data);
+int rc;
+
+opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, opcode);
+
+rc = ble_hs_hci_cmd_build_le_periodic_adv_data(
+instance,
+BLE_HCI_LE_SET_DATA_OPER_COMPLETE,
+*data, len,
+buf, sizeof(buf));
+
+if (rc) {
+return rc;
+}
+
+os_mbuf_adj(*data, MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE));
+*data = os_mbuf_trim_front(*data);
+
+return ble_hs_hci_cmd_tx_empty_ack(opcode, buf,
+BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN + len);
+#else
+static uint8_t buf[BLE_HCI_SET_PERIODIC_ADV_DATA_HDR_LEN +
+   BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN];

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250656047
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -969,23 +969,23 @@ int ble_gap_ext_adv_remove(uint8_t instance);
 
 #if MYNEWT_VAL(BLE_PERIODIC_ADV)
 /* Periodic Advertising */
-int ble_gap_per_adv_configure(uint8_t instance, const struct 
ble_gap_per_adv_params *params);
-int ble_gap_per_adv_start(uint8_t instance);
-int ble_gap_per_adv_stop(uint8_t instance);
-int ble_gap_per_adv_set_data(uint8_t instance, struct os_mbuf *data);
-int ble_gap_per_adv_create_sync (uint8_t filter_policy,
+int ble_gap_periodic_adv_configure(uint8_t instance, const struct 
ble_gap_periodic_adv_params *params);
+int ble_gap_periodic_adv_start(uint8_t instance);
+int ble_gap_periodic_adv_stop(uint8_t instance);
+int ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data);
+int ble_gap_periodic_adv_create_sync (uint8_t filter_policy,
 
 Review comment:
   indentation issue. Tabs used


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250689134
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1355,7 +1377,7 @@ ble_gap_rx_per_adv_sync_estab(struct 
hci_le_subev_per_adv_sync_estab *evt)
event.per_adv_sync_estab.per_adv_ival   = evt->per_adv_ival;
event.per_adv_sync_estab.adv_clk_accuracy   = evt->adv_clk_accuracy;
 
-   ble_gap_master.pending_create_sync--;
+   ble_gap_master.pending_create_sync = 0;
 
 Review comment:
   `pending_create_sync` is `uint32_t`. Can be reduced.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250689904
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1388,23 +1410,33 @@ ble_gap_rx_per_adv_rpt(struct hci_le_subev_per_adv_rpt 
*evt) {
 }
 
 void
-ble_gap_rx_per_adv_sync_lost(struct hci_le_subev_per_adv_sync_lost *evt) {
-
+ble_gap_rx_periodic_adv_sync_lost(
+   struct hci_le_subev_periodic_adv_sync_lost *evt) {
struct ble_gap_event event;
struct ble_gap_master_state state;
+struct ble_hs_periodic_disc *pdisc;
 
memset(, 0, sizeof event);
 
-   event.type = BLE_GAP_EVENT_PER_ADV_SYNC_LOST;
+   event.type = BLE_GAP_EVENT_PERIODIC_ADV_SYNC_LOST;
event.per_adv_sync_lost.sync_handle = evt->sync_handle;
 
+   ble_hs_lock();
+   /* The handle must be in the list */
+   pdisc = ble_hs_periodic_disc_find_assert(evt->sync_handle);
+
+   /* Remove the handle from the list */
+   ble_hs_periodic_disc_remove(pdisc);
 
 Review comment:
   maybe `ble_hs_periodic_sync_remove` and the same with other functions related


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250681768
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1336,14 +1336,36 @@ ble_gap_rx_adv_set_terminated(struct 
hci_le_adv_set_terminated *evt)
 /* Periodic adv events */
 #if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
 void
-ble_gap_rx_per_adv_sync_estab(struct hci_le_subev_per_adv_sync_estab *evt)
+ble_gap_rx_peroidic_adv_sync_estab(
+   struct hci_le_subev_periodic_adv_sync_estab *evt)
 {
struct ble_gap_event event;
struct ble_gap_master_state state;
+   struct ble_hs_periodic_disc * pdisc;
 
memset(, 0, sizeof event);
 
-   event.type = BLE_GAP_EVENT_PER_ADV_SYNC_ESTAB;
+   /* There must be memory for pdisc as the check was done when creating
+* sync
+*/
+   pdisc = ble_hs_periodic_disc_alloc(evt->sync_handle);
 
 Review comment:
   sync_handle seems to be written to pdisc twice. I would remove sync_handle 
from `ble_hs_periodic_disc_alloc` 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250682665
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1336,14 +1336,36 @@ ble_gap_rx_adv_set_terminated(struct 
hci_le_adv_set_terminated *evt)
 /* Periodic adv events */
 #if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
 void
-ble_gap_rx_per_adv_sync_estab(struct hci_le_subev_per_adv_sync_estab *evt)
+ble_gap_rx_peroidic_adv_sync_estab(
+   struct hci_le_subev_periodic_adv_sync_estab *evt)
 {
struct ble_gap_event event;
struct ble_gap_master_state state;
+   struct ble_hs_periodic_disc * pdisc;
 
 Review comment:
   `struct ble_hs_periodic_disc` is to describe `sync` so what about `struct 
ble_hs_periodic_sync` ? and then variable maybe `psync`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250625641
 
 

 ##
 File path: nimble/include/nimble/hci_common.h
 ##
 @@ -984,12 +1001,23 @@ struct hci_ext_adv_params
 uint8_t scan_req_notif;
 };
 
+
 
 Review comment:
   please remove redundant empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250688315
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -969,23 +969,23 @@ int ble_gap_ext_adv_remove(uint8_t instance);
 
 #if MYNEWT_VAL(BLE_PERIODIC_ADV)
 /* Periodic Advertising */
-int ble_gap_per_adv_configure(uint8_t instance, const struct 
ble_gap_per_adv_params *params);
-int ble_gap_per_adv_start(uint8_t instance);
-int ble_gap_per_adv_stop(uint8_t instance);
-int ble_gap_per_adv_set_data(uint8_t instance, struct os_mbuf *data);
-int ble_gap_per_adv_create_sync (uint8_t filter_policy,
+int ble_gap_periodic_adv_configure(uint8_t instance, const struct 
ble_gap_periodic_adv_params *params);
+int ble_gap_periodic_adv_start(uint8_t instance);
+int ble_gap_periodic_adv_stop(uint8_t instance);
+int ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data);
+int ble_gap_periodic_adv_create_sync (uint8_t filter_policy,
 
 Review comment:
   Did you consider to add callback to `ble_gap_periodic_adv_create_sync()` for 
periodic advertising events?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250629567
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_priv.h
 ##
 @@ -235,6 +235,48 @@ ble_hs_hci_cmd_build_le_ext_adv_params(uint8_t handle,
 int
 ble_hs_hci_cmd_build_le_ext_adv_remove(uint8_t handle,
uint8_t *cmd, int cmd_len);
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+int
+ble_hs_hci_cmd_build_le_per_adv_params(uint8_t handle,
+  
const struct hci_per_adv_params *params,
 
 Review comment:
   Indentation issue (tab used instead of spaces)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250683320
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1336,14 +1336,36 @@ ble_gap_rx_adv_set_terminated(struct 
hci_le_adv_set_terminated *evt)
 /* Periodic adv events */
 #if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
 void
-ble_gap_rx_per_adv_sync_estab(struct hci_le_subev_per_adv_sync_estab *evt)
+ble_gap_rx_peroidic_adv_sync_estab(
+   struct hci_le_subev_periodic_adv_sync_estab *evt)
 {
struct ble_gap_event event;
struct ble_gap_master_state state;
+   struct ble_hs_periodic_disc * pdisc;
 
memset(, 0, sizeof event);
 
-   event.type = BLE_GAP_EVENT_PER_ADV_SYNC_ESTAB;
+   /* There must be memory for pdisc as the check was done when creating
+* sync
+*/
+   pdisc = ble_hs_periodic_disc_alloc(evt->sync_handle);
+   BLE_HS_DBG_ASSERT(pdisc != NULL);
+
+   pdisc->sync_handle   = evt->sync_handle;
+   pdisc->adv_sid   = evt->sid;
+   pdisc->advertiser_addr_type  = evt->adv_addr_type;
+
+   memcpy(pdisc->advertiser_addr.val, evt->adv_addr, 6);
+
+   pdisc->advertiser_phy= evt->adv_phy;
+   pdisc->periodic_adv_itvl = evt->per_adv_ival;
+   pdisc->advertiser_clock_accuracy = evt->adv_clk_accuracy;
+
+   ble_hs_lock();
+   ble_hs_periodic_disc_insert(pdisc);
+   ble_hs_unlock();
+
+   event.type = BLE_GAP_EVENT_PERIODIC_ADV_SYNC_ESTAB;
 
 Review comment:
   did you consider having in the event only `sync_handle` and then some other 
API to get information? Maybe something more is immediately needed by 
applications (e.g. sid) but I I guess not all of them


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250625758
 
 

 ##
 File path: nimble/include/nimble/hci_common.h
 ##
 @@ -984,12 +1001,23 @@ struct hci_ext_adv_params
 uint8_t scan_req_notif;
 };
 
+
 /* LE Extended Advertising Report Event */
 struct hci_le_subev_ext_adv_rpt {
 uint8_t num_reports;
 struct hci_ext_adv_report_param params[0];
 } __attribute__((packed));
 
+
 
 Review comment:
   please remove redundant empty line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250695263
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1336,14 +1336,36 @@ ble_gap_rx_adv_set_terminated(struct 
hci_le_adv_set_terminated *evt)
 /* Periodic adv events */
 #if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
 void
-ble_gap_rx_per_adv_sync_estab(struct hci_le_subev_per_adv_sync_estab *evt)
+ble_gap_rx_peroidic_adv_sync_estab(
+   struct hci_le_subev_periodic_adv_sync_estab *evt)
 {
struct ble_gap_event event;
struct ble_gap_master_state state;
+   struct ble_hs_periodic_disc * pdisc;
 
memset(, 0, sizeof event);
 
-   event.type = BLE_GAP_EVENT_PER_ADV_SYNC_ESTAB;
+   /* There must be memory for pdisc as the check was done when creating
+* sync
+*/
+   pdisc = ble_hs_periodic_disc_alloc(evt->sync_handle);
+   BLE_HS_DBG_ASSERT(pdisc != NULL);
+
+   pdisc->sync_handle   = evt->sync_handle;
+   pdisc->adv_sid   = evt->sid;
+   pdisc->advertiser_addr_type  = evt->adv_addr_type;
+
+   memcpy(pdisc->advertiser_addr.val, evt->adv_addr, 6);
+
+   pdisc->advertiser_phy= evt->adv_phy;
+   pdisc->periodic_adv_itvl = evt->per_adv_ival;
+   pdisc->advertiser_clock_accuracy = evt->adv_clk_accuracy;
+
+   ble_hs_lock();
+   ble_hs_periodic_disc_insert(pdisc);
 
 Review comment:
   btw - please check `ble_hs_stop` where we disconnect all the connections on 
stack close. We need to add there code t terminate all periodic_sync


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250625027
 
 

 ##
 File path: porting/nimble/include/syscfg/syscfg.h
 ##
 @@ -378,7 +378,7 @@
 
 /*** nimble */
 #ifndef MYNEWT_VAL_BLE_EXT_ADV
-#define MYNEWT_VAL_BLE_EXT_ADV (0)
 
 Review comment:
   we don't want this change for porting version.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-24 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r250625159
 
 

 ##
 File path: porting/nimble/include/syscfg/syscfg.h
 ##
 @@ -418,6 +418,11 @@
 #define MYNEWT_VAL_BLE_ATT_PREFERRED_MTU (256)
 #endif
 
+
+#ifndef MYNEWT_VAL_BLE_PERIODIC_ADV
+#define MYNEWT_VAL_BLE_PERIODIC_ADV (1)
 
 Review comment:
   For porting I would got for 0 as a default.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-17 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r248716373
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -913,6 +966,28 @@ int ble_gap_ext_adv_stop(uint8_t instance);
 int ble_gap_ext_adv_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_rsp_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_remove(uint8_t instance);
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+/* Periodic Advertising */
+int ble_gap_per_adv_configure(uint8_t instance, const struct 
ble_gap_per_adv_params *params);
+int ble_gap_per_adv_start(uint8_t instance);
+int ble_gap_per_adv_stop(uint8_t instance);
+int ble_gap_per_adv_set_data(uint8_t instance, struct os_mbuf *data);
+int ble_gap_per_adv_create_sync (uint8_t filter_policy,
 
 Review comment:
   Ok I see. There is no way to establish sync without scanning.
   Now I'm thinking if it would make sense to provide a new callback there for 
data on established sync? This callback would be attached to the "conn" object 
I mentioned in other comments.
   I believe for application it would be convenient to receive data on 
particular callback. Does it make sense?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-17 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r248564637
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2915,8 +2985,515 @@ ble_gap_ext_adv_remove(uint8_t instance)
 
 memset(_gap_slave[instance], 0, sizeof(struct ble_gap_slave_state));
 ble_hs_unlock();
+}
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_per_adv_params_tx(uint8_t instance,
+   const struct ble_gap_per_adv_params *params)
+
+{
+   struct hci_per_adv_params hci_adv_params;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN];
+   int rc;
+
+   memset(_adv_params, 0, sizeof(hci_adv_params));
+
+   if (params->include_tx_power) {
+   hci_adv_params.properties |= 
BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR;
+   }
+
+   /* Fill optional fields if application did not specify them. */
+   if (params->itvl_min == 0 && params->itvl_max == 0) {
+   hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+   hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+   } else {
+   hci_adv_params.min_interval = params->itvl_min;
+   hci_adv_params.max_interval = params->itvl_max;
+   }
+
+   rc = ble_hs_hci_cmd_build_le_per_adv_params(instance, _adv_params, 
buf,
+   sizeof(buf));
+   if (rc != 0) {
+   return rc;
+   }
+
+   rc = ble_hs_hci_cmd_tx_empty_ack(
+   BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PER_ADV_PARAMS), buf,
+   sizeof(buf));
+
+   if (rc != 0) {
+   return rc;
+   }
+
+   return 0;
+}
+
+static int
+ble_gap_per_adv_params_validate(
+   const struct ble_gap_per_adv_params *params) {
+   if (!params) {
+   return BLE_HS_EINVAL;
+   }
+
+   // 1- if anonymous return BLE_HS_EINVAL
+
+   // 2- if periodic adv is enabled return BLE_HS_EINVAL
+
+   return 0;
 
-return 0;
+}
+
+int
+ble_gap_per_adv_configure(uint8_t instance,
+   const struct ble_gap_per_adv_params *params) {
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   rc = ble_gap_per_adv_params_validate(params);
+   if (rc) {
+   return rc;
+   }
+
+   rc = ble_gap_per_adv_params_tx(instance, params);
+   if (rc) {
+   ble_hs_unlock();
+   return rc;
+   }
+
+   return 0;
+}
+
+int
+ble_gap_per_adv_start(uint8_t instance) {
+   const uint8_t *rnd_addr;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_ENABLE_LEN];
+   uint16_t opcode;
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   ble_hs_lock();
+
+   // 1- if periodic_adv_enable when periodic_adv is enabled => change the 
random address
+
+   //if (!ble_gap_slave[instance].configured) {
+   //ble_hs_unlock();
+   //return BLE_HS_EINVAL;
+   //}
+
+   //if (ble_gap_slave[instance].op != BLE_GAP_OP_NULL) {
+   //ble_hs_unlock();
+   //return  BLE_HS_EALREADY;
+   //}
+
+   /* verify own address type if random address for instance wasn't 
explicitly
+* set
+*/
+   //switch (ble_gap_slave[instance].our_addr_type) {
+   //case BLE_OWN_ADDR_RANDOM:
+   //case BLE_OWN_ADDR_RPA_RANDOM_DEFAULT:
+   //if (ble_gap_slave[instance].rnd_addr_set) {
+   //break;
+   //}
+   ///* fall through */
+   //case BLE_OWN_ADDR_PUBLIC:
+   //case BLE_OWN_ADDR_RPA_PUBLIC_DEFAULT:
+   //default:
+   //rc = 
ble_hs_id_use_addr(ble_gap_slave[instance].our_addr_type);
+   //if (rc) {
+   //ble_hs_unlock();
+   //return BLE_HS_EINVAL;
+   //}
+   //break;
+   //}
+   /* fallback to ID static random address if using random address and 
instance
+* wasn't configured with own address
+*/
+   //if (!ble_gap_slave[instance].rnd_addr_set) {
+   //switch (ble_gap_slave[instance].our_addr_type) {
+   //case BLE_OWN_ADDR_RANDOM:
+   //case BLE_OWN_ADDR_RPA_RANDOM_DEFAULT:
+   //rc = ble_hs_id_addr(BLE_ADDR_RANDOM, _addr, NULL);
+   //if (rc != 0) {
+   //ble_hs_unlock();
+   //return rc;
+   //}
+   //
+   //rc = ble_gap_ext_adv_set_addr_no_lock(instance, rnd_addr);
+   //if (rc != 0) {
+   //ble_hs_unlock();
+   //return rc;
+   //}
+   //break;
+   //default:
+   //break;
+   

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-17 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r248563114
 
 

 ##
 File path: nimble/host/src/ble_hs_dbg.c
 ##
 @@ -675,7 +676,7 @@ ble_hs_dbg_event_disp(uint8_t *evbuf)
 ble_hs_dbg_num_comp_pkts_disp(evdata, len);
 break;
 case BLE_HCI_EVCODE_LE_META:
-ble_hs_dbg_le_event_disp(evdata[0], len-1, evdata + 1);
+ble_hs_dbg_le_event_disp(evdata[0], len - 1, evdata + 1);
 
 Review comment:
   this change is good. What I suggest it to create separate commit for this as 
this change is not related to this commit


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-17 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r248564324
 
 

 ##
 File path: nimble/include/nimble/hci_common.h
 ##
 @@ -552,9 +552,17 @@ extern "C" {
 
 /* --- LE set periodic advertising parameters (OCF 0x003E) */
 #define BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN   (7)
+#define BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR  (0x0040)
 
 /* --- LE set periodic advertising data (OCF 0x003F) */
 #define BLE_HCI_LE_SET_PER_ADV_DATA_LEN BLE_HCI_VARIABLE_LEN
+#define BLE_HCI_MAX_PER_ADV_DATA_LEN(252)
+#define BLE_HCI_SET_PER_ADV_DATA_HDR_LEN(3)
+
+#define BLE_HCI_LE_SET_PER_ADV_DATA_OPER_INT(0)
 
 Review comment:
   I like your idea. What about name BLE_HCI_LE_SET_DATA_OPER_INT?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-17 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r248563641
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -20,14 +20,14 @@
 #include 
 #include 
 #include 
+#include 
 #include "os/os.h"
 #include "nimble/hci_common.h"
 #include "nimble/ble_hci_trans.h"
 #include "host/ble_gap.h"
 #include "host/ble_monitor.h"
 #include "ble_hs_priv.h"
 #include "ble_hs_dbg_priv.h"
-
 
 Review comment:
   Removing line 30 is not needed. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246777201
 
 

 ##
 File path: nimble/host/src/ble_hs_dbg.c
 ##
 @@ -675,7 +676,7 @@ ble_hs_dbg_event_disp(uint8_t *evbuf)
 ble_hs_dbg_num_comp_pkts_disp(evdata, len);
 break;
 case BLE_HCI_EVCODE_LE_META:
-ble_hs_dbg_le_event_disp(evdata[0], len-1, evdata + 1);
+ble_hs_dbg_le_event_disp(evdata[0], len - 1, evdata + 1);
 
 Review comment:
   valid fix, please do in separate commit.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246772097
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -913,6 +966,28 @@ int ble_gap_ext_adv_stop(uint8_t instance);
 int ble_gap_ext_adv_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_rsp_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_remove(uint8_t instance);
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+/* Periodic Advertising */
+int ble_gap_per_adv_configure(uint8_t instance, const struct 
ble_gap_per_adv_params *params);
+int ble_gap_per_adv_start(uint8_t instance);
+int ble_gap_per_adv_stop(uint8_t instance);
+int ble_gap_per_adv_set_data(uint8_t instance, struct os_mbuf *data);
+int ble_gap_per_adv_create_sync (uint8_t filter_policy,
+uint8_t
adv_sid, uint8_t adv_add_type,
+const uint8_t 
*adv_addr, uint16_t skip,
+uint16_t 
sync_timeout, uint8_t unused);
+int ble_gap_per_adv_create_sync_cancel(void);
+int ble_gap_per_adv_terminate_sync(uint16_t sync_handle);
+int ble_gap_add_dev_to_per_adv_list(uint8_t adv_add_type, const uint8_t 
*adv_add,
 
 Review comment:
   what about `ble_gap_periodic_adv_list_add()` and  
`ble_gap_periodic_adv_list_remove()`, `ble_gap_periodic_adv_list_clear()`, 
`ble_gap_periodic_adv_list_read_size()` ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246743021
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2915,8 +2985,515 @@ ble_gap_ext_adv_remove(uint8_t instance)
 
 memset(_gap_slave[instance], 0, sizeof(struct ble_gap_slave_state));
 ble_hs_unlock();
 
 Review comment:
   there is missing return 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246771377
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -913,6 +966,28 @@ int ble_gap_ext_adv_stop(uint8_t instance);
 int ble_gap_ext_adv_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_rsp_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_remove(uint8_t instance);
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+/* Periodic Advertising */
+int ble_gap_per_adv_configure(uint8_t instance, const struct 
ble_gap_per_adv_params *params);
+int ble_gap_per_adv_start(uint8_t instance);
+int ble_gap_per_adv_stop(uint8_t instance);
+int ble_gap_per_adv_set_data(uint8_t instance, struct os_mbuf *data);
+int ble_gap_per_adv_create_sync (uint8_t filter_policy,
 
 Review comment:
   I think we should be consistent here with `ble_gap_connect` and 
`ble_gap_disc` and pass  ` ble_gap_event_fn *cb, void *cb_arg`
   
   I can imagine application which is interested only in periodic sync 
established so it needs to have a way to provide `ble_gap_event_fn` of course 
nowadays we have `ble_gap_event_listener_register()`  but for now I would 
suggest to be consistent with the way we do for connect and discovery


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246742609
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2915,8 +2985,515 @@ ble_gap_ext_adv_remove(uint8_t instance)
 
 memset(_gap_slave[instance], 0, sizeof(struct ble_gap_slave_state));
 ble_hs_unlock();
+}
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_per_adv_params_tx(uint8_t instance,
+   const struct ble_gap_per_adv_params *params)
+
+{
+   struct hci_per_adv_params hci_adv_params;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN];
+   int rc;
+
+   memset(_adv_params, 0, sizeof(hci_adv_params));
+
+   if (params->include_tx_power) {
+   hci_adv_params.properties |= 
BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR;
+   }
+
+   /* Fill optional fields if application did not specify them. */
+   if (params->itvl_min == 0 && params->itvl_max == 0) {
+   hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+   hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+   } else {
+   hci_adv_params.min_interval = params->itvl_min;
+   hci_adv_params.max_interval = params->itvl_max;
+   }
+
+   rc = ble_hs_hci_cmd_build_le_per_adv_params(instance, _adv_params, 
buf,
+   sizeof(buf));
+   if (rc != 0) {
+   return rc;
+   }
+
+   rc = ble_hs_hci_cmd_tx_empty_ack(
+   BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PER_ADV_PARAMS), buf,
+   sizeof(buf));
+
+   if (rc != 0) {
+   return rc;
+   }
+
+   return 0;
+}
+
+static int
+ble_gap_per_adv_params_validate(
+   const struct ble_gap_per_adv_params *params) {
+   if (!params) {
+   return BLE_HS_EINVAL;
+   }
+
+   // 1- if anonymous return BLE_HS_EINVAL
+
+   // 2- if periodic adv is enabled return BLE_HS_EINVAL
+
+   return 0;
 
-return 0;
+}
+
+int
+ble_gap_per_adv_configure(uint8_t instance,
+   const struct ble_gap_per_adv_params *params) {
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   rc = ble_gap_per_adv_params_validate(params);
+   if (rc) {
+   return rc;
+   }
+
+   rc = ble_gap_per_adv_params_tx(instance, params);
+   if (rc) {
+   ble_hs_unlock();
+   return rc;
+   }
+
+   return 0;
+}
+
+int
+ble_gap_per_adv_start(uint8_t instance) {
+   const uint8_t *rnd_addr;
 
 Review comment:
   it does not build due to this rnd_addr


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246773685
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2915,8 +2985,515 @@ ble_gap_ext_adv_remove(uint8_t instance)
 
 memset(_gap_slave[instance], 0, sizeof(struct ble_gap_slave_state));
 ble_hs_unlock();
+}
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_per_adv_params_tx(uint8_t instance,
+   const struct ble_gap_per_adv_params *params)
+
+{
+   struct hci_per_adv_params hci_adv_params;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN];
+   int rc;
+
+   memset(_adv_params, 0, sizeof(hci_adv_params));
+
+   if (params->include_tx_power) {
+   hci_adv_params.properties |= 
BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR;
+   }
+
+   /* Fill optional fields if application did not specify them. */
+   if (params->itvl_min == 0 && params->itvl_max == 0) {
+   hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+   hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+   } else {
+   hci_adv_params.min_interval = params->itvl_min;
+   hci_adv_params.max_interval = params->itvl_max;
+   }
+
+   rc = ble_hs_hci_cmd_build_le_per_adv_params(instance, _adv_params, 
buf,
+   sizeof(buf));
+   if (rc != 0) {
+   return rc;
+   }
+
+   rc = ble_hs_hci_cmd_tx_empty_ack(
+   BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PER_ADV_PARAMS), buf,
+   sizeof(buf));
+
+   if (rc != 0) {
+   return rc;
+   }
+
+   return 0;
+}
+
+static int
+ble_gap_per_adv_params_validate(
+   const struct ble_gap_per_adv_params *params) {
+   if (!params) {
+   return BLE_HS_EINVAL;
+   }
+
+   // 1- if anonymous return BLE_HS_EINVAL
+
+   // 2- if periodic adv is enabled return BLE_HS_EINVAL
+
+   return 0;
 
-return 0;
+}
+
+int
+ble_gap_per_adv_configure(uint8_t instance,
+   const struct ble_gap_per_adv_params *params) {
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   rc = ble_gap_per_adv_params_validate(params);
+   if (rc) {
+   return rc;
+   }
+
+   rc = ble_gap_per_adv_params_tx(instance, params);
+   if (rc) {
+   ble_hs_unlock();
+   return rc;
+   }
+
+   return 0;
+}
+
+int
+ble_gap_per_adv_start(uint8_t instance) {
+   const uint8_t *rnd_addr;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_ENABLE_LEN];
+   uint16_t opcode;
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   ble_hs_lock();
+
+   // 1- if periodic_adv_enable when periodic_adv is enabled => change the 
random address
 
 Review comment:
   Please use `/* TODO comments */`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246774743
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2915,8 +2985,515 @@ ble_gap_ext_adv_remove(uint8_t instance)
 
 memset(_gap_slave[instance], 0, sizeof(struct ble_gap_slave_state));
 ble_hs_unlock();
+}
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_per_adv_params_tx(uint8_t instance,
+   const struct ble_gap_per_adv_params *params)
+
+{
+   struct hci_per_adv_params hci_adv_params;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN];
+   int rc;
+
+   memset(_adv_params, 0, sizeof(hci_adv_params));
+
+   if (params->include_tx_power) {
+   hci_adv_params.properties |= 
BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR;
+   }
+
+   /* Fill optional fields if application did not specify them. */
+   if (params->itvl_min == 0 && params->itvl_max == 0) {
+   hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+   hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+   } else {
+   hci_adv_params.min_interval = params->itvl_min;
+   hci_adv_params.max_interval = params->itvl_max;
+   }
+
+   rc = ble_hs_hci_cmd_build_le_per_adv_params(instance, _adv_params, 
buf,
+   sizeof(buf));
+   if (rc != 0) {
+   return rc;
+   }
+
+   rc = ble_hs_hci_cmd_tx_empty_ack(
+   BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PER_ADV_PARAMS), buf,
+   sizeof(buf));
+
+   if (rc != 0) {
+   return rc;
+   }
+
+   return 0;
+}
+
+static int
+ble_gap_per_adv_params_validate(
+   const struct ble_gap_per_adv_params *params) {
+   if (!params) {
+   return BLE_HS_EINVAL;
+   }
+
+   // 1- if anonymous return BLE_HS_EINVAL
+
+   // 2- if periodic adv is enabled return BLE_HS_EINVAL
+
+   return 0;
 
-return 0;
+}
+
+int
+ble_gap_per_adv_configure(uint8_t instance,
+   const struct ble_gap_per_adv_params *params) {
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   rc = ble_gap_per_adv_params_validate(params);
+   if (rc) {
+   return rc;
+   }
+
+   rc = ble_gap_per_adv_params_tx(instance, params);
+   if (rc) {
+   ble_hs_unlock();
+   return rc;
+   }
+
+   return 0;
+}
+
+int
+ble_gap_per_adv_start(uint8_t instance) {
+   const uint8_t *rnd_addr;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_ENABLE_LEN];
+   uint16_t opcode;
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   ble_hs_lock();
+
+   // 1- if periodic_adv_enable when periodic_adv is enabled => change the 
random address
+
+   //if (!ble_gap_slave[instance].configured) {
+   //ble_hs_unlock();
+   //return BLE_HS_EINVAL;
+   //}
+
+   //if (ble_gap_slave[instance].op != BLE_GAP_OP_NULL) {
+   //ble_hs_unlock();
+   //return  BLE_HS_EALREADY;
+   //}
+
+   /* verify own address type if random address for instance wasn't 
explicitly
+* set
+*/
+   //switch (ble_gap_slave[instance].our_addr_type) {
+   //case BLE_OWN_ADDR_RANDOM:
+   //case BLE_OWN_ADDR_RPA_RANDOM_DEFAULT:
+   //if (ble_gap_slave[instance].rnd_addr_set) {
+   //break;
+   //}
+   ///* fall through */
+   //case BLE_OWN_ADDR_PUBLIC:
+   //case BLE_OWN_ADDR_RPA_PUBLIC_DEFAULT:
+   //default:
+   //rc = 
ble_hs_id_use_addr(ble_gap_slave[instance].our_addr_type);
+   //if (rc) {
+   //ble_hs_unlock();
+   //return BLE_HS_EINVAL;
+   //}
+   //break;
+   //}
+   /* fallback to ID static random address if using random address and 
instance
+* wasn't configured with own address
+*/
+   //if (!ble_gap_slave[instance].rnd_addr_set) {
+   //switch (ble_gap_slave[instance].our_addr_type) {
+   //case BLE_OWN_ADDR_RANDOM:
+   //case BLE_OWN_ADDR_RPA_RANDOM_DEFAULT:
+   //rc = ble_hs_id_addr(BLE_ADDR_RANDOM, _addr, NULL);
+   //if (rc != 0) {
+   //ble_hs_unlock();
+   //return rc;
+   //}
+   //
+   //rc = ble_gap_ext_adv_set_addr_no_lock(instance, rnd_addr);
+   //if (rc != 0) {
+   //ble_hs_unlock();
+   //return rc;
+   //}
+   //break;
+   //default:
+   //break;
+   

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246775167
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2915,8 +2985,515 @@ ble_gap_ext_adv_remove(uint8_t instance)
 
 memset(_gap_slave[instance], 0, sizeof(struct ble_gap_slave_state));
 ble_hs_unlock();
+}
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_per_adv_params_tx(uint8_t instance,
+   const struct ble_gap_per_adv_params *params)
+
+{
+   struct hci_per_adv_params hci_adv_params;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN];
+   int rc;
+
+   memset(_adv_params, 0, sizeof(hci_adv_params));
+
+   if (params->include_tx_power) {
+   hci_adv_params.properties |= 
BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR;
+   }
+
+   /* Fill optional fields if application did not specify them. */
+   if (params->itvl_min == 0 && params->itvl_max == 0) {
+   hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+   hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+   } else {
+   hci_adv_params.min_interval = params->itvl_min;
+   hci_adv_params.max_interval = params->itvl_max;
+   }
+
+   rc = ble_hs_hci_cmd_build_le_per_adv_params(instance, _adv_params, 
buf,
+   sizeof(buf));
+   if (rc != 0) {
+   return rc;
+   }
+
+   rc = ble_hs_hci_cmd_tx_empty_ack(
+   BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PER_ADV_PARAMS), buf,
+   sizeof(buf));
+
+   if (rc != 0) {
+   return rc;
+   }
+
+   return 0;
+}
+
+static int
+ble_gap_per_adv_params_validate(
+   const struct ble_gap_per_adv_params *params) {
+   if (!params) {
+   return BLE_HS_EINVAL;
+   }
+
+   // 1- if anonymous return BLE_HS_EINVAL
+
+   // 2- if periodic adv is enabled return BLE_HS_EINVAL
+
+   return 0;
 
-return 0;
+}
+
+int
+ble_gap_per_adv_configure(uint8_t instance,
+   const struct ble_gap_per_adv_params *params) {
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   rc = ble_gap_per_adv_params_validate(params);
+   if (rc) {
+   return rc;
+   }
+
+   rc = ble_gap_per_adv_params_tx(instance, params);
+   if (rc) {
+   ble_hs_unlock();
+   return rc;
+   }
+
+   return 0;
+}
+
+int
+ble_gap_per_adv_start(uint8_t instance) {
+   const uint8_t *rnd_addr;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_ENABLE_LEN];
+   uint16_t opcode;
+   int rc;
+
+   if (instance >= BLE_ADV_INSTANCES) {
+   return BLE_HS_EINVAL;
+   }
+
+   ble_hs_lock();
+
+   // 1- if periodic_adv_enable when periodic_adv is enabled => change the 
random address
+
+   //if (!ble_gap_slave[instance].configured) {
+   //ble_hs_unlock();
+   //return BLE_HS_EINVAL;
+   //}
+
+   //if (ble_gap_slave[instance].op != BLE_GAP_OP_NULL) {
+   //ble_hs_unlock();
+   //return  BLE_HS_EALREADY;
+   //}
+
+   /* verify own address type if random address for instance wasn't 
explicitly
+* set
+*/
+   //switch (ble_gap_slave[instance].our_addr_type) {
+   //case BLE_OWN_ADDR_RANDOM:
+   //case BLE_OWN_ADDR_RPA_RANDOM_DEFAULT:
+   //if (ble_gap_slave[instance].rnd_addr_set) {
+   //break;
+   //}
+   ///* fall through */
+   //case BLE_OWN_ADDR_PUBLIC:
+   //case BLE_OWN_ADDR_RPA_PUBLIC_DEFAULT:
+   //default:
+   //rc = 
ble_hs_id_use_addr(ble_gap_slave[instance].our_addr_type);
+   //if (rc) {
+   //ble_hs_unlock();
+   //return BLE_HS_EINVAL;
+   //}
+   //break;
+   //}
+   /* fallback to ID static random address if using random address and 
instance
+* wasn't configured with own address
+*/
+   //if (!ble_gap_slave[instance].rnd_addr_set) {
+   //switch (ble_gap_slave[instance].our_addr_type) {
+   //case BLE_OWN_ADDR_RANDOM:
+   //case BLE_OWN_ADDR_RPA_RANDOM_DEFAULT:
+   //rc = ble_hs_id_addr(BLE_ADDR_RANDOM, _addr, NULL);
+   //if (rc != 0) {
+   //ble_hs_unlock();
+   //return rc;
+   //}
+   //
+   //rc = ble_gap_ext_adv_set_addr_no_lock(instance, rnd_addr);
+   //if (rc != 0) {
+   //ble_hs_unlock();
+   //return rc;
+   //}
+   //break;
+   //default:
+   //break;
+   

[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246755740
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1033,7 +1033,7 @@ ble_gap_conn_broken(uint16_t conn_handle, int reason)
 ble_sm_connection_broken(conn_handle);
 ble_gatts_connection_broken(conn_handle);
 ble_gattc_connection_broken(conn_handle);
-ble_hs_flow_connection_broken(conn_handle);;
 
 Review comment:
   This is valid fix but this should be separate commit as this change is not 
relevant to periodic advertising.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246760551
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -1315,6 +1315,76 @@ ble_gap_rx_adv_set_terminated(struct 
hci_le_adv_set_terminated *evt)
 }
 #endif
 
+/* Periodic adv events */
+#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+void
+ble_gap_rx_per_adv_sync_estab(struct hci_le_subev_per_adv_sync_estab *evt)
+{
+   struct ble_gap_event event;
+   struct ble_gap_master_state state;
+
+   memset(, 0, sizeof event);
+
+   event.type = BLE_GAP_EVENT_PER_ADV_SYNC_ESTAB;
 
 Review comment:
   I think in this place we should create some object similar to `struct 
ble_hs_conn` and maintain for application on some list.
   
   I think this struct shall keep all the information from `struct 
hci_le_subev_per_adv_sync_estab` and BLE_GAP_EVENT_PER_ADV_SYNC_ESTAB could 
carry only sync_handle, status. Note that there can be one outstanding periodic 
sync create at the time.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246783111
 
 

 ##
 File path: nimble/host/src/ble_hs_startup.c
 ##
 @@ -256,12 +256,15 @@ ble_hs_startup_le_set_evmask_tx(void)
  * Enable the following LE events:
  *   0x0800 LE PHY Update Complete Event
  *   0x1000 LE Extended Advertising Report Event
+ *   0x2000 LE Periodic Advertising Sync Established Event
+ *   0x4000 LE Periodic Advertising Report Event
+ *   0x8000 LE Periodic Advertising Sync Lost Event
  *   0x0001 LE Extended Scan Timeout Event
  *   0x0002 LE Extended Advertising Set Terminated Event
  *   0x0004 LE Scan Request Received Event
  *   0x0008 LE Channel Selection Algorithm Event
  */
-mask |= 0x000f1800;
+mask |= 0x000f1800 | 0x2000 | 0x4000 | 0x8000;
 
 Review comment:
   I would just calculate it in


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246792196
 
 

 ##
 File path: nimble/include/nimble/hci_common.h
 ##
 @@ -552,9 +552,17 @@ extern "C" {
 
 /* --- LE set periodic advertising parameters (OCF 0x003E) */
 #define BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN   (7)
 
 Review comment:
   Looks like we also used PER_ADV, so probably this is why you use this 
pattern for functions name. Can we chance all this defines to 
FOO_PERIODIC_ADV_FOO?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246739108
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -124,6 +124,9 @@ struct hci_conn_update;
 #define BLE_GAP_EVENT_REPEAT_PAIRING17
 #define BLE_GAP_EVENT_PHY_UPDATE_COMPLETE   18
 #define BLE_GAP_EVENT_EXT_DISC  19
+#define BLE_GAP_EVENT_PER_ADV_SYNC_ESTAB   20
 
 Review comment:
   Please fix indentation.  Tab shall not be used.
   Please check 
https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md for 
conding standards here
   
   This comment is valid for full PR.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246776456
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -3478,7 +4055,7 @@ ble_gap_ext_conn_create_tx(
 memset(hcc.peer_addr, 0, sizeof hcc.peer_addr);
 } else {
 hcc.filter_policy = BLE_HCI_CONN_FILT_NO_WL;
-hcc.peer_addr_type = peer_addr->type;;
 
 Review comment:
   valid fix, please create separate commit.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246776946
 
 

 ##
 File path: nimble/host/src/ble_gap_priv.h
 ##
 @@ -82,6 +82,11 @@ void ble_gap_rx_le_scan_timeout(void);
 #if MYNEWT_VAL(BLE_EXT_ADV)
 void ble_gap_rx_ext_adv_report(struct ble_gap_ext_disc_desc *desc);
 void ble_gap_rx_adv_set_terminated(struct hci_le_adv_set_terminated *evt);
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+void ble_gap_rx_per_adv_sync_estab(struct hci_le_subev_per_adv_sync_estab 
*evt);
 
 Review comment:
   could we use same pattern as before? i.e. 
`ble_gap_rx_periodic_adv_sync_estab()`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246779124
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -92,14 +94,18 @@ static const struct ble_hs_hci_evt_le_dispatch_entry
 { BLE_HCI_LE_SUBEV_CONN_COMPLETE, ble_hs_hci_evt_le_conn_complete },
 { BLE_HCI_LE_SUBEV_ADV_RPT, ble_hs_hci_evt_le_adv_rpt },
 { BLE_HCI_LE_SUBEV_CONN_UPD_COMPLETE,
-  ble_hs_hci_evt_le_conn_upd_complete },
+   ble_hs_hci_evt_le_conn_upd_complete },
 { BLE_HCI_LE_SUBEV_LT_KEY_REQ, ble_hs_hci_evt_le_lt_key_req },
 { BLE_HCI_LE_SUBEV_REM_CONN_PARM_REQ, ble_hs_hci_evt_le_conn_parm_req },
 { BLE_HCI_LE_SUBEV_ENH_CONN_COMPLETE, ble_hs_hci_evt_le_conn_complete },
 { BLE_HCI_LE_SUBEV_DIRECT_ADV_RPT, ble_hs_hci_evt_le_dir_adv_rpt },
 { BLE_HCI_LE_SUBEV_PHY_UPDATE_COMPLETE,
-ble_hs_hci_evt_le_phy_update_complete },
+   ble_hs_hci_evt_le_phy_update_complete },
 
 Review comment:
   not related


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246777301
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_cmd.c
 ##
 @@ -1605,6 +1605,119 @@ ble_hs_hci_cmd_build_le_ext_adv_remove(uint8_t handle,
 
 return 0;
 }
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+int
+ble_hs_hci_cmd_build_le_per_adv_params(uint8_t handle,
+  
const struct hci_per_adv_params *params,
+  
uint8_t *cmd, int cmd_len)
+{
+
 
 Review comment:
   empty line not needed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246755098
 
 

 ##
 File path: nimble/host/include/host/ble_gap.h
 ##
 @@ -913,6 +966,28 @@ int ble_gap_ext_adv_stop(uint8_t instance);
 int ble_gap_ext_adv_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_rsp_set_data(uint8_t instance, struct os_mbuf *data);
 int ble_gap_ext_adv_remove(uint8_t instance);
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+/* Periodic Advertising */
+int ble_gap_per_adv_configure(uint8_t instance, const struct 
ble_gap_per_adv_params *params);
 
 Review comment:
i would suggest to use following pattern  for the function names:
   
   instead of `ble_gap_per_foo` I would go with `ble_gap_periodic_adv_foo`
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246773258
 
 

 ##
 File path: nimble/host/src/ble_gap.c
 ##
 @@ -2915,8 +2985,515 @@ ble_gap_ext_adv_remove(uint8_t instance)
 
 memset(_gap_slave[instance], 0, sizeof(struct ble_gap_slave_state));
 ble_hs_unlock();
+}
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+static int
+ble_gap_per_adv_params_tx(uint8_t instance,
+   const struct ble_gap_per_adv_params *params)
+
+{
+   struct hci_per_adv_params hci_adv_params;
+   uint8_t buf[BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN];
+   int rc;
+
+   memset(_adv_params, 0, sizeof(hci_adv_params));
+
+   if (params->include_tx_power) {
+   hci_adv_params.properties |= 
BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR;
+   }
+
+   /* Fill optional fields if application did not specify them. */
+   if (params->itvl_min == 0 && params->itvl_max == 0) {
+   hci_adv_params.min_interval = 30 / 1.25;   //30 ms
+   hci_adv_params.max_interval = 60 / 1.25;   //150 ms
+
+   } else {
+   hci_adv_params.min_interval = params->itvl_min;
+   hci_adv_params.max_interval = params->itvl_max;
+   }
+
+   rc = ble_hs_hci_cmd_build_le_per_adv_params(instance, _adv_params, 
buf,
+   sizeof(buf));
+   if (rc != 0) {
+   return rc;
+   }
+
+   rc = ble_hs_hci_cmd_tx_empty_ack(
+   BLE_HCI_OP(BLE_HCI_OGF_LE, 
BLE_HCI_OCF_LE_SET_PER_ADV_PARAMS), buf,
+   sizeof(buf));
+
+   if (rc != 0) {
+   return rc;
+   }
+
+   return 0;
+}
+
+static int
+ble_gap_per_adv_params_validate(
+   const struct ble_gap_per_adv_params *params) {
+   if (!params) {
+   return BLE_HS_EINVAL;
+   }
+
+   // 1- if anonymous return BLE_HS_EINVAL
 
 Review comment:
   looks like this function should  take instance and do full validation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246779059
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -92,14 +94,18 @@ static const struct ble_hs_hci_evt_le_dispatch_entry
 { BLE_HCI_LE_SUBEV_CONN_COMPLETE, ble_hs_hci_evt_le_conn_complete },
 { BLE_HCI_LE_SUBEV_ADV_RPT, ble_hs_hci_evt_le_adv_rpt },
 { BLE_HCI_LE_SUBEV_CONN_UPD_COMPLETE,
-  ble_hs_hci_evt_le_conn_upd_complete },
+   ble_hs_hci_evt_le_conn_upd_complete },
 
 Review comment:
   not related


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246780701
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -620,6 +626,73 @@ ble_hs_hci_evt_le_ext_adv_rpt(uint8_t subevent, uint8_t 
*data, int len)
 #endif
 return 0;
 }
+static int
+ble_hs_hci_evt_le_per_adv_sync_estab (uint8_t subevent, uint8_t *data, int len)
+{
+#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+   struct hci_le_subev_per_adv_sync_estab evt;
+
+   if (len < BLE_HCI_LE_PER_ADV_SYNC_ESTAB_LEN) {
+   return BLE_HS_ECONTROLLER;
+   }
+
+   evt.status = data[1];
+   evt.sync_handle = get_le16(data + 2);
+   evt.sid = data[4];
+   evt.adv_addr_type = data[5];
+   memcpy(evt.adv_addr, [6], 6);
+   evt.adv_phy = data[12];
+   evt.per_adv_ival = get_le16(data + 13);
+   evt.adv_clk_accuracy = data[15];
+
+   ble_gap_rx_per_adv_sync_estab();
+#endif
+
+   return 0;
+}
+
+static int
+ble_hs_hci_evt_le_per_adv_rpt (uint8_t subevent, uint8_t *data, int len)
 
 Review comment:
   redundant space before `(` - there are other places with the same.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246778276
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_cmd.c
 ##
 @@ -1605,6 +1605,119 @@ ble_hs_hci_cmd_build_le_ext_adv_remove(uint8_t handle,
 
 return 0;
 }
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+int
+ble_hs_hci_cmd_build_le_per_adv_params(uint8_t handle,
+  
const struct hci_per_adv_params *params,
+  
uint8_t *cmd, int cmd_len)
+{
+
+   BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN);
+
+   cmd[0] = handle;
+
+   put_le16([1], params->min_interval);
 
 Review comment:
   usually in this file we do HCI parameter validation. We should ad it here 
and for other commands below


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246784139
 
 

 ##
 File path: nimble/include/nimble/hci_common.h
 ##
 @@ -552,9 +552,17 @@ extern "C" {
 
 /* --- LE set periodic advertising parameters (OCF 0x003E) */
 #define BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN   (7)
+#define BLE_HCI_LE_SET_PER_ADV_PROP_INC_TX_PWR  (0x0040)
 
 /* --- LE set periodic advertising data (OCF 0x003F) */
 #define BLE_HCI_LE_SET_PER_ADV_DATA_LEN BLE_HCI_VARIABLE_LEN
+#define BLE_HCI_MAX_PER_ADV_DATA_LEN(252)
+#define BLE_HCI_SET_PER_ADV_DATA_HDR_LEN(3)
+
+#define BLE_HCI_LE_SET_PER_ADV_DATA_OPER_INT(0)
 
 Review comment:
   I think we should use the ones from EXT_ADV: 
BLE_HCI_LE_SET_EXT_ADV_DATA_OPER_INT  and so on


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246780280
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -20,14 +20,14 @@
 #include 
 #include 
 #include 
+#include 
 
 Review comment:
is already there in line 20


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rymanluk commented on a change in pull request #283: Adding Periodic Advertising Feature

2019-01-10 Thread GitBox
rymanluk commented on a change in pull request #283: Adding Periodic 
Advertising Feature
URL: https://github.com/apache/mynewt-nimble/pull/283#discussion_r246779326
 
 

 ##
 File path: nimble/host/src/ble_hs_hci_evt.c
 ##
 @@ -620,6 +626,73 @@ ble_hs_hci_evt_le_ext_adv_rpt(uint8_t subevent, uint8_t 
*data, int len)
 #endif
 return 0;
 }
+static int
 
 Review comment:
   please add empty line here


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services