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: CCP: Introduce new CCP API #77120

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Thalley
Copy link
Collaborator

@Thalley Thalley commented Aug 15, 2024

The CCP API for the Call Control Profile works on top of the
TBS API, and will eventually replace parts of the TBS API.

Signed-off-by: Emil Gydesen [email protected]

@Thalley Thalley force-pushed the ccp_register branch 3 times, most recently from 6aa67a6 to 63398a4 Compare August 21, 2024 09:31
@Thalley Thalley changed the title Ccp register Bluetooth: CCP: Introduce new CCP API Aug 21, 2024
@Thalley Thalley force-pushed the ccp_register branch 6 times, most recently from 4599b32 to fc5da98 Compare August 26, 2024 19:05
Comment on lines 66 to 71
int bt_ccp_server_register_bearer(const struct bt_tbs_register_param *param,
struct bt_ccp_server_bearer **bearer);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For reviewers: I consider the following naming options:

  1. bt_ccp_ccs (Bluetooth Call Control Profile Call Control Server, where Call Control Server is the actual name of the role in CCP)
  2. bt_ccp_call_control_server (more verbose version of the above; naming similar to e.g. bt_bap_unicast_server)
  3. bt_ccp_call_ctrl_srv (shorter version of 3, more similar to the VCP and MICP APIs (e.g. bt_vcp_vol_rend)
  4. bt_ccp_server (and in upcoming PRs, bt_ccp_client).

If you have any preferences or other suggestions, please let me know and we can just do a rename

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer 3. bt_ccp_call_control_server. It is easier to spelling and understand.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your input - Other reviews please also comment here about what you prefer

Copy link
Collaborator

Choose a reason for hiding this comment

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

No strong preferences, but I do in general prefer longer, more readable names.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@koffes is that a vote for 3) then?

Copy link
Collaborator

@koffes koffes Nov 27, 2024

Choose a reason for hiding this comment

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

Yes, 4 is good as well.

@Thalley Thalley force-pushed the ccp_register branch 4 times, most recently from 87437d8 to 1e8dcd7 Compare September 9, 2024 09:45
@Thalley Thalley force-pushed the ccp_register branch 6 times, most recently from 87a92cf to 3c73258 Compare September 17, 2024 08:56
@Thalley Thalley force-pushed the ccp_register branch 9 times, most recently from 0ede37d to c3c5f3f Compare September 24, 2024 16:16
@Thalley Thalley added this to the 4.1 milestone Oct 22, 2024
@erwango erwango modified the milestones: 4.1, v4.1.0 Oct 24, 2024
@Thalley Thalley force-pushed the ccp_register branch 3 times, most recently from 90f3646 to c53b52c Compare November 12, 2024 10:55
@hermabe hermabe removed their request for review November 12, 2024 11:08
gtbs}>]


In the following examples, notifications from GTBS is ignored, unless otherwise
Copy link
Collaborator

@kruithofa kruithofa Nov 22, 2024

Choose a reason for hiding this comment

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

Suggested change
In the following examples, notifications from GTBS is ignored, unless otherwise
In the following examples, notifications from GTBS are ignored, unless otherwise

This was also wrong in the original document, but suggest to fix it here

********

Application demonstrating the CCP Server functionality.
Starts by advertising for a CCP Clients to connect and set up calls.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Starts by advertising for a CCP Clients to connect and set up calls.
Starts by advertising for CCP Clients to connect and set up calls.

************

* BlueZ running on the host, or
* A board with Bluetooth Low Energy 5.2 support
Copy link
Contributor

Choose a reason for hiding this comment

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

5.3?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

5.3 is not required :)

}
}

if (adv != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe make sure adv is 0 on init

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's static :)

@Thalley Thalley force-pushed the ccp_register branch 2 times, most recently from 067ce29 to 2563cf7 Compare November 26, 2024 12:57
@Thalley
Copy link
Collaborator Author

Thalley commented Nov 26, 2024

Renamed to Call Control Profile Call Control Server as per #77120 (comment) and short discussion in the weekly LE Audio meeting

@Thalley Thalley force-pushed the ccp_register branch 2 times, most recently from fc12d63 to d5b6e68 Compare November 27, 2024 18:38
The CCP API for the Call Control Profile works on top of the
TBS API, and will eventually replace parts of the TBS API.

Signed-off-by: Emil Gydesen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Audio area: Bluetooth Host area: Bluetooth Qualification Bluetooth Qualification -related issues and pull requests area: Bluetooth area: Process area: Samples Samples platform: nRF BSIM Nordic Semiconductors, nRF BabbleSim
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

9 participants