Multi-NUS: Sending several messages in a row

Hi,

In my setup I have one central where several peripherals are connected. I'm using the Multi-NUS service to communicate from the central to the peripherals. Here you see a snippet of sending a broadcast to all peripherals. I'm using a single semaphore which will be freed in the on_ble_data_sent(...) callback

K_SEM_DEFINE(ble_send_semaphore, 1, 1);

void ble_connector_t::on_ble_data_sent(bt_nus_client *nus, uint8_t err, const uint8_t *const data, uint16_t len) {
	k_sem_give(&ble_send_semaphore);
}

int ble_connector_t::send_broadcast(uint8_t *data, uint16_t len) {
    int err = 0;
	LOG_INF("BLE broadcast: Length:%d", len);

	const size_t num_nus_conns = bt_conn_ctx_count(&conns_ctx_lib);
	for (size_t i = 0; i < num_nus_conns; i++) {
		const struct bt_conn_ctx *ctx = bt_conn_ctx_get_by_id(&conns_ctx_lib, i);
		if (ctx) {
			struct bt_nus_client *nus_client = (struct bt_nus_client*) ctx->data;
			if (nus_client) {
				err = k_sem_take(&ble_send_semaphore, K_MSEC(5000));
				if(err) {
					LOG_WRN("BLE semaphore timeout, dropping packet (err %d)", err);
				} else {
					err = bt_nus_client_send(nus_client, data, len);
					if (err) {
						LOG_WRN("Failed to send data over BLE connection (err %d)", err);
						k_sem_give(&ble_send_semaphore);
					}
				}
			}
			bt_conn_ctx_release(&conns_ctx_lib, (void *)ctx->data);
		}
	}
	return err;
}

The problem with this is you kinda destroy the performance. You can't send a broadcast to all peripherals in "parallel" because the system waits for each message to be finished (semaphore). If I get rid of the semaphore I can send one broadcast to all peripherals without any problems but if I try to send two broadcast fast in a row, the second one will fail.

I'm wondering how I could improve the performance of this.

  • If I get rid of the semaphore, several messages to the same peripheral will fail, cause there is still a write process ongoing (NUS_C_RX_WRITE_PENDING flag)
  • If I use the semaphore I wait for each message to be finished, although I could send all in "parallel" (if they are for different peripherals).
  • Should I use a semaphore array, one for each peripheral?
  • Is there any other elegant way, maybe a queue for each peripheral, am I missing anything?

My Setup is: NRF52840, Zephyr, NCS 2.5.1 

Thanks for your help,
Phobios

Parents
  • Hi,

    Are you basing it on this blogpost?

    I think the best would be to have one semaphore per link, that would you could send the messages in parallel but at the same time avoiding writing to more than one link at once,

    When you try write to one device more than once, what error code does it fail with?

    regards

    Jared 

  • Hi Jared,

    yes I'm basing on the blogpost.

    The error is -120 (-EALREADY) cause the bit NUS_C_RX_WRITE_PENDING is already set.

    int bt_nus_client_send(struct bt_nus_client *nus_c, const uint8_t *data, uint16_t len) {
    	int err;
    
    	if (!nus_c->conn) {
    		return -ENOTCONN;
    	}
    
    	if (atomic_test_and_set_bit(&nus_c->state, NUS_C_RX_WRITE_PENDING)) {
    		return -EALREADY;
    	}
    
    	nus_c->rx_write_params.func = on_sent;
    	nus_c->rx_write_params.handle = nus_c->handles.rx;
    	nus_c->rx_write_params.offset = 0;
    	nus_c->rx_write_params.data = data;
    	nus_c->rx_write_params.length = len;
    
    	err = bt_gatt_write(nus_c->conn, &nus_c->rx_write_params);
    	if (err) {
    		atomic_clear_bit(&nus_c->state, NUS_C_RX_WRITE_PENDING);
    	}
    
    	return err;
    }

    I'll try that with the semaphore array but how should I assign the semaphore index to the connections? On taking the semaphore I could use the index which I'm using with bt_conn_ctx_get_by_id(...) in the send_broadcast(...) function. But in the on_ble_data_sent function I do not have the id. So how should I know which semaphore index I should give when on_ble_data_sent is called?

Reply
  • Hi Jared,

    yes I'm basing on the blogpost.

    The error is -120 (-EALREADY) cause the bit NUS_C_RX_WRITE_PENDING is already set.

    int bt_nus_client_send(struct bt_nus_client *nus_c, const uint8_t *data, uint16_t len) {
    	int err;
    
    	if (!nus_c->conn) {
    		return -ENOTCONN;
    	}
    
    	if (atomic_test_and_set_bit(&nus_c->state, NUS_C_RX_WRITE_PENDING)) {
    		return -EALREADY;
    	}
    
    	nus_c->rx_write_params.func = on_sent;
    	nus_c->rx_write_params.handle = nus_c->handles.rx;
    	nus_c->rx_write_params.offset = 0;
    	nus_c->rx_write_params.data = data;
    	nus_c->rx_write_params.length = len;
    
    	err = bt_gatt_write(nus_c->conn, &nus_c->rx_write_params);
    	if (err) {
    		atomic_clear_bit(&nus_c->state, NUS_C_RX_WRITE_PENDING);
    	}
    
    	return err;
    }

    I'll try that with the semaphore array but how should I assign the semaphore index to the connections? On taking the semaphore I could use the index which I'm using with bt_conn_ctx_get_by_id(...) in the send_broadcast(...) function. But in the on_ble_data_sent function I do not have the id. So how should I know which semaphore index I should give when on_ble_data_sent is called?

Children
  • Hi,

    That is a very good point.

    What if you make two arrays, one containing the semaphores for each link and another containing the bt_nus_client pointer for each link. You then make sure that they are indexed correspondingly, meaning semaphore[i] contains the semaphore for the same link as bt_nus_client[i] points to.

    bt_nus_client is passed as parameter to the callback handler on_ble_data_sent. You can then iterate through the bt_nus_client array and then find the correct index, and then free the semaphore corresponding to that index.

    Something like this.

    K_SEM_DEFINE(ble_send_semaphore, 1, 1);
    
    semaphore[num_nus_conns]
    bt_nus_client [num_nus_conns]
    
    
    
    void ble_connector_t::on_ble_data_sent(bt_nus_client *nus, uint8_t err, const uint8_t *const data, uint16_t len) {
    
    	for (size_t i = 0; i< num_nus_conns; i++)
    	{
    		if(bt_nus_client[i] == nus)
    		{
    			k_sem_give(&semaphore[i]);
    		}
    	}
    
         
    }
    
    int ble_connector_t::send_broadcast(uint8_t *data, uint16_t len) {
        int err = 0;
    	LOG_INF("BLE broadcast: Length:%d", len);
    
    	const size_t num_nus_conns = bt_conn_ctx_count(&conns_ctx_lib);
    	for (size_t i = 0; i < num_nus_conns; i++) {
    		const struct bt_conn_ctx *ctx = bt_conn_ctx_get_by_id(&conns_ctx_lib, i);
    		if (ctx) {
    			struct bt_nus_client[i] = (struct bt_nus_client*) ctx->data;
    			if (nus_client[i]) {
    				err = k_sem_take(&semaphore[i]);, K_MSEC(5000));
    				if(err) {
    					LOG_WRN("BLE semaphore timeout, dropping packet (err %d)", err);
    				} else {
    					err = bt_nus_client_send(nus_client[i], data, len);
    					if (err) {
    						LOG_WRN("Failed to send data over BLE connection (err %d)", err);
    						k_sem_give(&semaphore[i]);
    					}
    				}
    			}
    			bt_conn_ctx_release(&conns_ctx_lib, (void *)ctx->data);
    		}
    	}
    	return err;
    }

    Note, this is just psudo code, it's not supposed to run, but I think it gives an overview of my suggestion.

    regards

    Jared 

  • Hi Jared,

    thanks a lot; I tried it and it works fine. I have altered it a bit and I am using bt_conn_index(...) to get the connection index, then I only need the semaphore array and not the bt_nus_client array.

    uint8_t ble_connector_t::on_ble_data_received(bt_nus_client *nus, const uint8_t *const ble_data, uint16_t len) {
        .....
        uint8_t conn_index = bt_conn_index(nus_client->conn);
        k_sem_take(&ble_send_semaphore[conn_index], K_MSEC(5000));
        ....
    }
    
    void ble_connector_t::on_ble_data_sent(bt_nus_client *nus, uint8_t err, const uint8_t *const data, uint16_t len) {
        uint8_t conn_index = bt_conn_index(nus_client->conn);
        k_sem_give(&ble_send_semaphore[conn_index]);
        ...
    }
    Best Regards,
    Manuel
Related