l2cap: Linux Support #39
Conversation
|
Instead of exposing the underlying platform-specific type, I think I'd rather have an optional implementation of the Tokio traits. I'm ok with an optional dependency on Tokio on other platforms. |
This also modifies the public api to rely on `AsyncWrite::poll_close` and `Drop` for closing the l2cap channels instead of having bespoke functions for it
I may want to add the feature in a future PR. For now, the function has been removed. |
|
Android ci is blocked until #41 is merged. |
| /// Close the L2CAP channel. | ||
| /// | ||
| /// This closes the entire channel, in both directions (reading and writing). | ||
| /// | ||
| /// The channel is automatically closed when `L2capChannel` is dropped, so | ||
| /// you don't need to call this explicitly. | ||
| #[inline] | ||
| pub async fn close(&mut self) -> Result<()> { | ||
| self.writer.close().await | ||
| } |
There was a problem hiding this comment.
We need to keep this function here and on the reader/writer halves. It's the only way to asynchronously wait for the close to complete and get any error related to closing the channel.
There was a problem hiding this comment.
Though, I think it should be changed to return a std::io::Result like the rest of the l2cap functions.
There was a problem hiding this comment.
Do we need to keep it on the writers? AsyncWriteExt::close exists.
There was a problem hiding this comment.
The problem with creating close functions for the reader is some platforms dont support that behavior. The OwnedReadHalf on linux does not seem to have a close function that closes the writer half.
There was a problem hiding this comment.
Ok, let's remove it from the non-writers on all platforms then.
There was a problem hiding this comment.
I think I did. What do you think I missed.
src/lib.rs
Outdated
| pub mod error; | ||
|
|
||
| #[cfg(feature = "l2cap")] | ||
| #[macro_use] |
There was a problem hiding this comment.
I'd prefer using pub(crate) ... instead of #[macro_use]
There was a problem hiding this comment.
TIL you could use pub(crate) use macro_name to scope macro definitions instead of #[macro_use]. Thanks for letting me know about this. Now, I just wish you could use pub(crate) macro_rules!
``` warning: the following packages contain code that will be rejected by a future version of Rust: bluer v0.16.1 note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1` ```
|
@alexmoon Is there anything I should be doing to get this merged? As far as I can tell, I have addressed all the comments. |
|
Sorry for the delay. Looks good to me. |
2 participants
Adds Linux L2Cap Support.
bluerusestokio::io::{AsyncRead, AsyncWrite}instead offutires_io::{AsyncRead, AsyncWrite}, so to maintain cross-platform compatibility, I wrap the bluer stream inside anCompat.