Introduce next_event_async allowing to poll event queue
#224
Conversation
tnull
force-pushed
the
2024-01-async-events
branch
3 times, most recently
from
9e7c2b6
to
c7ac3cd
Compare
| /// Will asynchronously poll the event queue until the next event is ready. | ||
| /// | ||
| /// **Note:** this will always return the same event until handling is confirmed via [`Node::event_handled`]. | ||
| pub async fn next_event_async(&self) -> Event { |
There was a problem hiding this comment.
its worth following the naming convention with wait_next_event and call this async_next_event
also, Its a bit confusing to have wait and async as usually they both mean async something..
maybe wait_next_event should be sync_next_event?
There was a problem hiding this comment.
Mh, I'm not sure: wait_next_event is called that way to follow std::sync::Condvar's naming that indicates it's going to block the current thread. I disagree that wait and async "both mean async something" as blocking or not blocking the thread is a fundamental difference here.
That said, I'm generally also not the biggest fan of the _async suffix here as it's redundant to the actual return type/async keyword of the method. I considered poll_next_event as an alternative name for next_event_async, however, it may also be a bit misleading as the semantics of Future's poll are slightly different. As we also use the _async suffix for LDK's process_events_async I stuck with that for now. Generally I'm still open for better suggestions though, poll_next_event might be an alternative candidate.
There was a problem hiding this comment.
Thanks for the explanation.
I would also go with poll_ , would look better than async fn name_async().
future_next_event could be another option..
There was a problem hiding this comment.
I don't mind next_event_async, would also consider next_event_future.
tnull
force-pushed
the
2024-01-async-events
branch
2 times, most recently
from
9075a24
to
cf06e9c
Compare
|
Rebased on #230. |
tnull
force-pushed
the
2024-01-async-events
branch
6 times, most recently
from
48c161c
to
c43fb01
Compare
tnull
force-pushed
the
2024-01-async-events
branch
from
c43fb01
to
290a543
Compare
|
Rebased on main after #230 landed. |
We implement a way to asynchronously poll the queue for new events, providing an async alternative to `wait_next_event`.
.. which requires us to include a dependency on the `kotlinx-coroutines` package.
tnull
force-pushed
the
2024-01-async-events
branch
from
290a543
to
77dfa83
Compare
| /// Will asynchronously poll the event queue until the next event is ready. | ||
| /// | ||
| /// **Note:** this will always return the same event until handling is confirmed via [`Node::event_handled`]. | ||
| pub async fn next_event_async(&self) -> Event { |
There was a problem hiding this comment.
I don't mind next_event_async, would also consider next_event_future.
|
Going ahead with this for now, can always revisit the naming in the future (no pun intended). |
We implement a way to asynchronously poll the queue for new events, providing an async alternative to
wait_next_event.Bindings exposure is still blocked on the next UniFFI release.Now also exposed in bindings as UniFFI 0.26 has been released, now based on #230.