Skip to content

Conversation

@lylezhu2012
Copy link
Contributor

Extend the function bt_sdp_discover to support service search transaction and service attribute transaction.

Improve the session->rec_buf. If the net buffer cannot be allocated from the channel, disconnect the SDP session.

Set the MaximumAttributeByteCount of the request SDP_SERVICE_SEARCH_ATTR_REQ with the tail room of session->rec_buf.

Set the MaximumAttributeByteCount of the request SDP_SERVICE_ATTR_REQ with the tail room of session->rec_buf.

Set the MaximumServiceRecordCount of the request SDP_SERVICE_SEARCH_REQ according to the tail room of session->rec_buf.

Handle the response code SDP_SERVICE_SEARCH_RSP, and SDP_SERVICE_ATTR_RSP.

Handle the error SDP_ERROR_RSP. Start the next SDP discovery if the error received.

If there no more request, disconnect the session.

If the request cannot be sent, start the next SDP discovery.

@zephyrbot zephyrbot added area: Bluetooth area: Bluetooth Classic Bluetooth Classic (BR/EDR) area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Samples Samples labels Sep 23, 2024
@hermabe hermabe removed their request for review September 23, 2024 11:24
@Thalley Thalley removed their request for review September 23, 2024 11:52
@jhedberg
Copy link
Member

What's with the super short commit message lines? :) It seems like you've set your editor to wrap the lines at 49 characters? Zephyr CI should be happy up to 74, so please update your editor configuration accordingly!

@lylezhu2012
Copy link
Contributor Author

What's with the super short commit message lines? :) It seems like you've set your editor to wrap the lines at 49 characters? Zephyr CI should be happy up to 74, so please update your editor configuration accordingly!

Sure. I didn't realize this would be a problem.

Comment on lines 1959 to 1960
Copy link
Member

Choose a reason for hiding this comment

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

Minor: merge these to just one line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is done.

Comment on lines 1970 to 1971
Copy link
Member

Choose a reason for hiding this comment

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

I think these can also be merged to a single line, without violating the 100 max line length. That said, whenever you do have to split the lines, please align the split line so that it starts right after the opening parenthesis of the previous line (N tabs + 0-7 spaces)

Copy link
Contributor Author

@lylezhu2012 lylezhu2012 Nov 11, 2024

Choose a reason for hiding this comment

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

Yes. It is done.

Comment on lines 1991 to 1995
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is done.

Comment on lines 2056 to 2057
Copy link
Member

Choose a reason for hiding this comment

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

Incorrectly aligned line, which makes the subsequent LOG_DBG statement indistinguishable from being part of the if-condition itself. However, you can probably just merge here too. You'll note that the clang-format checker is actually suggesting you to do exactly this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I updated all according to the suggestion of clang-format checker.

Comment on lines 2080 to 2084
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is done.

@jhedberg
Copy link
Member

@lylezhu2012 I posted some small coding style comments. They're generally minor issues, but since the Zephyr 4.0 code freeze will anyway prevent this PR from getting merged until next week (post-4.0) you might as well fix those now.

Extend the function `bt_sdp_discover` to support service search
transaction and service attribute transaction.

Improve the `session->rec_buf`. If the net buffer cannot be allocated
from the channel, disconnect the SDP session.

Set the `MaximumAttributeByteCount` of the request
`SDP_SERVICE_SEARCH_ATTR_REQ` with the tail room of `session->rec_buf`.

Set the `MaximumAttributeByteCount` of the request `SDP_SERVICE_ATTR_REQ`
with the tail room of `session->rec_buf`.

Set the `MaximumServiceRecordCount` of the request
`SDP_SERVICE_SEARCH_REQ` according to the tail room of
`session->rec_buf`.

Handle the response code `SDP_SERVICE_SEARCH_RSP`, and
`SDP_SERVICE_ATTR_RSP`.

Handle the error `SDP_ERROR_RSP`. Start the next SDP discovery if the
error received.

If there no more request, disconnect the session.

If the request cannot be sent, start the next SDP discovery.

Signed-off-by: Lyle Zhu <[email protected]>
Since the function `bt_sdp_discover` has been updated, the caller of
function needs to be updated to avoid the building and functionality
fault.

Add set the parameter `type` to value
`BT_SDP_DISCOVER_SERVICE_SEARCH_ATTR`.

Update the SDP discovery callback function. Make it align with
`bt_sdp_discover_func_t`.

Signed-off-by: Lyle Zhu <[email protected]>
Add set the parameter `type` to value
`BT_SDP_DISCOVER_SERVICE_SEARCH_ATTR`.

Update the SDP discovery callback function by adding a third parameter
`const struct bt_sdp_discover_params *params`. Make it align with
`bt_sdp_discover_func_t`.

Signed-off-by: Lyle Zhu <[email protected]>
goto iterate;
}

net_buf_add_mem(session->rec_buf, buf->data, record_len);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to consider the peer doesn't comply with the agreement, and response more data? Add if here to check whether session->rec_buf is overwritten.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kartben kartben merged commit 2b27b1c into zephyrproject-rtos:main Dec 6, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Bluetooth Classic Bluetooth Classic (BR/EDR) area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth area: Samples Samples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants