diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3e92357e34..b85c8b416b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -169,8 +169,6 @@ jobs: with: path: ~/.conan/data key: ${{ runner.os }}-${{ hashFiles('cpp/**/conanfile.py') }} - - uses: satackey/action-docker-layer-caching@v0.0.11 - continue-on-error: true - run: make ci-format-check - run: make ci - run: make test-host diff --git a/cpp/bench/conanfile.py b/cpp/bench/conanfile.py index c99439fca5..4878ed2aab 100644 --- a/cpp/bench/conanfile.py +++ b/cpp/bench/conanfile.py @@ -4,7 +4,7 @@ class McapBenchmarksConan(ConanFile): settings = "os", "compiler", "build_type", "arch" generators = "cmake" - requires = "benchmark/1.7.0", "mcap/1.2.0" + requires = "benchmark/1.7.0", "mcap/1.2.1" def build(self): cmake = CMake(self) diff --git a/cpp/build-docs.sh b/cpp/build-docs.sh index 73cc626aa7..1d7e9fd4c2 100755 --- a/cpp/build-docs.sh +++ b/cpp/build-docs.sh @@ -4,7 +4,7 @@ set -e conan config init -conan editable add ./mcap mcap/1.2.0 +conan editable add ./mcap mcap/1.2.1 conan install docs --install-folder docs/build/Release \ -s compiler.cppstd=17 -s build_type=Release --build missing diff --git a/cpp/build.sh b/cpp/build.sh index 258fbac067..53cff3b93d 100755 --- a/cpp/build.sh +++ b/cpp/build.sh @@ -4,7 +4,7 @@ set -e conan config init -conan editable add ./mcap mcap/1.2.0 +conan editable add ./mcap mcap/1.2.1 conan install test --install-folder test/build/Debug \ -s compiler.cppstd=17 -s build_type=Debug --build missing diff --git a/cpp/docs/conanfile.py b/cpp/docs/conanfile.py index 05d7650ae1..9ff9116f01 100644 --- a/cpp/docs/conanfile.py +++ b/cpp/docs/conanfile.py @@ -4,7 +4,7 @@ class McapDocsConan(ConanFile): settings = "os", "compiler", "build_type", "arch" generators = "cmake" - requires = "mcap/1.2.0" + requires = "mcap/1.2.1" def build(self): cmake = CMake(self) diff --git a/cpp/examples/conanfile.py b/cpp/examples/conanfile.py index 250c2b26b9..f2821d0215 100644 --- a/cpp/examples/conanfile.py +++ b/cpp/examples/conanfile.py @@ -5,7 +5,7 @@ class McapExamplesConan(ConanFile): settings = "os", "compiler", "build_type", "arch" generators = "cmake" requires = [ - "mcap/1.2.0", + "mcap/1.2.1", "protobuf/3.21.1", "nlohmann_json/3.10.5", "catch2/2.13.8", diff --git a/cpp/mcap/conanfile.py b/cpp/mcap/conanfile.py index eee35a3876..ef824c151f 100644 --- a/cpp/mcap/conanfile.py +++ b/cpp/mcap/conanfile.py @@ -3,7 +3,7 @@ class McapConan(ConanFile): name = "mcap" - version = "1.2.0" + version = "1.2.1" url = "https://github.com/foxglove/mcap" homepage = "https://github.com/foxglove/mcap" description = "A C++ implementation of the MCAP file format" diff --git a/cpp/mcap/include/mcap/reader.hpp b/cpp/mcap/include/mcap/reader.hpp index 43ccbaf588..d1810f4034 100644 --- a/cpp/mcap/include/mcap/reader.hpp +++ b/cpp/mcap/include/mcap/reader.hpp @@ -657,13 +657,11 @@ struct MCAP_PUBLIC LinearMessageView { friend LinearMessageView; Iterator() = default; - Iterator(McapReader& mcapReader, ByteOffset dataStart, ByteOffset dataEnd, - const ReadMessageOptions& options, const ProblemCallback& onProblem); + Iterator(LinearMessageView& view); class Impl { public: - Impl(McapReader& mcapReader, ByteOffset dataStart, ByteOffset dataEnd, - const ReadMessageOptions& options, const ProblemCallback& onProblem); + Impl(LinearMessageView& view); Impl(const Impl&) = delete; Impl& operator=(const Impl&) = delete; @@ -674,11 +672,10 @@ struct MCAP_PUBLIC LinearMessageView { reference dereference() const; bool has_value() const; - McapReader& mcapReader_; + LinearMessageView& view_; + std::optional recordReader_; std::optional indexedMessageReader_; - ReadMessageOptions readMessageOptions_; - const ProblemCallback& onProblem_; Message curMessage_; std::optional curMessageView_; @@ -686,6 +683,7 @@ struct MCAP_PUBLIC LinearMessageView { void onMessage(const Message& message, RecordOffset offset); }; + bool begun_ = false; std::unique_ptr impl_; }; diff --git a/cpp/mcap/include/mcap/reader.inl b/cpp/mcap/include/mcap/reader.inl index e9e098831b..955de4390f 100644 --- a/cpp/mcap/include/mcap/reader.inl +++ b/cpp/mcap/include/mcap/reader.inl @@ -1588,8 +1588,7 @@ LinearMessageView::Iterator LinearMessageView::begin() { if (dataStart_ == dataEnd_ || !mcapReader_.dataSource()) { return end(); } - return LinearMessageView::Iterator{mcapReader_, dataStart_, dataEnd_, readMessageOptions_, - onProblem_}; + return LinearMessageView::Iterator{*this}; } LinearMessageView::Iterator LinearMessageView::end() { @@ -1598,37 +1597,27 @@ LinearMessageView::Iterator LinearMessageView::end() { // LinearMessageView::Iterator ///////////////////////////////////////////////// -LinearMessageView::Iterator::Iterator(McapReader& mcapReader, ByteOffset dataStart, - ByteOffset dataEnd, - const ReadMessageOptions& readMessageOptions, - const ProblemCallback& onProblem) - : impl_(std::make_unique(mcapReader, dataStart, dataEnd, readMessageOptions, onProblem)) { +LinearMessageView::Iterator::Iterator(LinearMessageView& view) + : impl_(std::make_unique(view)) { if (!impl_->has_value()) { impl_ = nullptr; } } -LinearMessageView::Iterator::Impl::Impl(McapReader& mcapReader, ByteOffset dataStart, - ByteOffset dataEnd, - const ReadMessageOptions& readMessageOptions, - const ProblemCallback& onProblem) - : mcapReader_(mcapReader) - , readMessageOptions_(readMessageOptions) - , onProblem_(onProblem) { - auto optionsStatus = readMessageOptions_.validate(); - if (!optionsStatus.ok()) { - onProblem(optionsStatus); - } - if (readMessageOptions_.readOrder == ReadMessageOptions::ReadOrder::FileOrder) { - recordReader_.emplace(*mcapReader.dataSource(), dataStart, dataEnd); +LinearMessageView::Iterator::Impl::Impl(LinearMessageView& view) : view_(view) { + auto dataStart = view.dataStart_; + auto dataEnd = view.dataEnd_; + auto readMessageOptions = view.readMessageOptions_; + if (readMessageOptions.readOrder == ReadMessageOptions::ReadOrder::FileOrder) { + recordReader_.emplace(*(view_.mcapReader_.dataSource()), dataStart, dataEnd); recordReader_->onSchema = [this](const SchemaPtr schema, ByteOffset, std::optional) { - mcapReader_.schemas_.insert_or_assign(schema->id, schema); + view_.mcapReader_.schemas_.insert_or_assign(schema->id, schema); }; recordReader_->onChannel = [this](const ChannelPtr channel, ByteOffset, std::optional) { - mcapReader_.channels_.insert_or_assign(channel->id, channel); + view_.mcapReader_.channels_.insert_or_assign(channel->id, channel); }; recordReader_->onMessage = [this](const Message& message, ByteOffset messageStartOffset, std::optional chunkStartOffset) { @@ -1638,7 +1627,7 @@ LinearMessageView::Iterator::Impl::Impl(McapReader& mcapReader, ByteOffset dataS onMessage(message, offset); }; } else { - indexedMessageReader_.emplace(mcapReader, readMessageOptions_, + indexedMessageReader_.emplace(view_.mcapReader_, readMessageOptions, std::bind(&LinearMessageView::Iterator::Impl::onMessage, this, std::placeholders::_1, std::placeholders::_2)); } @@ -1652,15 +1641,15 @@ LinearMessageView::Iterator::Impl::Impl(McapReader& mcapReader, ByteOffset dataS */ void LinearMessageView::Iterator::Impl::onMessage(const Message& message, RecordOffset offset) { // make sure the message is within the expected time range - if (message.logTime < readMessageOptions_.startTime) { + if (message.logTime < view_.readMessageOptions_.startTime) { return; } - if (message.logTime >= readMessageOptions_.endTime) { + if (message.logTime >= view_.readMessageOptions_.endTime) { return; } - auto maybeChannel = mcapReader_.channel(message.channelId); + auto maybeChannel = view_.mcapReader_.channel(message.channelId); if (!maybeChannel) { - onProblem_( + view_.onProblem_( Status{StatusCode::InvalidChannelId, internal::StrCat("message at log_time ", message.logTime, " (seq ", message.sequence, ") references missing channel id ", message.channelId)}); @@ -1669,14 +1658,14 @@ void LinearMessageView::Iterator::Impl::onMessage(const Message& message, Record auto& channel = *maybeChannel; // make sure the message is on the right topic - if (readMessageOptions_.topicFilter && !readMessageOptions_.topicFilter(channel.topic)) { + if (view_.readMessageOptions_.topicFilter && !view_.readMessageOptions_.topicFilter(channel.topic)) { return; } SchemaPtr maybeSchema; if (channel.schemaId != 0) { - maybeSchema = mcapReader_.schema(channel.schemaId); + maybeSchema = view_.mcapReader_.schema(channel.schemaId); if (!maybeSchema) { - onProblem_(Status{StatusCode::InvalidSchemaId, + view_.onProblem_(Status{StatusCode::InvalidSchemaId, internal::StrCat("channel ", channel.id, " (", channel.topic, ") references missing schema id ", channel.schemaId)}); return; @@ -1698,7 +1687,7 @@ void LinearMessageView::Iterator::Impl::increment() { // Surface any problem that may have occurred while reading auto& status = recordReader_->status(); if (!status.ok()) { - onProblem_(status); + view_.onProblem_(status); } if (!found) { @@ -1714,7 +1703,7 @@ void LinearMessageView::Iterator::Impl::increment() { // alert with onProblem_. auto status = indexedMessageReader_->status(); if (!status.ok()) { - onProblem_(status); + view_.onProblem_(status); } indexedMessageReader_ = std::nullopt; return; @@ -1740,6 +1729,7 @@ LinearMessageView::Iterator::pointer LinearMessageView::Iterator::operator->() c } LinearMessageView::Iterator& LinearMessageView::Iterator::operator++() { + begun_ = true; impl_->increment(); if (!impl_->has_value()) { impl_ = nullptr; @@ -1752,7 +1742,17 @@ void LinearMessageView::Iterator::operator++(int) { } bool operator==(const LinearMessageView::Iterator& a, const LinearMessageView::Iterator& b) { - return a.impl_ == b.impl_; + if (a.impl_ == nullptr || b.impl_ == nullptr) { + // special case for Iterator::end() == Iterator::end() + return a.impl_ == b.impl_; + } + if (!a.begun_ && !b.begun_) { + // special case for Iterator::begin() == Iterator::begin() + // comparing iterators to the beginning of the same view should return true. + return &(a.impl_->view_) == &(b.impl_->view_); + } + // In all other cases, compare by object identity. + return &(a) == &(b); } bool operator!=(const LinearMessageView::Iterator& a, const LinearMessageView::Iterator& b) { diff --git a/cpp/mcap/include/mcap/types.hpp b/cpp/mcap/include/mcap/types.hpp index def8496adb..b186d2681b 100644 --- a/cpp/mcap/include/mcap/types.hpp +++ b/cpp/mcap/include/mcap/types.hpp @@ -13,7 +13,7 @@ namespace mcap { -#define MCAP_LIBRARY_VERSION "1.2.0" +#define MCAP_LIBRARY_VERSION "1.2.1" using SchemaId = uint16_t; using ChannelId = uint16_t; diff --git a/cpp/test/conanfile.py b/cpp/test/conanfile.py index 824bf9bbb4..6bcd8a5b3c 100644 --- a/cpp/test/conanfile.py +++ b/cpp/test/conanfile.py @@ -4,7 +4,7 @@ class McapTestConan(ConanFile): settings = "os", "compiler", "build_type", "arch" generators = "cmake" - requires = "catch2/2.13.8", "mcap/1.2.0", "nlohmann_json/3.10.5" + requires = "catch2/2.13.8", "mcap/1.2.1", "nlohmann_json/3.10.5" def build(self): cmake = CMake(self) diff --git a/cpp/test/unit_tests.cpp b/cpp/test/unit_tests.cpp index 90e05b61f6..18abf66f60 100644 --- a/cpp/test/unit_tests.cpp +++ b/cpp/test/unit_tests.cpp @@ -476,6 +476,56 @@ TEST_CASE("McapReader::readMessages()", "[reader]") { reader.close(); } + + SECTION("IteratorComparison") { + Buffer buffer; + + mcap::McapWriter writer; + writer.open(buffer, mcap::McapWriterOptions("test")); + mcap::Schema schema("schema", "schemaEncoding", "ab"); + writer.addSchema(schema); + mcap::Channel channel("topic", "messageEncoding", schema.id); + writer.addChannel(channel); + std::vector data = {std::byte(1), std::byte(2), std::byte(3)}; + WriteMsg(writer, channel.id, 0, 2, 1, data); + WriteMsg(writer, channel.id, 1, 4, 3, data); + writer.close(); + + mcap::McapReader reader; + requireOk(reader.open(buffer)); + + auto view = reader.readMessages(); + auto it = view.begin(); + REQUIRE(it != view.end()); + REQUIRE(it == view.begin()); + REQUIRE(it == it); + ++it; + REQUIRE(it != view.end()); + REQUIRE(it != view.begin()); + REQUIRE(it == it); + ++it; + REQUIRE(it == view.end()); + REQUIRE(it != view.begin()); + REQUIRE(it == it); + + reader.close(); + } + SECTION("IteratorComparisonEmpty") { + Buffer buffer; + + mcap::McapWriter writer; + writer.open(buffer, mcap::McapWriterOptions("test")); + writer.close(); + + mcap::McapReader reader; + requireOk(reader.open(buffer)); + + auto view = reader.readMessages(); + auto it = view.begin(); + REQUIRE(it == view.begin()); + REQUIRE(it == view.end()); + reader.close(); + } } /**