Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth:Host:Scan: "bt_le_per_adv_list_add" function doesn't work #38520

Closed
saleh-unikie opened this issue Sep 14, 2021 · 5 comments · Fixed by #38594
Closed

Bluetooth:Host:Scan: "bt_le_per_adv_list_add" function doesn't work #38520

saleh-unikie opened this issue Sep 14, 2021 · 5 comments · Fixed by #38594
Assignees
Labels
area: Bluetooth Controller area: Bluetooth bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@saleh-unikie
Copy link
Contributor

saleh-unikie commented Sep 14, 2021

Describe the bug
I am trying to develop an application that supports multiple "periodic extended advertisement" based on this sample:
https://docs.zephyrproject.org/latest/samples/bluetooth/direction_finding_connectionless_rx/README.html

By default it only supports one synchronized device at a time, to support multiple device, I tried to use "bt_le_per_adv_list_add" function to add peer device LE address to the periodic advertising list. Also "BT_LE_PER_ADV_SYNC_OPT_USE_PER_ADV_LIST" is added to the options. the final source code of "create_sync" function is:

static void create_sync(void)
{
	int err;

	printk("Creating Periodic Advertising Sync...");
	bt_addr_le_copy(&sync_create_param.addr, &per_addr);
	sync_create_param.options = BT_LE_PER_ADV_SYNC_OPT_USE_PER_ADV_LIST;
	sync_create_param.sid = per_sid;
	sync_create_param.skip = 0;
	sync_create_param.timeout = 0xa;
	err = bt_le_per_adv_sync_create(&sync_create_param, &sync);

	err = bt_le_per_adv_list_add(&per_addr, per_sid);
	if (err != 0) {
		printk("add failed (err %d)\n", err);
		return;
	}
	printk("success.\n");
}

In the above code, failed (err -5) is printed. bt_le_per_adv_list_add calls bt_hci_cmd_send_sync function which fails by opcode 0x2047 status 0x01, where status 0x01 means BT_HCI_ERR_UNKNOWN_CMD as defined at hci_err.h

Indeed, executing the above code, calls cmd_handle from hci_driver.c which consequently calls hci_cmd_handle function. The corresponding opcode is "0x2047", so it calls controller_cmd_handle from hci.c which returns -EINVAL because this opcode is not implemented.

To Reproduce
Steps to reproduce the behavior:

  1. modify "Bluetooth: Direction Finding Periodic Advertising Locator" sample as described above.
  2. west build -b nrf52833dk_nrf52833
  3. west flash
  4. See error

Expected behavior
bt_le_per_adv_list_add function adds devices to the periodic advertising list as it is described in the bluetooth.h header file.

Impact
It is not possible to receive advertisement information from more than one AoA tag at the same time.

Logs and console output
I have added printk function to some of the above methods to see the errors, result is:

*** Booting Zephyr OS build v2.7.0-rc1-100-g50ff77fa910d  ***                                                            
opcode 0x0c03, err:0                                                                                                     
opcode 0x1003, err:0                                                                                                     
opcode 0x1001, err:0                                                                                                     
opcode 0x1002, err:0                                                                                                     
opcode 0x2003, err:0                                                                                                     
opcode 0x201c, err:0                                                                                                     
opcode 0x2058, err:0                                                                                                     
opcode 0x2001, err:0                                                                                                     
opcode 0x0c01, err:0                                                                                                     
opcode 0x2044, err:0                                                                                   
opcode 0x2047, err:-22                                                                                                   
opcode 0x2047 status 0x01                                                                                                
add failed (err -5) 

modified files:
hci.c:

struct net_buf *hci_cmd_handle(struct net_buf *cmd, void **node_rx)
{
        ...
        if (err == -EINVAL) {
		evt = cmd_status(BT_HCI_ERR_UNKNOWN_CMD);
	}

	printk("opcode 0x%04x, err:%i\n", _opcode, err);

	return evt;
}

hci_core.c

int bt_hci_cmd_send_sync(uint16_t opcode, struct net_buf *buf,
			 struct net_buf **rsp)
{
    ...
	if (status) {
               ...
		printk("opcode 0x%04x status 0x%02x\n", opcode, status);
               ...
        }
    ...
}

Environment (please complete the following information):

  • OS: Linux
  • Toolchain: Zephyr SDK
  • Commit SHA: 50ff77f

Additional context
By searching BT_HCI_OP_LE_ADD_DEV_TO_PER_ADV_LIST in the project files, you can see it is defined at hci.h and only is used by scan.c but is not implemented ate any other source file.

@saleh-unikie saleh-unikie added the bug The issue is a bug, or the PR is fixing a bug label Sep 14, 2021
@Thalley
Copy link
Collaborator

Thalley commented Sep 14, 2021

Hi @saleh-unikie the feature is missing from the controller, but is implemented by: #38338

@saleh-unikie
Copy link
Contributor Author

saleh-unikie commented Sep 14, 2021

Hi
Thanks for the reply! @Thalley

In connection with this problem, I think it is needed to define the maximum number of supported simultaneous synchs by modifying this config: CONFIG_BT_PER_ADV_SYNC_MAX

But the above config, causes to use a lot of memory space and in a normal application, it is not possible to set it to a number more than 5 or 6. It is not reasonable and I think somewhere it is not implemented efficient (maybe here at ul_df_types.h or here at ull_df.c)

Is it needed to set this config to a number bigger than 1? if yes, is this usage of memory normal?

@Thalley
Copy link
Collaborator

Thalley commented Sep 14, 2021

Hi
Thanks for the reply! @Thalley

In connection with this problem, I think it is needed to define the maximum number of supported simultaneous synchs by modifying this config: CONFIG_BT_PER_ADV_SYNC_MAX

But the above config, causes to use a lot of memory space and in a normal application, it is not possible to set it to a number more than 5 or 6. It is not reasonable and I think somewhere it is not implemented efficient (maybe here at ul_df_types.h or here at ull_df.c)

Is it needed to set this config to a number bigger than 1? if yes, is this usage of memory normal?

I think @ppryga Might be best at answering that

@saleh-unikie
Copy link
Contributor Author

saleh-unikie commented Sep 14, 2021

I am not sure, but it seems "CONFIG_BT_PER_ADV_SYNC_MAX" is similar to this new defined config "BT_CTLR_SYNC_PERIODIC_ADV_LIST_SIZE", or at least it is possible to define the second one based on the first one.

@cvinayak
Copy link
Contributor

CONFIG_BT_PER_ADV_SYNC_MAX is the compile time maximum supported simultaneous Periodic Synchronizations, and Memory Capacity Exceeded error is used to convey insufficient resources to handle any more periodic advertising trains.

CONFIG_BT_CTLR_SYNC_PERIODIC_ADV_LIST_SIZE is the value returned by HCI_LE_Read_Periodic_Advertiser_List_Size command, and is the total number of Periodic Advertiser list entries that can be stored in the Controller.

CONFIG_BT_CTLR_SYNC_PERIODIC_ADV_LIST_SIZE should not be derived from CONFIG_BT_PER_ADV_SYNC_MAX as the former is Bluetooth Core Specification's feature required definition and later is solely implementation (host/controller) defined resource count value.

@cfriedt cfriedt added the priority: low Low impact/importance bug label Sep 14, 2021
cvinayak added a commit to cvinayak/zephyr that referenced this issue Sep 16, 2021
Fix Periodic Advertising Sync Establishment to accept
synchronization establishment to device listed in the
Periodic Advertisers List when filter policy was used.

Fixes zephyrproject-rtos#38520.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
carlescufi pushed a commit that referenced this issue Sep 16, 2021
Fix Periodic Advertising Sync Establishment to accept
synchronization establishment to device listed in the
Periodic Advertisers List when filter policy was used.

Fixes #38520.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Controller area: Bluetooth bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants