-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: BR: Add listener cb for discovery #78203
Bluetooth: BR: Add listener cb for discovery #78203
Conversation
1c69049
to
c95220e
Compare
/** private */ | ||
uint8_t _priv[4]; |
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.
I was just looking at how this is used, and it seems quite fragile. I'd recommend to clean it up in the same go. These four bytes are cast into the internally defined struct discovery_priv
, but there's not even any build assert to ensure that the sizes match up. I'd suggest to just put the private struct in the public header, but cleanly document that it's private data, i.e. something like:
struct bt_br_discovery_result {
struct bt_br_discovery_priv _priv;
...
};
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.
Updated.
@@ -38,10 +38,20 @@ extern "C" { | |||
* @{ | |||
*/ | |||
|
|||
/** @brief BR/EDR discovery private structure */ |
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.
Either drop the doxygen annotation from the comments, or mark this as hidden or private (there's dedicated oxygen syntax for it, IIRC)
uint8_t pscan_rep_mode; | ||
/** Resolving remote name*/ | ||
uint8_t resolving; | ||
} __packed; |
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.
I don't think you need the __packed
. It's also confusing, since this is normally reserved for protocol structs.
/** Page scan repetition mode */ | ||
uint8_t pscan_rep_mode; | ||
/** Resolving remote name*/ | ||
uint8_t resolving; |
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.
Should this be a bool
?
The results of inquiry and extended inquiry are only reported from application level after the inquiry complete event notified. While the event of inquiry result is notified by controller in real time. It is not a good user experience. Just like scanning of LE, add a listener cb for discovery. When the event of inquiry result, extended inquiry result, or remote name request complete notified, call listener `recv` cb to notify the upper layer. When the event of inquiry complete notified or no pending of remote name request , call listener `timeout` cb to notify the upper layer. Signed-off-by: Lyle Zhu <[email protected]>
Move source code of classic from bluetooth.h to ./classic/classic.h. Signed-off-by: Lyle Zhu <[email protected]>
Remove BR discovery callback from bt_br_discovery_start. All discovery results will be notified through callback registered by bt_br_discovery_cb_register. Signed-off-by: Lyle Zhu <[email protected]>
Due to the update of function bt_br_discovery_start, register discovery callback by calling bt_br_discovery_cb_register. Signed-off-by: Lyle Zhu <[email protected]>
Due to the update of function bt_br_discovery_start, register discovery callback by calling bt_br_discovery_cb_register. Signed-off-by: Lyle Zhu <[email protected]>
Move `struct discovery_priv` to classic.h Rename `struct discovery_priv` to `struct bt_br_discovery_priv`. Modify the structure `struct bt_br_discovery_priv` with `@private`. Change field `_priv` of `struct bt_br_discovery_result` from `uint8_t _priv[4]` to `struct bt_br_discovery_priv _priv`. Signed-off-by: Lyle Zhu <[email protected]>
The `resolving` is used to flag the status that the stack is requesting remote device name. It is better to use the type `bool` instead of the original type `uint8_t`. Signed-off-by: Lyle Zhu <[email protected]>
5f51f6d
to
9627db2
Compare
The old PR was closed due to the source branch has been deleted. Create a new PR for the changes.
The results of inquiry and extended inquiry are only reported from application level after the inquiry complete event notified.
While the event of inquiry result is notified by controller in real time.
It is not a good user experience.
Just like scanning of LE, add a listener cb for discovery.
When the event of inquiry result, extended inquiry result, or remote name request complete notified, call listener recv cb to notify the upper layer.
When the event of inquiry complete notified or no pending of remote name request , call listener timeout cb to notify the upper layer.