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

Support for BLE and BTP #155

Merged
merged 8 commits into from
Jun 28, 2024
Merged

Support for BLE and BTP #155

merged 8 commits into from
Jun 28, 2024

Conversation

ivmarkov
Copy link
Contributor

Currently runs on Linux/BlueZ only, but adding support for another BLE stack should be easy (user needs to implement the GattPeripheral trait - and - we might add out of the box BLE/GATT box implementations to rs-matter over time, as we did for mDNS).

Keeping as Draft for now, as the code needs to be tested on MCUs (doing this just now with ESP-IDF + Bluedroid) and needs unit tests.

@ivmarkov
Copy link
Contributor Author

Forgot to say: this code is all new in that it does not touch/change pre-existing metaphors. Everything necessary to implement pluggable protocols (today - BTP, tomorrow - TCP) had been merged already.

examples/onoff_light_bt/src/comm.rs Show resolved Hide resolved
examples/onoff_light_bt/src/comm.rs Outdated Show resolved Hide resolved
rs-matter/src/error.rs Show resolved Hide resolved
rs-matter/src/transport/network/btp/context.rs Outdated Show resolved Hide resolved
rs-matter/src/transport/network/btp/session.rs Outdated Show resolved Hide resolved
rs-matter/src/transport/network/btp/session.rs Outdated Show resolved Hide resolved
rs-matter/src/transport/network/btp/session.rs Outdated Show resolved Hide resolved
@ivmarkov
Copy link
Contributor Author

@andreilitvin Many thanks for the code review so far!

  • For your convenience, I took the freedom to mark with "Resolved" all comments of yours where I took your suggestions with no or minimal changes. Feel free to "unresolve" them if you would like to additionally comment or you still have concerns after looking at the code I did check-in
  • I've left those comments where I'm pushing back - two - if I'm not mistaken, where we disagree a bit on what the naming stuff around the GattPeripheral abstraction should be

@ivmarkov
Copy link
Contributor Author

Oh - I also pushed-back on our favorite "Copyright headers" topic from before (and did mark it as "Resolved"), but really - let's first agree on what to do, and then do a one-pass regexp on all of it. Until then, I keep all headers the same, so the regexp has an easier time operating.

ivmarkov and others added 8 commits June 27, 2024 14:11
Fix CI to install libdbus

Fix a few typos in doc comments

MAX_BTP_SESSIONS needs to be public

Clone for the callback

Compute MTU more precisely, also based on the reported GATT MTU

mut and ref standard impls for GattPeripheral

WIP: unit tests

More unit tests

Move unit tests to separate file

More unit tests work

More unit tests work

More unit tests work
Co-authored-by: Andrei Litvin <andy314@gmail.com>
Co-authored-by: Andrei Litvin <andy314@gmail.com>
@ivmarkov
Copy link
Contributor Author

@andy31415 What do you think we might still need to do for this PR to land?

There are a few open items (primarily w.r.t. naming) that I feel we should not really address, but please push back if you feel otherwise. Anything else outstanding?
No rush, take your time, but just wondering where we are.

@andy31415
Copy link
Contributor

CSA member meeting caused a bit of a delay. I expect to get some time for one more pass tomorrow, expect it to check mark it then. Sorry for the delay.

@andy31415 andy31415 merged commit 90412d7 into project-chip:main Jun 28, 2024
12 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.

None yet

2 participants