Skip to content

Conversation

acschang
Copy link
Contributor

These changes only apply to models in the submitted_models folder.

I tested all changes to LIDAR parameters by checking the points topic with rostopic echo <topic_name> --noarr and visual inspection in RViz.

…nsor. Removed a handful of unnecessary comments from other model.sdfs as it pertains to the 160,000 ray limit.
@osrf-jenkins
Copy link

Build finished. 15 tests run, 0 skipped, 0 failed.

@peci1
Copy link
Collaborator

peci1 commented Jan 19, 2021

As pointed out in #691, shouldn't this also affect the default X1-X4 robots?

https://bitbucket.org/ignitionrobotics/ign-sensors/issues/8 -->
<samples>10000</samples>
<samples>1800</samples>
<resolution>0.1</resolution>

Choose a reason for hiding this comment

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

I suggest increasing horizontal resolution to 1 as well. Otherwise I guess accuracy could become far inferior to physical sensor.

Copy link
Contributor Author

@acschang acschang Jan 19, 2021

Choose a reason for hiding this comment

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

I suggest increasing horizontal resolution to 1 as well. Otherwise I guess accuracy could become far inferior to physical sensor.

I agree that the parameter should be 1.0 however whenever I experimentally tested the impact of changing resolution I could not find any difference between. This could be indicative of a larger problem with horizontal LIDAR ray resolution.

Experimentally I tested values of 1.0, 0.1, and 0.01 in the resolution parameter for horizontal returns without luck.

Choose a reason for hiding this comment

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

Probably you are right. May be gpu ray sensor doesn't even use that parameter. (Just a thought need to check code to confirm) Or the way we think what that parameter is for, is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not currently factor in that parameter. I'm looking into the issue a bit more before submitting an issue on the Ignition repositories.

@AravindaDP
Copy link

As @peci1 pointed out models of default X1-X4 robots would also need update. But I guess they are in a private repo.

@acschang
Copy link
Contributor Author

acschang commented Jan 19, 2021

As pointed out in #691, shouldn't this also affect the default X1-X4 robots?

As @peci1 pointed out models of default X1-X4 robots would also need update. But I guess they are in a private repo.

The X1-X4 vehicles are version controlled through Ignition fuel as opposed to through this repository. A separate request has been put in to update those models directly on Ignition fuel.

@nkoenig
Copy link
Contributor

nkoenig commented Jan 20, 2021

The models on Fuel have been updated.

@nkoenig nkoenig merged commit a2fb8c6 into master Jan 20, 2021
@nkoenig nkoenig deleted the submitted_models/vlp16_correction branch September 27, 2021 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants