-
Notifications
You must be signed in to change notification settings - Fork 32
Add PA sync service #352
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
base: dev
Are you sure you want to change the base?
Add PA sync service #352
Conversation
63d8e82 to
d11efce
Compare
huangyulong3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
huangyulong3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
huangyulong3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| strlcpy(info->name, (char*)data->data, MIN(sizeof(info->name), data->len)); | ||
| break; | ||
|
|
||
| #ifdef CONFIG_BLUETOOTH_AURACAST_SINK | ||
| case BT_AD_BROADCAST_NAME: | ||
| strlcpy(info->broadcast_name, (char*)data->data, | ||
| MIN(sizeof(info->broadcast_name), data->len)); |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strlcpy function call specifies a maximum copy length using MIN(sizeof(info->name), data->len). However, data->len includes the AD Type byte (1 octet), which should not be counted in the data length for the name. This means an extra byte from the next AD entry could be copied into the name buffer. The correct length should be data->len - 1 to account for the AD Type byte that has already been excluded from data->data.
| strlcpy(info->name, (char*)data->data, MIN(sizeof(info->name), data->len)); | |
| break; | |
| #ifdef CONFIG_BLUETOOTH_AURACAST_SINK | |
| case BT_AD_BROADCAST_NAME: | |
| strlcpy(info->broadcast_name, (char*)data->data, | |
| MIN(sizeof(info->broadcast_name), data->len)); | |
| strlcpy(info->name, (char*)data->data, MIN(sizeof(info->name), data->len - 1)); | |
| break; | |
| #ifdef CONFIG_BLUETOOTH_AURACAST_SINK | |
| case BT_AD_BROADCAST_NAME: | |
| strlcpy(info->broadcast_name, (char*)data->data, | |
| MIN(sizeof(info->broadcast_name), data->len - 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ADV data is a string but without an ending '\0'.
Therefore we need the length data->len, which is exactly the length of the string, plus a space for '\0'.
| #ifdef CONFIG_BLUETOOTH_AURACAST_SINK | ||
| case BT_AD_BROADCAST_NAME: | ||
| strlcpy(info->broadcast_name, (char*)data->data, | ||
| MIN(sizeof(info->broadcast_name), data->len)); |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strlcpy function call specifies a maximum copy length using MIN(sizeof(info->broadcast_name), data->len). However, data->len includes the AD Type byte (1 octet), which should not be counted in the data length for the broadcast name. This means an extra byte from the next AD entry could be copied into the broadcast_name buffer. The correct length should be data->len - 1 to account for the AD Type byte that has already been excluded from data->data.
| MIN(sizeof(info->broadcast_name), data->len)); | |
| MIN(sizeof(info->broadcast_name), data->len > 0 ? data->len - 1 : 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data->len does not include the ending '\0', so we do need the orignal data to get the full string.
| typedef struct { | ||
| bt_address_t addr; | ||
| int8_t rssi; /* RSSI in dBm (-127, +20). 0x7F if unavailable */ | ||
| int8_t tx_power; /* transmit power of the advertiser in dBm (-127, +20). 0x7F if unavailable */ | ||
| uint8_t dev_type; /* bt_device_type_t */ | ||
| int8_t rssi; | ||
| uint8_t addr_type; /* ble_addr_type_t */ | ||
| uint8_t adv_type; /* ble_adv_type_t */ | ||
| uint8_t length; | ||
| uint8_t pad[1]; | ||
| uint8_t length; /* length of `adv_data` */ | ||
| uint16_t interval; /* periodic advertising interval in 1.25 microseconds, 0 if not presented */ | ||
| uint8_t sid; /* advertising set identifier, valid from 0x00 to 0x0F, 0xFF if not provided */ | ||
| uint8_t flags; /* e.g., `SCAN_RESULT_FLAG_PERIODIC_ADVERTISING` */ | ||
| uint8_t adv_data[1]; | ||
| } ble_scan_result_t; |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ble_scan_result_t structure has been extended with new fields (tx_power, interval, sid, flags), but the binder parcel serialization/deserialization functions in service/ipc/binder/parcel/parcel.c have not been updated to handle these fields. The AParcel_writeBleScanResult function at lines ~577-609 and AParcel_readBleScanResult at lines ~611-653 only serialize/deserialize the old fields. This will cause data loss and potential compatibility issues for binder-based IPC.
bug: v/82275 The event definition is necessary before the callbacks from periodic advertising sync. Signed-off-by: Zihao Gao <[email protected]>
e7905ee to
4f65d20
Compare
bug: v/82275 This patch defines the APIs required to synchronize a periodic advertising train. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82275 This patch includes the documentation of SAL APIs. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 A initial version that has nothing implemented. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 A initial version that starts compiling the files. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 Init and uninit for PA sync. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 Periodic advertising often comes from extended scanning, so we add an parser here. An alternative is Periodic Advertising Sync Transfer (to be implemented later). Signed-off-by: Zihao Gao <[email protected]>
bug: v/82380 Initial version of Auracast sink test tool Signed-off-by: Zihao Gao <[email protected]>
bug: v/82380 Add scan command to search active Auracast sources. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 Parse the scan result at the client side, so we can reduce the uplink data. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82275 Add TxPower, Periodic Advertising Interval, and correct the conversion of adv_type. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 Parse scan result at bttol to check if there is an Auracast source available. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 Skip logs that are not available. Add more debug logs on error. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82379 Allow to stop scanning for periodic advertisers. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82820 Add an extra flag to indicate whether periodic advertising is present in the scan result. Signed-off-by: Zihao Gao <[email protected]>
bug: v/82820 Modify message codes so its safe when client and server have different version of codes. Signed-off-by: Zihao Gao <[email protected]>
d7c9c60
No description provided.