From d8269639a588dbb92335dc934f912f860b8f5d8c Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 28 Aug 2020 14:58:37 -0700 Subject: [PATCH] =?UTF-8?q?=207=20=E2=9E=A1=EF=B8=8F=208=20=20(#155)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove warnings using ZMQ 4.3.1 or greater. * Do not use ZMQ_CPP11 * Win debugging. * backport improved compiler support for std::filesystem * Close branch backport_compiler_filesystem * Restore original Playback::Start and add overload with new parameter to fix ABI * bump to 7.2.2 and update changelog * Close branch fix_abi_7 * Write to disk from a background thread in log recorder * Update Changelog * Move `dataWriterState = true` to Recorder::Implementation::DataWriterThread() thread. * Revert moving dataWriterState * Failing test with incorrect time stamps * Correctly record message reception time stamp * Reorder functions * Specify buffer size in MB rather than number of elements in the data queue * Flush any remaining data to log file when stopping the Recorder * Codecheck. The rvalue ref is used to ensure that std::vector is always moved * Version update * Added tag ignition-transport7_7.3.0~pre1 for changeset 173fae6c362d * recorder.cc: include * Add console message to indicate buffer being flushed * Add * Close branch async_recorder * Prepare for 7.3.0 * Added tag ignition-transport7_7.3.0 for changeset 367d4f1bfcf7 * Configurable buffer sizes. * Fix typo. * Changelog. * Clarify high water mark policy. * Tweak documentation and error messages. * Close branch issue_116_transport7 * fix line lengths * Close branch codecheck7 * Update default values for the high water marks * Update buffer default values * Changelog.md edited online with Bitbucket * Close branch default_hwm * Adding connection message. * ConnectionMsg implementation. * Test * No control socket. * Preserve ABI. * Discard registrations when needed. * Tweaks. * Changelog * Fix issue #114. * Close branch discovery_extended_p2 * 7.4.0 * Move changelog entry * Close branch ign-transport7-4 * Added tag ignition-transport7_7.4.0 for changeset 083e7bf41080 * Protobuf warnings * Close branch proto_deprecations * Close branch issue_111 * Windows warnings * revert commit to release branch * Fix version for send_falgs command * Close branch ign-transport7_fix_send_flags * Backport pull request #441 * updates * Added another check * Close branch issue_118 * mv hgignore Signed-off-by: claireyywang * add gitignore Signed-off-by: claireyywang * [ign-transport7] Update BitBucket links (#123) * [ign-transport7] Update BitBucket links Signed-off-by: Louise Poubel * changelog pull-requests * Apply suggestions from code review * Update tutorials/07_relay.md Co-authored-by: Marya Belanger * [ign-transport7] Workflow updates (#132) * [ign-transport7] Workflow updates Signed-off-by: Louise Poubel * Helper function to get a valid topic name (#153) Signed-off-by: Louise Poubel * Remove Windows warnings (#151) Signed-off-by: Carlos Aguero * Remove warnings on Homebrew (#150) Signed-off-by: Carlos Aguero Co-authored-by: Louise Poubel * Bump to 7.5.0 (#156) Signed-off-by: Louise Poubel * Modernize actions CI (#158) Signed-off-by: Louise Poubel * remove ci-bionic Signed-off-by: Louise Poubel * add focal Signed-off-by: Louise Poubel * msgs5 Signed-off-by: Louise Poubel * Suppress focal-specific warnings (#159) * Suppress focal-specific warnings Signed-off-by: Michael Carroll * Warn when lsb_release isn't present Signed-off-by: Michael Carroll * Adding header guard. Signed-off-by: Carlos Agüero * Include correct header file for version check Signed-off-by: Michael Carroll * Added more debug output Signed-off-by: Nate Koenig * Fix focal test and codecheck Signed-off-by: Nate Koenig * Change endtime expectation Signed-off-by: Carlos Agüero Co-authored-by: Carlos Agüero Co-authored-by: Nate Koenig Co-authored-by: Carlos Aguero Co-authored-by: Steve Peters Co-authored-by: Steve Peters Co-authored-by: Carlos Agüero Co-authored-by: Addisu Z. Taddese Co-authored-by: Nate Koenig Co-authored-by: Carlos Aguero Co-authored-by: Jose Luis Rivero Co-authored-by: claireyywang Co-authored-by: Marya Belanger Co-authored-by: Michael Carroll Co-authored-by: Nate Koenig --- .github/ci/packages.apt | 11 ++++++ .github/workflows/ci-bionic.yml | 17 -------- .github/workflows/ci.yml | 27 +++++++++++++ CMakeLists.txt | 14 +++++++ Changelog.md | 17 ++++++++ include/ignition/transport/Helpers.hh | 13 +++++-- include/ignition/transport/TopicUtils.hh | 8 ++++ include/ignition/transport/config.hh.in | 6 +++ log/src/Log_TEST.cc | 3 +- src/CIface_TEST.cc | 2 + src/Node.cc | 2 +- src/NodeShared.cc | 16 ++++---- src/TopicUtils.cc | 20 ++++++++++ src/TopicUtils_TEST.cc | 49 ++++++++++++++++++++++++ 14 files changed, 174 insertions(+), 31 deletions(-) create mode 100644 .github/ci/packages.apt delete mode 100644 .github/workflows/ci-bionic.yml create mode 100644 .github/workflows/ci.yml diff --git a/.github/ci/packages.apt b/.github/ci/packages.apt new file mode 100644 index 000000000..8747d7a0e --- /dev/null +++ b/.github/ci/packages.apt @@ -0,0 +1,11 @@ +libignition-cmake2-dev +libignition-math6-dev +libignition-msgs5-dev +libignition-tools-dev +libprotobuf-dev +libprotoc-dev +libsqlite3-dev +libzmq3-dev +pkg-config +protobuf-compiler +uuid-dev diff --git a/.github/workflows/ci-bionic.yml b/.github/workflows/ci-bionic.yml deleted file mode 100644 index f851b0a58..000000000 --- a/.github/workflows/ci-bionic.yml +++ /dev/null @@ -1,17 +0,0 @@ -name: Ubuntu Bionic CI - -on: [push, pull_request] - -jobs: - bionic-ci: - runs-on: ubuntu-latest - name: Ubuntu Bionic CI - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Bionic CI - id: ci - uses: ignition-tooling/ubuntu-bionic-ci-action@master - with: - apt-dependencies: 'pkg-config libprotobuf-dev protobuf-compiler libprotoc-dev libzmq3-dev uuid-dev libsqlite3-dev libignition-cmake2-dev libignition-math6-dev libignition-msgs5-dev libignition-tools-dev' - codecov-token: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..5d9be4dd1 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,27 @@ +name: Ubuntu CI + +on: [push] + +jobs: + bionic-ci: + runs-on: ubuntu-latest + name: Ubuntu Bionic CI + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Compile and test + id: ci + uses: ignition-tooling/action-ignition-ci@master + with: + codecov-token: ${{ secrets.CODECOV_TOKEN }} + focal-ci: + runs-on: ubuntu-latest + name: Ubuntu Focal CI + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Compile and test + id: ci + uses: ignition-tooling/action-ignition-ci@focal + with: + codecov-token: ${{ secrets.CODECOV_TOKEN }} diff --git a/CMakeLists.txt b/CMakeLists.txt index 12e1942a4..72a523b22 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,6 +46,20 @@ ign_find_package(IgnProtobuf # Find ZeroMQ ign_find_package(ZeroMQ VERSION 4 REQUIRED PRIVATE) +if (UNIX AND NOT APPLE) + execute_process(COMMAND lsb_release -cs + OUTPUT_VARIABLE RELEASE_CODENAME + RESULT_VARIABLE LSB_RESULT + OUTPUT_STRIP_TRAILING_WHITESPACE + ) + + if( NOT (${LSB_RESULT} STREQUAL "0")) + message(WARNING "lsb_release executable not found. Disabling focal-specific workarounds") + elseif (${RELEASE_CODENAME} STREQUAL "focal") + set(UBUNTU_FOCAL 1) + endif() +endif() + #-------------------------------------- # Find cppzmq ign_find_package(CPPZMQ REQUIRED PRIVATE diff --git a/Changelog.md b/Changelog.md index 99d1ed420..3f8a618e6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -44,6 +44,23 @@ ### Ignition Transport 7.x.x (202x-xx-xx) +### Ignition Transport 7.5.0 (2020-07-29) + +1. Helper function to get a valid topic name + * [Pull request 153](https://github.com/ignitionrobotics/ign-transport/pull/153) + +1. GitHub migration + * [Pull request 132](https://github.com/ignitionrobotics/ign-transport/pull/132) + * [Pull request 123](https://github.com/ignitionrobotics/ign-transport/pull/123) + * [Pull request 126](https://github.com/ignitionrobotics/ign-transport/pull/126) + +1. Fix ZMQ and Protobuf warnings + * [BitBucket pull request 442](https://osrf-migration.github.io/ignition-gh-pages/#!/ignitionrobotics/ign-transport/pull-requests/442) + * [BitBucket pull request 438](https://osrf-migration.github.io/ignition-gh-pages/#!/ignitionrobotics/ign-transport/pull-requests/438) + * [BitBucket pull request 439](https://osrf-migration.github.io/ignition-gh-pages/#!/ignitionrobotics/ign-transport/pull-requests/439) + * [Pull request 150](https://github.com/ignitionrobotics/ign-transport/pull/150) + * [Pull request 151](https://github.com/ignitionrobotics/ign-transport/pull/151) + 1. Handle `getpwduid_r` error cases. This addresses issue #118. Solution was created in pull request #441 by Poh Zhi-Ee. * [BitBucket pull request 444](https://osrf-migration.github.io/ignition-gh-pages/#!/ignitionrobotics/ign-transport/pull-requests/444) diff --git a/include/ignition/transport/Helpers.hh b/include/ignition/transport/Helpers.hh index 338f33b9b..8b0323cd3 100644 --- a/include/ignition/transport/Helpers.hh +++ b/include/ignition/transport/Helpers.hh @@ -18,7 +18,7 @@ #ifndef IGN_TRANSPORT_HELPERS_HH_ #define IGN_TRANSPORT_HELPERS_HH_ -#include +#include #include #include @@ -30,14 +30,19 @@ #include "ignition/transport/config.hh" #include "ignition/transport/Export.hh" -#define STR_HELPER(x) #x -#define STR(x) STR_HELPER(x) - // Avoid using deprecated message send/receive function when possible. #if ZMQ_VERSION >= ZMQ_MAKE_VERSION(4, 3, 1) #define IGN_ZMQ_POST_4_3_1 #endif +// Avoid using deprecated set function when possible +#if CPPZMQ_VERSION >= ZMQ_MAKE_VERSION(4, 7, 0) + // Ubuntu Focal (20.04) packages a different "4.7.0" + #ifndef UBUNTU_FOCAL + #define IGN_CPPZMQ_POST_4_7_0 + #endif +#endif + namespace ignition { namespace transport diff --git a/include/ignition/transport/TopicUtils.hh b/include/ignition/transport/TopicUtils.hh index bbfda0551..301575d3d 100644 --- a/include/ignition/transport/TopicUtils.hh +++ b/include/ignition/transport/TopicUtils.hh @@ -128,6 +128,14 @@ namespace ignition std::string &_partition, std::string &_namespaceAndTopic); + /// \brief Convert a topic name to a valid topic. The input topic is + /// modified by: + /// * turning white space into `_`. + /// * removing special characters and combinations. + /// \param[in] _topic Input topic, which may be invalid. + /// \return A valid topic, or empty string if not possible to convert. + public: static std::string AsValidTopic(const std::string &_topic); + /// \brief The kMaxNameLength specifies the maximum number of characters /// allowed in a namespace, a partition name, a topic name, and a fully /// qualified topic name. diff --git a/include/ignition/transport/config.hh.in b/include/ignition/transport/config.hh.in index ef965e59f..ab9ea3b10 100644 --- a/include/ignition/transport/config.hh.in +++ b/include/ignition/transport/config.hh.in @@ -1,5 +1,8 @@ /* Config.hh. Generated by CMake for @PROJECT_NAME@. */ +#ifndef IGNITION_${IGN_DESIGNATION_UPPER}_CONFIG_HH_ +#define IGNITION_${IGN_DESIGNATION_UPPER}_CONFIG_HH_ + /* Version number */ #define IGNITION_TRANSPORT_MAJOR_VERSION ${PROJECT_VERSION_MAJOR} #define IGNITION_TRANSPORT_MINOR_VERSION ${PROJECT_VERSION_MINOR} @@ -17,3 +20,6 @@ #cmakedefine BUILD_TYPE_RELEASE 1 #cmakedefine HAVE_IFADDRS 1 +#cmakedefine UBUNTU_FOCAL 1 + +#endif diff --git a/log/src/Log_TEST.cc b/log/src/Log_TEST.cc index 1132a564e..7eab28334 100644 --- a/log/src/Log_TEST.cc +++ b/log/src/Log_TEST.cc @@ -255,7 +255,8 @@ TEST(Log, OpenCorruptDatabase) testing::portablePathUnion(IGN_TRANSPORT_LOG_TEST_PATH, "data"); path = testing::portablePathUnion(path, "state.tlog"); logFile.Open(path); - EXPECT_EQ(4806000000ns, logFile.EndTime()); + EXPECT_GT(logFile.EndTime(), 0ns) << "logFile.EndTime() == " + << logFile.EndTime().count() << "ns";; } diff --git a/src/CIface_TEST.cc b/src/CIface_TEST.cc index 467ba0c65..3c40a561b 100644 --- a/src/CIface_TEST.cc +++ b/src/CIface_TEST.cc @@ -77,6 +77,7 @@ TEST(CIfaceTest, PubSub) ignition::msgs::StringMsg msg; msg.set_data("HELLO"); + // Get the size of the serialized message #if GOOGLE_PROTOBUF_VERSION >= 3004000 int size = msg.ByteSizeLong(); #else @@ -132,6 +133,7 @@ TEST(CIfaceTest, PubSubPartitions) ignition::msgs::StringMsg msg; msg.set_data("HELLO"); + // Get the size of the serialized message #if GOOGLE_PROTOBUF_VERSION >= 3004000 int size = msg.ByteSizeLong(); #else diff --git a/src/Node.cc b/src/Node.cc index 551717833..c10e53511 100644 --- a/src/Node.cc +++ b/src/Node.cc @@ -619,7 +619,7 @@ bool Node::Unsubscribe(const std::string &_topic) if (!this->dataPtr->shared->localSubscribers .HasSubscriber(fullyQualifiedTopic)) { -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 this->dataPtr->shared->dataPtr->subscriber->set( zmq::sockopt::unsubscribe, fullyQualifiedTopic); #else diff --git a/src/NodeShared.cc b/src/NodeShared.cc index 4c1c89a77..1ea7cc2a0 100644 --- a/src/NodeShared.cc +++ b/src/NodeShared.cc @@ -1068,7 +1068,7 @@ void NodeShared::OnNewConnection(const MessagePublisher &_pub) this->dataPtr->subscriber->connect(addr.c_str()); // Add a new filter for the topic. -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 this->dataPtr->subscriber->set(zmq::sockopt::subscribe, topic); #else this->dataPtr->subscriber->setsockopt(ZMQ_SUBSCRIBE, @@ -1263,7 +1263,7 @@ bool NodeShared::InitializeSockets() int lingerVal = 0; -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 this->dataPtr->publisher->set(zmq::sockopt::linger, lingerVal); #else this->dataPtr->publisher->setsockopt(ZMQ_LINGER, @@ -1301,7 +1301,7 @@ bool NodeShared::InitializeSockets() << std::endl; } } -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 this->dataPtr->subscriber->set(zmq::sockopt::rcvhwm, rcvQueueVal); #else this->dataPtr->subscriber->setsockopt(ZMQ_RCVHWM, @@ -1339,7 +1339,7 @@ bool NodeShared::InitializeSockets() << std::endl; } } -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 this->dataPtr->publisher->set(zmq::sockopt::sndhwm, sndQueueVal); this->dataPtr->publisher->bind(anyTcpEp.c_str()); @@ -1439,7 +1439,7 @@ int NodeShared::RcvHwm() int rcvHwm; try { -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 rcvHwm = this->dataPtr->subscriber->get(zmq::sockopt::rcvhwm); #else size_t rcvHwmSize = sizeof(rcvHwm); @@ -1460,7 +1460,7 @@ int NodeShared::SndHwm() int sndHwm; try { -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 sndHwm = this->dataPtr->publisher->get(zmq::sockopt::sndhwm); #else size_t sndHwmSize = sizeof(sndHwm); @@ -1561,7 +1561,7 @@ void NodeSharedPrivate::SecurityOnNewConnection() // See issue #74 if (userPass(user, pass)) { -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 this->subscriber->set(zmq::sockopt::plain_username, user); this->subscriber->set(zmq::sockopt::plain_password, pass); #else @@ -1586,7 +1586,7 @@ void NodeSharedPrivate::SecurityInit() int asPlainSecurityServer = static_cast( ZmqPlainSecurityServerOptions::ZMQ_PLAIN_SECURITY_SERVER_ENABLED); -#if (CPPZMQ_VERSION >= 40700) +#ifdef IGN_CPPZMQ_POST_4_7_0 this->publisher->set(zmq::sockopt::plain_server, asPlainSecurityServer); this->publisher->set(zmq::sockopt::zap_domain, kIgnAuthDomain); #else diff --git a/src/TopicUtils.cc b/src/TopicUtils.cc index 18ea87a8b..76c18949f 100644 --- a/src/TopicUtils.cc +++ b/src/TopicUtils.cc @@ -15,6 +15,7 @@ * */ +#include #include #include "ignition/transport/TopicUtils.hh" @@ -154,3 +155,22 @@ bool TopicUtils::DecomposeFullyQualifiedTopic( _namespaceAndTopic = possibleTopic; return true; } + +////////////////////////////////////////////////// +std::string TopicUtils::AsValidTopic(const std::string &_topic) +{ + std::string validTopic{_topic}; + + // Substitute spaces with _ + validTopic = std::regex_replace(validTopic, std::regex(" "), "_"); + + // Remove special characters and combinations + validTopic = std::regex_replace(validTopic, std::regex("@|~|//|:="), ""); + + if (!IsValidTopic(validTopic)) + { + return std::string(); + } + + return validTopic; +} diff --git a/src/TopicUtils_TEST.cc b/src/TopicUtils_TEST.cc index 195bd65d3..aca3809df 100644 --- a/src/TopicUtils_TEST.cc +++ b/src/TopicUtils_TEST.cc @@ -242,6 +242,55 @@ TEST(TopicUtilsTest, testFullyQualifiedName) } } +////////////////////////////////////////////////// +TEST(TopicUtilsTest, asValidTopic) +{ + for (auto unmodified : + { + "/abc", + "abc/de", + "a", + "ABC/", + "/abc", + "/abc/d", + "/abc/d/e", + "a(bc)d-e_f=h+i.j" + }) + { + auto valid = transport::TopicUtils::AsValidTopic(unmodified); + EXPECT_EQ(unmodified, valid); + EXPECT_TRUE(transport::TopicUtils::IsValidTopic(valid)) << valid; + } + + std::vector> modifiedStrings = + { + {"a b c", "a_b__c"}, + {"a@b@c", "abc"}, + {"a:=b:=c", "abc"}, + {"a//b/c", "ab/c"}, + {"a~b~c", "abc"} + }; + + for (auto modified : modifiedStrings) + { + auto valid = transport::TopicUtils::AsValidTopic(modified.first); + EXPECT_EQ(modified.second, valid); + EXPECT_TRUE(transport::TopicUtils::IsValidTopic(valid)) << valid; + } + + for (auto fail : + { + "", + "@@@", + "~@~", + }) + { + auto empty = transport::TopicUtils::AsValidTopic(fail); + EXPECT_TRUE(empty.empty()); + EXPECT_FALSE(transport::TopicUtils::IsValidTopic(empty)); + } +} + ////////////////////////////////////////////////// int main(int argc, char **argv) {