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

Adds support for ble logging characteristic #616

Merged
merged 15 commits into from
Jun 30, 2024
Merged

Conversation

thebentern
Copy link
Contributor

Closes #609
image

@geeksville
Copy link
Member

ooh this is super useful!

@geeksville
Copy link
Member

geeksville commented Jun 29, 2024

btw - @thebentern no need for you to try and fix the build errors which were reported. (by coincidence) When I recently tried using --ble it seems to me that the old version of the "Bleak" BLE library that were using had problems now connecting to meshtastic devices (at least on linux). But the new/current version has a number of breaking API changes. My WIP branch to make the current Bleak work for us is here:

https://github.com/geeksville/Meshtastic-python/tree/pr-fixbluetooth

I'll finish that work up and send a PR in. Or if it is okay with you I can merge it into this PR?

@thebentern
Copy link
Contributor Author

Sure!

on linux breaks all but the first connection attempts.
Also remove unneeded event stuff and arbitrary timeouts, better just to
use thread.join()
@geeksville
Copy link
Member

@thebentern okay - fixes to make bleak work robustly again on Linux are in. Would you mind trying on OS-X to confirm I didn't break anything there?

@geeksville
Copy link
Member

oops - I forgot to run pylint before submitting. fixing.

@geeksville
Copy link
Member

btw: note: I added an optional --ble-dest flag to specify target device. --ble now takes no args and if no device is specified and we can only find one paired meshtastic device, we just use that.

@thebentern
Copy link
Contributor Author

@geeksville I'm having some environment issues on my OS-X machine right now but it tested out fine on my Windows box, so I say ship it!

It turns out that Bleak is kinda racey.  If we call disconnect()
and then immediately close() the disconnect may or may not actually happen
(probably because it was merely queued for dbus).
So instead: When we want to close the BLEInterface we call disconnect()
and then in a preregistered 'on disconnect' handler we actually close
down our interface/datastructures.
Copy link

codecov bot commented Jun 29, 2024

Codecov Report

Attention: Patch coverage is 29.00000% with 71 lines in your changes missing coverage. Please review.

Project coverage is 61.92%. Comparing base (18c2d08) to head (5c2851d).

Files Patch % Lines
meshtastic/ble_interface.py 29.89% 68 Missing ⚠️
meshtastic/__main__.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #616      +/-   ##
==========================================
- Coverage   62.04%   61.92%   -0.13%     
==========================================
  Files          14       14              
  Lines        3009     3015       +6     
==========================================
  Hits         1867     1867              
- Misses       1142     1148       +6     
Flag Coverage Δ
unittests 61.92% <29.00%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@geeksville
Copy link
Member

heh - I'm not sure how to fix the sentry thing. Because I kinda doubt that sentry had any real code coverage testing on the existing (buggier) BLE code ;-)

Copy link
Member

@geeksville geeksville left a comment

Choose a reason for hiding this comment

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

LGTM

@ianmcorvidae
Copy link
Contributor

Nice, looks good to me!

@ianmcorvidae ianmcorvidae merged commit 4f1ea5b into master Jun 30, 2024
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for meshtastic logging over BLE
3 participants