zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.44k stars 6.4k forks source link

[nRF52840 Dongle][Bluetooth/Peripheral] CTS : Notify/Read and HRM : Write features are not working #73503

Closed SridhaKesa closed 1 week ago

SridhaKesa commented 3 months ago

Describe the bug Read and Notify Feature of Current Time Service(CTS) & Write Feature of Heart Rate Service's characteristic Heart Rate control Point is not working

Please also mention any information which could help others to understand the problem you're facing:

To Reproduce Steps to reproduce the behavior:

  1. Build the application Bluetooth: Peripheral using the command : west build -b nrf52840dongle/nrf52840 samples\bluetooth\peripheral
  2. Generate the package using the command : nrfutil pkg generate --hw-version 52 --sd-req=0x00 --application build/zephyr/zephyr.hex --application-version 1 peripheral.zip
  3. Now Reset the board into the Nordic bootloader by plugging the board in USB port while pressing the RESET button. The Red LED should start a fade pattern, signalling the bootloader is running.
  4. Now Program the application to the Target board : nRF52840 using the command : nrfutil dfu usb-serial -pkg peripheral.zip -p <COM Port>
  5. Wait for the application to boot completely and boot logs are displayed in Teraterm application via UART.
  6. Once the application starts, board starts advertising with name : Zephyr Peripheral Sample Long Name
  7. Scan for the advertisements using Client app like AIROC Bluetooth Connect Application installed on Android/iPhone.
  8. Using the app, initiate connection between Smartphone and board.
  9. Initiate pairing process, by performing Read/Write/Notify operation from any of the Characteristic listed under the unknown service with ID : 12345678-1234-5678-1234-56789abcdef0
  10. Issue 1 - CTS Notify: Open CTS Service from GATT DB and click Notify option available in service to start notification. Issue can be observed now
  11. Issue 2 - CTS Read: Open CTS Service from GATT DB and perform read operation before and after writing a value using Read option
  12. Issue 3 - HRM Write: Open HRM Service from GATT DB and Write any value using Write option available in Heart Rate Control Point service. Issue can be observed now.

Expected behavior Issue 1 - CTS Notify
Like BAS & HRS, for CTS also DUT should start sending notifications to the app installed on smartphone and PUART trace specifying the notification status should be printed. Issue 2 - CTS Read CTS Value should change periodically as its representing time. Issue 3 - HRM Write Write operation should be successful

Observation Issue 1 - CTS Notify: DUT does not start sending notifications to the app installed on smartphone and PUART trace specifying the notification status is not printed. Issue 2 - CTS Read: Same CTS Value is printed on performing read operation for multiple time CTS Value before performing write Operation: df07051e0c2d1e010000 CTS Value after performing write Operation: 1e0c2d1e010000 eg : write value : 0aob0c -> Read Value : 0a0b0c1e0c2d1e010000 Issue 3 - HRM Write: Write operation is not successful with error message. Error Message Android - Error occurred in writing data. Error Code : 3. Please Try again iOS - Error occurred in writing data. Error : Writing is not Permitted. Please Try again

Note : Please refer Screen Recordings attached.

Impact Annoyance - As unable to utilize the features of Current Time Service and Heart Rate Service

Logs and console output Attached Screen recording and Logs like Console logs from Teraterm & Ellisys Air Logs

Environment (please complete the following information):

Attachment nRF52840Dongle_Logs_ScreenRecording.zip

github-actions[bot] commented 3 months ago

Hi @SridhaKesa! We appreciate you submitting your first issue for our open-source project. šŸŒŸ

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. šŸ¤–šŸ’™

alwa-nordic commented 3 months ago

I believe the issue is that the services in question are only partially implemented.

E.g. the Heart Rate Service Control Point has no write handler: https://github.com/zephyrproject-rtos/zephyr/blob/4d0ef142f3a3391be6e6d7fed0bbef2eda33c5ee/subsys/bluetooth/services/hrs.c#L92-L94

The Current Time Service is indeed just mocking a static value. But I'm not sure if there is purpose to yet-another demonstration of a varying characteristic in the samples folder. I'm not even sure what technique cts.c demonstrates that is not covered elsewhere. https://github.com/zephyrproject-rtos/zephyr/blob/79e6b0e0f679b1c914352734d67b4a6da14eb465/samples/bluetooth/peripheral/src/cts.c#L68-L93

The lack of notifications from CTS follows from the time value not changing. https://github.com/zephyrproject-rtos/zephyr/blob/79e6b0e0f679b1c914352734d67b4a6da14eb465/samples/bluetooth/peripheral/src/cts.c#L101-L105

@kapi-no @ppryga-nordic, can you assist in triaging this issue?

My recommendation is to remove cts.c from the sample. If write-ability of the Control Point for HRS is optional according to spec, then we should explain that in a code comment.

jori-nordic commented 1 week ago

@cx-anuj-pathak did you check that your PR actually fixes this issue? ie use the reproducing steps on top of your branch and check that the issue is not present.

cx-anuj-pathak commented 1 week ago

@cx-anuj-pathak did you check that your PR actually fixes this issue? ie use the reproducing steps on top of your branch and check that the issue is not present.

jori-nordic commented 1 week ago

@cx-anuj-pathak thanks for testing thoroughly.

I think you should do the HRM fixes in another PR as you said.

I made https://github.com/zephyrproject-rtos/zephyr/issues/77595 and https://github.com/zephyrproject-rtos/zephyr/issues/77596 to track the two issues separately.

PS: For linking an issue, github wants exactly that text Fixes #issuenumber . Else it will not make the link. Note that you can also manually link them, using the UI (right pane, under "development")

edit: I will close this ticket when the two issues are fixed:

jori-nordic commented 1 week ago

Thanks for taking care of this @cx-anuj-pathak !