-
Notifications
You must be signed in to change notification settings - Fork 177
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] add/improved IsPublished/IsSubscribed logic (5.13.x) #1632
Conversation
…IsSubscribed" (we may rename both functions to "IsConnected" in the future) new additional check for IsPublished using the connection state of the matching subscriber (should ensure that IsPublished is flagged as true only if publisher is able to send data) test added to test new behavior
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.
Let's remove IsPublished() for 5.13 and add it only for 5.14.
@@ -295,8 +295,12 @@ namespace eCAL | |||
|
|||
iter->second->ApplyLocLayerParameter(process_id, topic_id, tlayer.type(), writer_par); | |||
} | |||
// inform for local publisher connection | |||
iter->second->ApplyLocPublication(process_id, topic_id, topic_info); | |||
// we only inform the subscriber when the publisher has already recognized at least on local subscriber |
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.
at least on -> at least one
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.
fixed
@@ -345,8 +349,12 @@ namespace eCAL | |||
const std::string writer_par = tlayer.par_layer().SerializeAsString(); | |||
iter->second->ApplyExtLayerParameter(host_name, tlayer.type(), writer_par); | |||
} | |||
// inform for external publisher connection | |||
iter->second->ApplyExtPublication(host_name, process_id, topic_id, topic_info); | |||
// we only inform the subscriber when the publisher has already recognized at least on external subscriber |
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.
At least on -> at least one
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.
fixed
…compatibility within 5.13.x
Description
Related issues