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: SDP: fix 32 and 128-bit UUID configuration (IDFGH-10312) #11572

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

ilutchenko
Copy link
Contributor

The problem:

esp_sdp_create_record() can't create SDP records with 32 and 128-bit UUIDs.

Summary

Added proper conversion of 4 and 16-byte UUIDs values to binary streams.
UUIDs are now set with SDP_AddAttribute() instead of SDP_AddServiceClassIdList().

Changes were tested with the following code:

esp_bluetooth_sdp_record_t record = {};
                
record.hdr.type = ESP_SDP_TYPE_RAW;
record.hdr.uuid.len = 2;
record.hdr.uuid.uuid.uuid16 = 0xfefe;
record.hdr.service_name_length = strlen("16-bit UUID") + 1;
record.hdr.service_name = "16-bit UUID";
record.hdr.rfcomm_channel_number = BT_IAP2_RFCOMM;
record.hdr.l2cap_psm = BT_L2CAP_DYNMIC_PSM;
record.hdr.profile_version = BT_IAP2_PROFILE_VERSION;
esp_sdp_create_record(&record);

record.hdr.type = ESP_SDP_TYPE_RAW;
record.hdr.uuid.len = 4;
record.hdr.uuid.uuid.uuid32 = 0xfefefefe;
record.hdr.service_name_length = strlen("32-bit UUID") + 1;
record.hdr.service_name = "32-bit UUID";
record.hdr.rfcomm_channel_number = BT_IAP2_RFCOMM;
record.hdr.l2cap_psm = BT_L2CAP_DYNMIC_PSM;
record.hdr.profile_version = BT_IAP2_PROFILE_VERSION;
esp_sdp_create_record(&record);

static const uint8_t IAP2_UUID[] = {0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe,
                                    0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
               
record.hdr.type = ESP_SDP_TYPE_RAW;
record.hdr.uuid.len = sizeof(IAP2_UUID);
memcpy(record.hdr.uuid.uuid.uuid128, IAP2_UUID, sizeof(IAP2_UUID));
record.hdr.service_name_length = strlen("128-bit UUID") + 1;
record.hdr.service_name = "128-bit UUID";
record.hdr.rfcomm_channel_number = BT_IAP2_RFCOMM;
record.hdr.l2cap_psm = BT_L2CAP_DYNMIC_PSM;
record.hdr.profile_version = BT_IAP2_PROFILE_VERSION;
esp_sdp_create_record(&record);

sdptool scan outputs will be:

Service Name: 16-bit UUID
Service RecHandle: 0x10001
Service Class ID List:
  "" (0xfefe)
Protocol Descriptor List:
  "L2CAP" (0x0100)
  "RFCOMM" (0x0003)
    Channel: 1

Service Name: 32-bit UUID
Service RecHandle: 0x10001
Service Class ID List:
  "" (0xfefefefe)
Protocol Descriptor List:
  "L2CAP" (0x0100)
  "RFCOMM" (0x0003)
    Channel: 1

Service Name: 128-bit UUID
Service RecHandle: 0x10001
Service Class ID List:
  UUID 128: fefefefe-fefe-fefe-fefe-fefefefefefe
Protocol Descriptor List:
  "L2CAP" (0x0100)
  "RFCOMM" (0x0003)
    Channel: 1

Caveats

There are still some strange things like
memcpy(&service, &rec->hdr.bt_uuid.uuid.uuid16, sizeof(service));
I plan to rework them when I will fix #11528

@CLAassistant
Copy link

CLAassistant commented Jun 1, 2023

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jun 1, 2023
@github-actions github-actions bot changed the title Bluetooth: SDP: fix 32 and 128-bit UUID configuration Bluetooth: SDP: fix 32 and 128-bit UUID configuration (IDFGH-10312) Jun 1, 2023
} else if (rec->hdr.bt_uuid.len == 2) {
memcpy(&service, &rec->hdr.bt_uuid.uuid.uuid16, sizeof(service));
UINT8_TO_BE_STREAM (p_temp, (UUID_DESC_TYPE << 3) | SIZE_TWO_BYTES);
UINT16_TO_BE_STREAM (p_temp, rec->hdr.bt_uuid.uuid.uuid16);
} else if (rec->hdr.bt_uuid.len == 4) {
memcpy(&service, &rec->hdr.bt_uuid.uuid.uuid16, sizeof(service));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to convert 32 bit or 128 bit uuid to 16 bit uuid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!
What is the purpose of converting them to 16-bit UUID?

I dug down and realized that we must patch bta_sys_add_uuid(UINT16 uuid16) and other functions in the chain to support 32 and 128 bit UUIDs as well.
I will do that to fix the issue #11529 , but it exceeds this pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we don't need to modify bta_sys_add_uuid. Just need to convert them to 16 bit uuid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not the topic of this PR, but I want to explain myself:

Bluedroid in some places has the ability to use 128-bit UUID.
For example, in the bta_dm_set_eir() code checks for the 128-bit UUIDs.
But the problem is that these 128 and 32-bit UUIDs are nowhere saved in the bta_dm_cb.custom_uuid[] array. Only 16-bit values are stored.
That's why I want to modify bta_sys_add_uuid one day.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right. In order to support 128 and 32-bit UUIDs need to modify bta_sys_add_uuid.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jun 12, 2023
} else {
SDP_DeleteRecord(sdp_handle);
sdp_handle = 0;
return sdp_handle;
}
/* add service class */
status &= SDP_AddServiceClassIdList(sdp_handle, 1, &service);
status &= SDP_AddAttribute (sdp_handle, ATTR_ID_SERVICE_CLASS_ID_LIST,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete the trailing-whitespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very sorry, maybe I expressed it wrong. It is necessary to delete the space behind ,.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, got it.
Fixed!

} else if (rec->hdr.bt_uuid.len == 2) {
memcpy(&service, &rec->hdr.bt_uuid.uuid.uuid16, sizeof(service));
UINT8_TO_BE_STREAM (p_temp, (UUID_DESC_TYPE << 3) | SIZE_TWO_BYTES);
UINT16_TO_BE_STREAM (p_temp, rec->hdr.bt_uuid.uuid.uuid16);
} else if (rec->hdr.bt_uuid.len == 4) {
memcpy(&service, &rec->hdr.bt_uuid.uuid.uuid16, sizeof(service));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we don't need to modify bta_sys_add_uuid. Just need to convert them to 16 bit uuid.

Added proper conversion of 4 and 16-byte UUIDs values to binary streams.
UUIDs now set with SDP_AddAttribute() instead of
SDP_AddServiceClassIdList().
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress labels Jun 28, 2023
@espressif-bot espressif-bot merged commit 52071f4 into espressif:master Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bluetooth: Extended Inquiry Request: can't set service UUIDs (IDFGH-10266)
4 participants