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

Fix perfect_odometry rosbag #238

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Fix perfect_odometry rosbag #238

merged 2 commits into from
Jul 11, 2023

Conversation

nahueespinosa
Copy link
Member

@nahueespinosa nahueespinosa commented Jul 1, 2023

Proposed changes

Fixes #232.

This patch fixes message timestamps and starting time in the rosbag to match the header stamps recorded with sim time.

  • The contents of the messages were not altered.
  • The storage backend was changed to mcap.
  • The last messages where there was a collision against one of the walls were deleted.

Type of change

  • 🐛 Bugfix (change which fixes an issue)
  • 🚀 Feature (change which adds functionality)
  • 📚 Documentation (change which fixes or extends documentation)

Checklist

  • Lint and unit tests (if any) pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All commits have been signed for DCO

@nahueespinosa nahueespinosa added the bug Something isn't working label Jul 1, 2023
@nahueespinosa nahueespinosa self-assigned this Jul 1, 2023
Copy link
Collaborator

@glpuga glpuga left a comment

Choose a reason for hiding this comment

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

The topics LGTM, but when I tried to run the perfect odometry example rosbag does not start because the MCAP plugin is not installed.

After installing manually ros-humble-rosbag2-storage-mcap it worked just fine. We are probably missing a dependency declaration.

I had started the docker with ./run --build .

This patch fixes message timestamps in the rosbag to match the header
timestamps that were recorded using simulation time. The storage backend
was changed to mcap, the contents of the messages were not changed.
It also removes the last messages where there was a collision against one
of the walls.

Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
@nahueespinosa
Copy link
Member Author

@glpuga Good catch! I originally tested on Rolling.

I reverted to using sqlite3 instead of mcap so no need to add extra dependencies.

@nahueespinosa nahueespinosa requested a review from glpuga July 3, 2023 17:57
@glpuga
Copy link
Collaborator

glpuga commented Jul 9, 2023

I looked into the results of the beluga_benchmark that run on top of this. The data gathered looks good and can be plotted just fine for beluga_amcl, but nav2_amcl seems to be crashing right after starting.

This can be duplicated by running:

colcon build 
source install/setup.bash
cd src/beluga/beluga_benchmark/docs/reports/2023-06-03
mkdir test
cd test
ros2 run beluga_benchmark parameterized_run 250 300 400 500 750 1000 2000 5000 10000 20000 50000 100000 200000 --params-file ../ref_config/likelihood_params.yaml --package nav2_amcl --executable amcl
cd -

Nothing is left on the logs to indicate why nav2_amcl is giving up.

@nahueespinosa
Copy link
Member Author

@glpuga I can reproduce the error in Humble, but everything works fine in Rolling. More investigation is needed.

@nahueespinosa
Copy link
Member Author

nahueespinosa commented Jul 10, 2023

@glpuga I think the failure is most likely related to what @ivanpauno found here: #117 (comment)

I just checked and we have adaptive recovery disabled in those benchmarks, but I don't know if it's worth digging too hard if we know that the Humble version may crash unexpectedly.

We can benchmark using Rolling where the nav2_amcl bug was fixed.
I don't think that the issue is related to the changes in this PR.

@glpuga
Copy link
Collaborator

glpuga commented Jul 11, 2023

@glpuga I think the failure is most likely related to what @ivanpauno found here: #117 (comment)

I just checked and we have adaptive recovery disabled in those benchmarks, but I don't know if it's worth digging too hard if we know that the Humble version may crash unexpectedly.

We can benchmark using Rolling where the nav2_amcl bug was fixed. I don't think that the issue is related to the changes in this PR.

I see. I think it's ok we benchmark in rolling for the time being. Let's merge this then.

@nahueespinosa nahueespinosa merged commit 6e9d7e9 into main Jul 11, 2023
8 checks passed
@nahueespinosa nahueespinosa deleted the nahuel/fix-rosbag branch July 11, 2023 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeline mixup when running bagfile
2 participants