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
[core] ported CMsgPublisher/Subscriber to v6 #1944
[core] ported CMsgPublisher/Subscriber to v6 #1944
Changes from 15 commits
3a36e3f
1172908
d6164c6
0065213
e327753
e651e79
8064e89
a45837d
287c7e8
52f64d3
ec5eec4
0c911d9
9ddae4c
bb5f00b
380d1ea
70805f3
7d0078b
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.
Could this in any way be problematic?
So with eCAL 5, publishers will have the same ID they had when originally recording the measurement. In eCAL 6, they won't.
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.
Its about the topic id for subscriber side filtering. This feature is rarely used and no way to support this in the future, it's removed on both sides (pub and sub). So in v5 I can filter out different "streams" of the the same topic on subscriber side in v6 the connection will just receive all messages and I need to implement an ID myself (for example as part of my protobuf description).
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.
Why surpress it only for Windows? also maybe just comment in the function declaration
int main(int /*argc*/, char** /*argv*/)
(also this has nothing to do with this PR!!! 😏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.
For LINUX platform both arguments are used unfortunately and yes it's not really related but I tried to fix at least some warnings when I touched a file.
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 kind of don't like that now we're "forcing" code to use pointers.
Do you now have to check the pointers in other locations?
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.
Without a Create/Destroy interface and the removed empty default constructors I see no alternative here.
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.
We need to think about error handling for all communication entities (pub/sub/server/client) in case construction fails.
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.
Yes, but the thing is, construction is unlikely to fail. Failure is more likely to occur during the runtime. E.g. The connections (memory files, sockets, ...) are usually not created during construction, but when we receive monitoring information about other entities in the system.
So we have to find a way to deal with this effectively.
Maybe error/event callbacks would be interesting in that case, too.