Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 #454 create test file for iceoryx roudi and roudi app #587
Iox #454 create test file for iceoryx roudi and roudi app #587
Changes from 13 commits
70f6244
753db4d
4422aef
8e305fd
cdf198d
1b48a05
6fb2542
34f0b6f
e13a042
9ca6ce5
36e4c15
88f2286
8c71388
057dcc8
24a819f
281887a
0caad32
7a1d57e
7eee047
c71382c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think we do not want this kind of behavior or is there a use case that the
RoudiApp
should be instantiated twice in the same process?! @budrusThere 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 guess this come from the current implementation. The
RouDiLock
is located inIceOryxRouDiComponents
.The Components are only instantiated when you call
IceOryxRouDiApp::run()
. Here in this case only the class is instantiated... Maybe the lock should be relocated to fire earlier.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.
Well, nothing bad happens until
run
is called, so this is no problem. What's instantiated is just a small shell around some configuration data, the actual roudi is created inrun
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.
Yeah, the lock's description is
Locks the socket for preventing multiple start of RouDi
, so, is the intention just to preventrun
or to instantiate it? In case ofrun
-only another test for that is missing?!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.
from my point of view it should only be
run
, else you cannot execute./iox-roudi -v
anymore when RouDi is running. All the stuff that can break if two instances are running happens in therun
methodThere 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.
Makes sense and I'm fine with that. So, is the PR complete now or should a 2nd run be tested?
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.
As discussed in the dev meeting, a test for a 2nd run will be done in #621.