Skip to content
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 (master) #1633

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

rex-schilasky
Copy link
Contributor

Description

  • new CSubscriber method "IsPublished" added
  • additional check for IsPublished using the connection state of the matching subscriber to ensure that IsPublished is flagged true only if publisher is able to send data
  • test added to test new behavior

Related issues

…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
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -216,3 +216,97 @@ TEST(core_cpp_pubsub, SporadicEmptyReceives)
// finalize eCAL API
EXPECT_EQ(0, eCAL::Finalize());
}

TEST(PubSub, TestSubscriberSeen)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(PubSub, TestSubscriberSeen)
TEST(PubSub /*unused*/, TestSubscriberSeen /*unused*/)

auto max_lines(10);
auto receive_lambda = [&received, &max_lines](const char* /*topic_name_*/, const struct eCAL::SReceiveCallbackData* data_)
{
if (max_lines)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion]

Suggested change
if (max_lines)
if (max_lines != 0)

@rex-schilasky rex-schilasky added the cherry-pick-to-NONE Don't cherry-pick these changes label Jun 20, 2024
Copy link
Contributor

@KerstinKeller KerstinKeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@KerstinKeller KerstinKeller self-requested a review June 21, 2024 09:21
@rex-schilasky rex-schilasky merged commit 35c1fd9 into master Jun 25, 2024
19 of 22 checks passed
@rex-schilasky rex-schilasky deleted the hotfix/pub_sub_state_logic_master branch July 17, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-to-NONE Don't cherry-pick these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants