resolve relative file paths #345

Karsten1987 · 2020-04-02T00:07:06Z

fixes #229

It basically takes the relative file paths and resolves them relativ to the base folder name, essentially the name of the rosbag to be replayed.
If one of these relativ files is actually in an absolute form, it will ignore it.

Signed-off-by: Karsten Knese karsten@openrobotics.org

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp

Karsten1987 · 2020-04-02T21:15:29Z

I've updated the logic to differentiate between version 4 and <=4. I've also included a test for compatibility.

That being said, version 4 removes the folder name and the file path is now relativ to the parent folder a.k.a the rosbag2 folder.

zmichaels11

This looks mostly good to me.
Only suggestion that requires changes is size_t -> int for version

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp

rosbag2_cpp/test/rosbag2_cpp/test_multifile_reader.cpp

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp

Karsten1987 · 2020-04-02T23:04:35Z

CI:

Linux
Linux-aarch64
macOS
Windows

mabelzhang

Works for me.
Since end-to-end record tests are updated to version 4, a to-do item for later might be to test end-to-end playback with version 3 (when that test is re-enabled and files exist to test it...).

Karsten1987 · 2020-04-03T00:07:35Z

@emersonknapp I am getting QoS test failures, did I miss something?

16:14:56 7: [ RUN      ] RecordIntegrationTestFixture.published_messages_from_multiple_topics_are_recorded
16:14:56 7: [INFO] [1585869296.012310473] [rosbag2_transport]: Listening for topics...
16:14:56 7: [INFO] [1585869296.018655979] [rosbag2_transport]: Subscribed to topic '/string_topic'
16:14:56 7: [WARN] [1585869296.021000346] [rosbag2_transport]: A new publisher for subscribed topic /string_topic was found that is offering a (possibly) incompatible QoS profile. Not changing subscription QoS. Messages from this publisher may not be recorded.
16:14:56 7: [INFO] [1585869296.023990862] [rosbag2_transport]: Subscribed to topic '/array_topic'
16:14:56 7: [INFO] [1585869296.024012022] [rosbag2_transport]: All requested topics are subscribed. Stopping discovery...
16:15:55 2/2 Test #7: test_record ......................***Timeout  60.04 sec

and

16:13:55 2: [ RUN      ] RecordIntegrationTestFixture.published_messages_from_multiple_topics_are_recorded
16:13:56 2: [INFO] [1585869235.999994675] [rosbag2_transport]: Listening for topics...
16:13:56 2: [INFO] [1585869236.004100034] [rosbag2_transport]: Subscribed to topic '/string_topic'
16:13:56 2: [INFO] [1585869236.005604653] [rosbag2_transport]: Subscribed to topic '/rosout'
16:13:56 2: [INFO] [1585869236.007492335] [rosbag2_transport]: Subscribed to topic '/parameter_events'
16:13:56 2: [INFO] [1585869236.009682717] [rosbag2_transport]: Subscribed to topic '/array_topic'
16:13:56 2: [WARN] [1585869236.013561291] [rosbag2_transport]: A new publisher for subscribed topic /string_topic was found that is offering a (possibly) incompatible QoS profile. Not changing subscription QoS. Messages from this publisher may not be recorded.
16:13:56 2: [WARN] [1585869236.013963259] [rosbag2_transport]: A new publisher for subscribed topic /rosout was found that is offering a (possibly) incompatible QoS profile. Not changing subscription QoS. Messages from this publisher may not be recorded.
16:13:56 2: [WARN] [1585869236.014366566] [rosbag2_transport]: A new publisher for subscribed topic /parameter_events was found that is offering a (possibly) incompatible QoS profile. Not changing subscription QoS. Messages from this publisher may not be recorded.
16:13:56 2: [WARN] [1585869236.015690209] [rosbag2_transport]: A new publisher for subscribed topic /array_topic was found that is offering a (possibly) incompatible QoS profile. Not changing subscription QoS. Messages from this publisher may not be recorded.
16:14:55 1/2 Test #2: test_record_all ..................***Timeout  60.02 sec

emersonknapp · 2020-04-03T00:29:57Z

Hmm, it all passed in my actions and CI run before merging, taking a look

emersonknapp · 2020-04-03T00:42:10Z

Well, the branch was behind, but it seemed like it was somehow running my newest code - I rebased and pushed here (not sure if that was the right move but we'll see what happens with the CI run)

Linux
Linux-aarch64
macOS
Windows

rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp

emersonknapp · 2020-04-03T00:46:28Z

Feel free to force push back over it, I made it fail the DCO. But i'm interested to see how the CI run does with this version of the branch

piraka9011

🚢

emersonknapp · 2020-04-03T01:06:51Z

Ah, I noticed that your CI runs are again using only cyclone, whereas mine build with all three, and i assume just run on FastRTPS by default. When I explicitly run

RMW_IMPLEMENTATION=rmw_cyclonedds_cpp colcon test --packages-select rosbag2_transport

locally now i see the timeouts.

The spurious warnings are definitely a symptom of what we discussed this morning, and the hanging at the end of the test, I'm not exactly sure. I think that we should probably do the following:

disable the feature for now (not revert the whole PR)
add an rmw_implementation matrix on our tests so we catch this by default
fix it on cyclone and re-enable

Sound good? If so, here's a PR #348

Karsten1987 · 2020-04-03T01:16:03Z

CI (with Fast-RTPS):

Linux
Linux-aarch64
macOS
Windows

I did indeed only use cyclone, because it's just so much faster to compile without any additional typesupport.

Karsten1987 · 2020-04-03T01:25:26Z

Aaaaand once more:

Linux
Linux-aarch64
macOS
Windows

emersonknapp · 2020-04-03T04:34:42Z

Ok @Karsten1987 you should be unblocked by the QoS tests on this one now, that feature is disabled, I am now looking into making it succeed on Cyclone

Karsten1987 · 2020-04-03T05:08:25Z

thanks. I have to give some more love to the Windows build.

Karsten1987 · 2020-04-03T15:37:03Z

rebase and another try for windows:

Linux
Linux-aarch64
macOS
Windows

Karsten1987 · 2020-04-03T17:09:42Z

Linux
Linux-aarch64
macOS
Windows

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Co-Authored-By: Zachary Michaels <zmichaels11@users.noreply.github.com>

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 · 2020-04-03T18:53:46Z

again after rebasing:

Linux
Linux-aarch64
macOS
Windows

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 · 2020-04-03T19:31:05Z

Linux
Linux-aarch64
macOS
Windows

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 · 2020-04-03T21:24:04Z

I am going slightly crazy with windows and file paths.
If that doesn't work now, I'll go ahead and disable that weird ctrl-c workaround for all tests on windows and address them in #271

Linux
Linux-aarch64
macOS
Windows

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

piraka9011 · 2020-04-03T23:03:45Z

@Karsten1987 I'd recommend just adding DCO and moving forward since we have it on our backlog to resolve the ctrl-c issue.

I've already added some Python tests based on launch in #347 and plan to expand from there to resolve the Windows ctrl-c issue.

Karsten1987 · 2020-04-03T23:10:57Z

The last one indeed did it though:
// only the regular test failures.

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp

Karsten1987 requested review from zmichaels11 and piraka9011 April 2, 2020 00:07

Karsten1987 self-assigned this Apr 2, 2020

Karsten1987 commented Apr 2, 2020

View reviewed changes

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp Outdated Show resolved Hide resolved

zmichaels11 approved these changes Apr 2, 2020

View reviewed changes

zmichaels11 suggested changes Apr 2, 2020

View reviewed changes

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp Outdated Show resolved Hide resolved

rosbag2_cpp/test/rosbag2_cpp/test_multifile_reader.cpp Outdated Show resolved Hide resolved

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp Outdated Show resolved Hide resolved

zmichaels11 reviewed Apr 2, 2020

View reviewed changes

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp Outdated Show resolved Hide resolved

emersonknapp approved these changes Apr 2, 2020

View reviewed changes

zmichaels11 approved these changes Apr 2, 2020

View reviewed changes

mabelzhang approved these changes Apr 2, 2020

View reviewed changes

Karsten1987 mentioned this pull request Apr 2, 2020

Enable play end-to-end tests when deterministic start is available #109

Open

emersonknapp force-pushed the fix_229 branch from e750353 to 35007dd Compare April 3, 2020 00:39

piraka9011 reviewed Apr 3, 2020

View reviewed changes

rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp Show resolved Hide resolved

piraka9011 approved these changes Apr 3, 2020

View reviewed changes

emersonknapp mentioned this pull request Apr 3, 2020

Disable adaptive qos subscription for now #348

Merged

mabelzhang mentioned this pull request Apr 3, 2020

rosbag2_py reader and writer #308

Merged

Karsten1987 force-pushed the fix_229 branch from d244af0 to 1a06b3d Compare April 3, 2020 15:36

Karsten1987 and others added 11 commits April 3, 2020 11:48

resolve relative file paths

5f6e145

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

remove subfolder in version4

d4da216

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Update rosbag2_cpp/test/rosbag2_cpp/test_multifile_reader.cpp

24f3ed9

Co-Authored-By: Zachary Michaels <zmichaels11@users.noreply.github.com>

Update rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp

a3e6ae3

Co-Authored-By: Zachary Michaels <zmichaels11@users.noreply.github.com>

address review comments

f7b195a

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

adopt rosbag2_tests

3199154

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

fix windows (hopefully)

180d550

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

uncrustify

e0fdb2c

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

fix compilation

6fa3d05

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

another try on windows

f8cfddd

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

windows try no 3

a710fe4

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 force-pushed the fix_229 branch from 2096166 to a710fe4 Compare April 3, 2020 18:48

Karsten1987 added 2 commits April 3, 2020 12:30

adopt sequentialreader to version4

32a9840

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

windows no 4

fc68a3f

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

windows no 5

3d6e7e7

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

windows no 6

107ae7b

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 merged commit 59c90c8 into master Apr 3, 2020

delete-merged-branch bot deleted the fix_229 branch April 3, 2020 23:11

jacobperron reviewed Jun 18, 2020

View reviewed changes

rosbag2_cpp/src/rosbag2_cpp/readers/sequential_reader.cpp Show resolved Hide resolved

llapx mentioned this pull request Sep 7, 2021

relative_file_paths issue when replaying compressed bag file #794

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resolve relative file paths #345

resolve relative file paths #345

Karsten1987 commented Apr 2, 2020

Karsten1987 commented Apr 2, 2020

zmichaels11 left a comment

Karsten1987 commented Apr 2, 2020

mabelzhang left a comment

Karsten1987 commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

piraka9011 left a comment

emersonknapp commented Apr 3, 2020 •

edited

Loading

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

piraka9011 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

resolve relative file paths #345

resolve relative file paths #345

Conversation

Karsten1987 commented Apr 2, 2020

Karsten1987 commented Apr 2, 2020

zmichaels11 left a comment

Choose a reason for hiding this comment

Karsten1987 commented Apr 2, 2020

mabelzhang left a comment

Choose a reason for hiding this comment

Karsten1987 commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

piraka9011 left a comment

Choose a reason for hiding this comment

emersonknapp commented Apr 3, 2020 • edited Loading

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

emersonknapp commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

piraka9011 commented Apr 3, 2020

Karsten1987 commented Apr 3, 2020

emersonknapp commented Apr 3, 2020 •

edited

Loading