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: services: Move GATT services source files #17355

Merged
merged 6 commits into from
Jul 11, 2019

Conversation

joerchan
Copy link
Contributor

@joerchan joerchan commented Jul 4, 2019

Move the GATT services source files. Components that are re-used are moved into the bluetooth subsystem and Kconfig options are added to include them in the build.
Services that are only used once are moved into the sample that includes it.
The IPSP service was not in use so it was removed.

@joerchan joerchan changed the title Move GATT services source files Bluetooth: services: Move GATT services source files Jul 4, 2019
@zephyrbot zephyrbot added area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test labels Jul 4, 2019
@jhedberg
Copy link
Member

jhedberg commented Jul 4, 2019

In general I'm happy for the direction this is taking, however one thing to be aware of is that you're now introducing these as official public APIs. I think it might make sense to flag them as experimental for now, so we can spend some time seeing if they need any fine-tuning or not.

lemrey and others added 6 commits July 5, 2019 11:12
This commit moves the BLE GATT Battery service
from /samples/bluetooth/gatt to /subsys/bluetooth/services and
adds a Kconfig entry to enable and configure the service;
when enabled, it will register itself automatically.

Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
This commit moves the BLE GATT heart rate service from
samples/bluetooth/gatt to subsys/bluetooth/services and adds a Kconfig
entry to enable and configure the service.

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Move the health thermometer service into the sample folder that
demonstrates it. This avoids long build paths

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Move the Current Time service into the sample that demonstrates it.
This avoids long build paths

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Move the HID over GATT service into the sample that demonstrates it.
This avoids long build paths

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Delete IPSP sample file, this source file is not included in any build
files. The service contains no valuable logic other than advertising
with the IPSP service in the advertising data.

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
@joerchan
Copy link
Contributor Author

joerchan commented Jul 5, 2019

@jhedberg Good point. Which is also why I didn't move all of them, only the ones that was being re-used. I have marked BAS and HRS as Experimental (Hopefully that is the correct way to do it, I wasn't really sure).


menuconfig BT_GATT_BAS
bool "Enable GATT Battery service"
select SENSOR
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is no longer needed.


if BT_GATT_BAS

config BT_GATT_BAS_LOG_LEVEL
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can use the Kconfig template for this now.

Copy link
Collaborator

@lemrey lemrey left a comment

Choose a reason for hiding this comment

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

Perhaps then it'd be a good idea to give the user the option to not run bas_init during SYS_INIT, and make that API public?

@joerchan
Copy link
Contributor Author

joerchan commented Jul 5, 2019

Perhaps then it'd be a good idea to give the user the option to not run bas_init during SYS_INIT, and make that API public?

There is nothing happening in bas_init now. The service is registered as a static service.

@lemrey
Copy link
Collaborator

lemrey commented Jul 5, 2019

There is nothing happening in bas_init now. The service is registered as a static service.

Right, so my question is whether we want to make the bas_init public now so that we don't have to introduce it afterwards if something gets added to it, and there could be a reason for users to run it manually (not during SYS_INIT).

@joerchan
Copy link
Contributor Author

joerchan commented Jul 5, 2019

@lemrey I don't think that is necessary. And later if I'm wrong it should be no trouble to add it :)

@jhedberg jhedberg merged commit 99e8710 into zephyrproject-rtos:master Jul 11, 2019
@joerchan joerchan deleted the gatt-services branch September 24, 2019 11:37
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 area: Samples Samples area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants