Skip to content

Conversation

@szymon-czapracki
Copy link
Contributor

@szymon-czapracki szymon-czapracki commented Apr 12, 2022

Add Immediate Alert Service client.
Move IAS into separate subdirectory.
Add IAS shell.

fixes #44055

@github-actions github-actions bot added area: API Changes to public APIs area: Bluetooth area: Bluetooth Host Bluetooth Host (excluding BR/EDR) labels Apr 12, 2022
@szymon-czapracki
Copy link
Contributor Author

@MariuszSkamra fyi

@szymon-czapracki szymon-czapracki force-pushed the ias_client branch 3 times, most recently from e4ba8a2 to e978cc5 Compare April 14, 2022 12:32
@szymon-czapracki szymon-czapracki marked this pull request as ready for review April 19, 2022 06:06
Comment on lines 140 to 194
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 reset the client on disconnect? Would it make more sense to reset the client when bt_ias_discover is called, so that we don't perform any operations on disconnecting connections that never did any IAS stuff?

Copy link
Contributor Author

@szymon-czapracki szymon-czapracki Apr 25, 2022

Choose a reason for hiding this comment

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

I've made a change so on discovery we are doing:
client->conn = bt_conn_ref(conn)
And then we check on disconnect whether client->conn isn't NULL, and if not - we call client_reset() with bt_conn_unref() within. Is this approach correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so.

Since we are not suing using the ias_client as a application struct, and we simply use the conn index for lookups, there's no reason to store the bt_conn in the ias client struct.

A much simpler approach is to simply call client_reset in the bt_ias_discover and remove the disconnected callback, as that doesn't really serve any purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we are doing reset in bt_ias_discover now can we mark this as resolved?

@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Apr 25, 2022
@szymon-czapracki szymon-czapracki force-pushed the ias_client branch 5 times, most recently from f399ef3 to cc88d5d Compare April 25, 2022 10:58
@szymon-czapracki szymon-czapracki marked this pull request as draft April 25, 2022 11:04
@szymon-czapracki szymon-czapracki marked this pull request as ready for review April 25, 2022 11:38
@szymon-czapracki szymon-czapracki force-pushed the ias_client branch 2 times, most recently from e939645 to e605606 Compare April 25, 2022 11:58
@szymon-czapracki
Copy link
Contributor Author

@Thalley @MariuszSkamra I've added another commit with shell implementation for IAS within this PR, if you could take a look at it with your next review that would be great :)

Thalley
Thalley previously approved these changes Sep 8, 2022
Implement Immediate Alert Client
Move IAS into separate subdirectory

Signed-off-by: Szymon Czapracki <[email protected]>

general fixes
This commit adds IAS functionality to the shell.

Signed-off-by: Szymon Czapracki <[email protected]>

shell fixes
Add babblesim tests for IAS.

Signed-off-by: Szymon Czapracki <[email protected]>
@carlescufi
Copy link
Member

@jhedberg is away this week so merging to get this in time for the release.

@carlescufi carlescufi merged commit 1e4960b into zephyrproject-rtos:main Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: API Changes to public APIs area: Bluetooth Audio area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Immediate alert client

6 participants