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

Single Launch file for Thorvald & Robotnik #7

Merged
merged 8 commits into from
Apr 13, 2021

Conversation

pulver22
Copy link
Contributor

@pulver22 pulver22 commented Apr 7, 2021

No description provided.

@pulver22 pulver22 added the enhancement New feature or request label Apr 7, 2021
@pulver22 pulver22 requested a review from sergimolina April 7, 2021 09:53
@pulver22 pulver22 self-assigned this Apr 7, 2021
@pulver22 pulver22 requested review from marc-hanheide and sergimolina and removed request for sergimolina April 12, 2021 11:03
@pulver22
Copy link
Contributor Author

Right now, the costmap is not generated when using multiple hokuyo, making the navigation impossible. I am going to include the laser merger, adjust the topics and fix this in a new commit.

Copy link
Member

@marc-hanheide marc-hanheide left a comment

Choose a reason for hiding this comment

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

I haven't really tested anything, but I don't see major issues with this.

@marc-hanheide
Copy link
Member

marc-hanheide commented Apr 12, 2021

I haven't really tested anything, but I don't see major issues with this.

well, other than the failed CI build ;-)

https://lcas.lincoln.ac.uk/buildfarm/job/Mpr__bacchus_lcas__ubuntu_bionic_amd64/16/testReport/

@pulver22
Copy link
Contributor Author

Yes I know that ;) I'm on it!

@pulver22
Copy link
Contributor Author

I haven't really tested anything, but I don't see major issues with this.

well, other than the failed CI build ;-)

https://lcas.lincoln.ac.uk/buildfarm/job/Mpr__bacchus_lcas__ubuntu_bionic_amd64/16/testReport/

This build fails because the test file assumes there is only one hokuyo generating the /thorvald_001/scan topic. This condition is not met when using the laser merger which, yes, generates this topic but should be launch in a second terminal window at a later stage in order to correctly subscribe to the two laser topics and generate the merged one (see HERE).

If we want to keep the unit test as they are, I will modify the launch file to load different sensor configuration based on a parameter and by default use the one with only one hokuyo (so to pass the test).

The test should now pass
@pulver22 pulver22 merged commit 30b5dca into LCAS:master Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants