-
Notifications
You must be signed in to change notification settings - Fork 40
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
[#390] event based comm example #519
base: main
Are you sure you want to change the base?
[#390] event based comm example #519
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #519 +/- ##
==========================================
- Coverage 78.87% 78.86% -0.02%
==========================================
Files 201 201
Lines 24219 24220 +1
==========================================
- Hits 19103 19100 -3
- Misses 5116 5120 +4
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just some nitpicks.
What do you think about adding an FD based event to the example, e.g. for unix domain sockets.
self.notifier | ||
.notify_with_custom_event_id(PubSubEvent::SentHistory.into()) | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Philosophical question. Should the SentSample
event also be triggert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not distinguish this. It is a simple example and the history may consist of N samples but then only 1 SentSample
event was triggered which would be inconsistent.
faa7f5c
to
b46883b
Compare
97dcaad
to
dbf37d1
Compare
Notes for Reviewer
ExamplePublisher
orExampleSubscriber
that also contain an notifier/listener to wait or emit events that are related to them. It also provides a first preview how a high-level iceoryx2 API could look like, where the event notification and the message transport are no longer separated.WaitSet
where it went into an infinite loop after the first deadline/interval has passed. The only thing here missing was to store the last timemissed_deadlines
was called in the deadline queue.Pre-Review Checklist for the PR Author
SPDX-License-Identifier: Apache-2.0 OR MIT
iox2-123-introduce-posix-ipc-example
)[#123] Add posix ipc example
)task-list-completed
)Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Relates to #390
Closes #518