-
Notifications
You must be signed in to change notification settings - Fork 388
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
iox-#751 create unnamed semaphore #1367
iox-#751 create unnamed semaphore #1367
Conversation
@elBoberido @FerdinandSpitzschnueffler For the sake of efficient reviews I created this PR without having tests. The implementation is complete, could you please take a look at the overall design and when you agree with it I would add the tests and documentation to the proposed classes. It would be sufficient to review |
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 in general
iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp
Outdated
Show resolved
Hide resolved
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.
LGTM
1a7aa4c
to
311de8f
Compare
e345581
to
7d5ece3
Compare
Codecov Report
@@ Coverage Diff @@
## master #1367 +/- ##
==========================================
- Coverage 78.82% 78.65% -0.18%
==========================================
Files 373 376 +3
Lines 14680 14765 +85
Branches 2053 2065 +12
==========================================
+ Hits 11572 11613 +41
- Misses 2433 2471 +38
- Partials 675 681 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
0a4a4dd
to
d4fa175
Compare
iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp
Outdated
Show resolved
Hide resolved
iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp
Outdated
Show resolved
Hide resolved
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.
Just a few small things
iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/semaphore.hpp
Outdated
Show resolved
Hide resolved
iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp
Outdated
Show resolved
Hide resolved
iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp
Outdated
Show resolved
Hide resolved
iceoryx_hoofs/test/moduletests/test_posix_unnamed_semaphore.cpp
Outdated
Show resolved
Hide resolved
iceoryx_hoofs/test/moduletests/test_posix_unnamed_semaphore.cpp
Outdated
Show resolved
Hide resolved
iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp
Outdated
Show resolved
Hide resolved
iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
…semaphore platform does not overflow Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
… thread in unit test Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
…tState with getValue for semaphore Signed-off-by: Christian Eltzschig <me@elchris.org>
Signed-off-by: Christian Eltzschig <me@elchris.org>
81655a7
to
ac6e768
Compare
Pre-Review Checklist for the PR Author
iox-#123-this-is-a-branch
)iox-#123 commit text
)git commit -s
)task-list-completed
)Notes for Reviewer
At the moment our semaphore wrapper has the following issues:
This PR lays the foundation for two classes
UnnamedSemaphore
andNamedSemaphore
which will both inherit fromSemaphoreInterface
which provides methods likepost
,wait
,timedWait
etc.They won't be moveable anymore and use the builder pattern for creation.
test_posix_semaphore_interface.cpp
were taken by copy fromtest_semaphore_module.cpp
and cleaned up a bit.Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References