sys/stdio_nimble: add version note to README by crasbe · Pull Request #21085 · RIOT-OS/RIOT

crasbe · 2024-12-13T15:45:28Z

Contribution description

In the latest release of ble-serial, a new command line option --write-with-response was added and the default value is False.
(See Jakeler/ble-serial#109 and https://github.com/Jakeler/ble-serial/releases/tag/v3.0.0 ).
This will lead to the following error when trying to connect to the test/sys/shell_ble example or any application that uses the stdio_nimble module.

~/RIOTdev/RIOT/tests/sys/shell_ble$ ble-serial -d DA:ED:02:2A:35:CE -p /tmp/dev_riot_ble --write-uuid ccdd113f-40d5-4d68-86ac-a728dd82f4aa --read-uuid 35f28386-3070-4f3b-ba38-27507e991762
16:46:15.471 | INFO | linux_pty.py: Port endpoint created on /tmp/dev_riot_ble -> /dev/pts/1
16:46:15.471 | INFO | ble_client.py: Receiver set up
16:46:17.565 | INFO | ble_client.py: Trying to connect with DA:ED:02:2A:35:CE: tests/sys/shell/_ble
16:46:17.973 | WARNING | ble_client.py: Device DA:ED:02:2A:35:CE disconnected
16:46:17.974 | INFO | ble_client.py: Stopping Bluetooth event loop
16:46:19.446 | INFO | ble_client.py: Device DA:ED:02:2A:35:CE connected
16:46:19.446 | ERROR | main.py: No characteristic with ['write-without-response'] property found!
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/ble_serial/main.py", line 53, in _run
    await self.bt.setup_chars(args.write_uuid, args.read_uuid, args.mode, args.write_with_response)
  File "/usr/local/lib/python3.10/dist-packages/ble_serial/bluetooth/ble_client.py", line 44, in setup_chars
    self.write_char = self.find_char(write_uuid, write_cap)
  File "/usr/local/lib/python3.10/dist-packages/ble_serial/bluetooth/ble_client.py", line 84, in find_char
    assert len(results) > 0, \
AssertionError: No characteristic with ['write-without-response'] property found!
16:46:19.447 | WARNING | main.py: Shutdown initiated
16:46:19.447 | INFO | linux_pty.py: Serial reader and symlink removed
16:46:21.677 | WARNING | ble_client.py: Device DA:ED:02:2A:35:CE disconnected
16:46:21.677 | INFO | ble_client.py: Stopping Bluetooth event loop
16:46:21.678 | INFO | ble_client.py: Bluetooth disconnected
16:46:21.678 | INFO | main.py: Shutdown complete.

Using the new command line option makes the command work as expected:

~/RIOTdev/RIOT/tests/sys/shell_ble$ ble-serial -d DA:ED:02:2A:35:CE -p /tmp/dev_riot_ble --write-uuid ccdd113f-40d5-4d68-86ac-a728dd82f4aa --read-uuid 35f28386-3070-4f3b-ba38-27507e991762 --write-with-response
16:47:52.167 | INFO | linux_pty.py: Port endpoint created on /tmp/dev_riot_ble -> /dev/pts/1
16:47:52.167 | INFO | ble_client.py: Receiver set up
16:47:52.803 | INFO | ble_client.py: Trying to connect with DA:ED:02:2A:35:CE: tests/sys/shell/_ble
16:47:54.068 | INFO | ble_client.py: Device DA:ED:02:2A:35:CE connected
16:47:54.068 | INFO | ble_client.py: Found write characteristic ccdd113f-40d5-4d68-86ac-a728dd82f4aa (H. 14)
16:47:54.068 | INFO | ble_client.py: Found notify characteristic 35f28386-3070-4f3b-ba38-27507e991762 (H. 11)
16:47:54.409 | INFO | main.py: Running main loop!

Testing procedure

This is only a documentation change, but the test procedure would be as following:

Make sure you have the latest version of ble-serial installed. Otherwise you can upgrade it with pip install ble-serial --upgrade.
The latest version at the time of writing is 3.0.0.

$ pip show ble-serial
Name: ble-serial
Version: 3.0.0
Summary: Connects BLE adapters with virtual serial ports
Home-page: 
Author: 
Author-email: Jake <[email protected]>
License: MIT License
Location: /usr/local/lib/python3.10/dist-packages
Requires: bleak, coloredlogs
Required-by:

The test in tests/sys/shell_ble runs on all nRF52 devices (and probably others as well with NimBLE support). I used an nRF52840DK and our own hardware.
You can follow the procedure described in the text and should observe the aforementioned error when trying to connect to the device.

crasbe · 2024-12-13T15:46:28Z

Maybe there's a better name for the PR/commit, but I couldn't think of anything better. I'm open to suggestions :D

sys/stdio_nimble/README.md

benpicco · 2024-12-13T17:26:28Z

huh didn't know this file exist - I think it's contents should be moved to stdio_nimble.h so it shows up in the documentation

crasbe · 2024-12-13T20:36:46Z

huh didn't know this file exist - I think it's contents should be moved to stdio_nimble.h so it shows up in the documentation

Just as it is, basically Copy & Paste (with some deduplication of what's already there)?

crasbe · 2024-12-16T13:32:53Z

I don't like necessary line breaks in the code blocks, but otherwise the static tests will bonk me. I did not find a command how to add a line wrap to code blocks in Doxygen.

For example

Line 129-142:
...
 * ```
 * $ ble-serial -d 6BE8174C-A0F8-4479-AFA6-9828372CAFE9 -p /tmp/dev_riot_ble
 * --write-uuid ccdd113f-40d5-4d68-86ac-a728dd82f4aa
 * --read-uuid 35f28386-3070-4f3b-ba38-27507e991762 --write-with-response
 *
 * 17:44:18.765 | INFO | linux_pty.py: Slave created on /tmp/dev_riot_ble -> /dev/ttys006
 * 17:44:18.766 | INFO | ble_interface.py: Receiver set up
 * 17:44:18.766 | INFO | ble_interface.py: Trying to connect with 6BE8174C-A0F8-4479-AFA6-
 * 9828372CAFE9
 * 17:44:19.861 | INFO | ble_interface.py: Device 6BE8174C-A0F8-4479-AFA6-9828372CAFE9 connected
 * 17:44:19.862 | INFO | ble_interface.py: Found write characteristic ccdd113f-40d5-4d68-86ac-
 * a728dd82f4aa (H. 14)
 * 17:44:19.862 | INFO | ble_interface.py: Found notify characteristic 35f28386-3070-4f3b-ba38-
 * 27507e991762 (H. 11)
 * 17:44:19.883 | INFO | main.py: Running main loop!
 * ```
...

We have all this space in the resulting documentation and don't use it:

riot-ci · 2024-12-17T12:32:46Z

Murdock results

✔️ PASSED

9c44b7f tests/sys/shell_ble: update reference to instructions

Success	Failures	Total	Runtime
1	0	1	01m:15s

Artifacts

Documentation preview

mguetschow

Thanks for spotting and fixing the documentation!

One minor comment below, and tests/sys/shell_ble still references the (now deleted) README. That should be changed to point to the documentation instead.

sys/include/stdio_nimble.h

crasbe · 2024-12-19T12:34:40Z

Can I squash this?

mguetschow · 2024-12-21T18:43:20Z

Yes, please squash!

crasbe · 2025-01-07T14:19:11Z

Is there something left to do for this to be merged? It looks like this PR will not have any collisions with #21108 either, so it should be all good.

crasbe · 2025-01-08T10:03:57Z

Thanks everyone :)

crasbe requested a review from jia200x as a code owner December 13, 2024 15:45

github-actions bot added Area: doc Area: Documentation Area: sys Area: System labels Dec 13, 2024

benpicco reviewed Dec 13, 2024

View reviewed changes

sys/stdio_nimble/README.md Outdated Show resolved Hide resolved

mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 17, 2024

mguetschow reviewed Dec 17, 2024

View reviewed changes

sys/include/stdio_nimble.h Show resolved Hide resolved

crasbe requested review from MichelRottleuthner, aabadie and leandrolanzieri as code owners December 17, 2024 13:29

github-actions bot added the Area: tests Area: tests and testing framework label Dec 17, 2024

crasbe added 2 commits December 21, 2024 20:57

sys/stdio_nimble: move doc from README to header, add version note

cafcd01

tests/sys/shell_ble: update reference to instructions

9c44b7f

crasbe force-pushed the pr/sys/stdio_nimble branch from 6831f7b to 9c44b7f Compare December 21, 2024 20:03

benpicco approved these changes Jan 8, 2025

View reviewed changes

benpicco added this pull request to the merge queue Jan 8, 2025

Merged via the queue into RIOT-OS:master with commit 758a92f Jan 8, 2025

crasbe deleted the pr/sys/stdio_nimble branch January 16, 2025 10:06

MrKevinWeiss added this to the Release 2025.01 milestone Jan 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys/stdio_nimble: add version note to README#21085

sys/stdio_nimble: add version note to README#21085
benpicco merged 2 commits intoRIOT-OS:masterfrom
crasbe:pr/sys/stdio_nimble

crasbe commented Dec 13, 2024

Uh oh!

crasbe commented Dec 13, 2024

Uh oh!

Uh oh!

benpicco commented Dec 13, 2024

Uh oh!

crasbe commented Dec 13, 2024

Uh oh!

crasbe commented Dec 16, 2024

Uh oh!

riot-ci commented Dec 17, 2024 •

edited

Loading

Uh oh!

mguetschow left a comment

Uh oh!

Uh oh!

crasbe commented Dec 19, 2024

Uh oh!

mguetschow commented Dec 21, 2024

Uh oh!

crasbe commented Jan 7, 2025

Uh oh!

crasbe commented Jan 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

crasbe commented Dec 13, 2024

Contribution description

Testing procedure

Uh oh!

crasbe commented Dec 13, 2024

Uh oh!

Uh oh!

benpicco commented Dec 13, 2024

Uh oh!

crasbe commented Dec 13, 2024

Uh oh!

crasbe commented Dec 16, 2024

Uh oh!

riot-ci commented Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

mguetschow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crasbe commented Dec 19, 2024

Uh oh!

mguetschow commented Dec 21, 2024

Uh oh!

crasbe commented Jan 7, 2025

Uh oh!

crasbe commented Jan 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

riot-ci commented Dec 17, 2024 •

edited

Loading