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

Breaking changes: persistent dbus connections and configurable timeouts #21

Merged
merged 8 commits into from
Oct 31, 2018

Conversation

bsphere
Copy link
Contributor

@bsphere bsphere commented Oct 23, 2018

this PR includes some changes and improvements that we needed for using the crate;

  • BluetoothSession is used as a persistent connection to dbus, instead of opening a different connection for each method call, the session can be initialized with an optional dbus path, which is added as add_match to the dbus connection (useful for filtering messages that are received via incoming() an example usage is in examples/test5.rs

  • the timeout for dbus method calls is passed as an argument, most commands are left with 1000 but some (like connect()) have it exposed all the way to make it controllable by the user.

  • BluetoothEvent is an enum for BlueZ events, BluetoothEvent::from() can be used to parse incoming messages into BluetoothEvent. the events that we needed are currently in the enum. usage example is also in examples/test5.rs

  • implemented some additional functionality - BluetoothGATTCharacteristic::acquire_write() and BluetoothGATTCharacteristic::acquire_notify() (inspired by code from the somewhere in the repository's issues) and BluetoothDiscoverySession::set_discovery_filter()

@LucaCerina
Copy link

+1 on acquire_notify functionality. Notification management is fundamental for any BLE application

@dati91
Copy link
Member

dati91 commented Oct 24, 2018

Hi there!

Wow, great job!

First of all, I'm not working on this project anymore, so it's really great to see that it's not dead yet :).
And I can't really try it to see that everything is OK, so I have to trust you on that one.
But I went through the code and it looks fine to me 👍.

My only request is that: can you make this into maybe 4-5 (or even more if you see that is necessary) meaningful commits? And also move the revision bump into a separate commit.

I hope that's not too much to ask, because I really love to merge this PR.

@bsphere
Copy link
Contributor Author

bsphere commented Oct 24, 2018 via email

@dati91
Copy link
Member

dati91 commented Oct 25, 2018

This repository is part of a larger ecosystem for Servo. And I'm not the owner of this project anymore, it's under @szeged now. I'm only maintaining it when necessary.

BluetoothDiscoverySession now uses a connection from BluetoothSession

filter only signals
@bsphere
Copy link
Contributor Author

bsphere commented Oct 27, 2018

I did the rebase, i don't have a version bump commit. is it good enough?

@dati91
Copy link
Member

dati91 commented Oct 31, 2018

Yes, thanks.

@dati91 dati91 merged commit d91f753 into szeged:master Oct 31, 2018
@bsphere
Copy link
Contributor Author

bsphere commented Nov 25, 2018

can you please publish a new version to crates.io ?

@dati91
Copy link
Member

dati91 commented Nov 26, 2018

It's already published as 0.4.0.

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.

3 participants