Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Make lidar measurement model class #195
Make lidar measurement model class #195
Changes from 20 commits
e619675
81cf634
0a7a366
7e6e2ad
2b8bff5
26e606b
4ba9695
2982960
59645ad
c7de317
75d130c
4c5350c
7caf1e0
3c87670
2d5d062
d024a99
a28be14
6c4befc
5cd5ff2
9a8a451
0d8990a
f9444b8
b37c72e
de81133
b2e8e71
a038fad
257a144
49572be
239b387
2efa4a7
cca96e3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
define measurement result type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I've forgotten this. thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LidarMeasurementModelBeam looks like not independent from the specific point type as it access to intensity field in the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when you add support new point type, proper way can take slightly different, so that I think it's ok to create different classes depending on point type. do you think so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my previous opinion, I would like to add specialized nodes for each input point type (PointXYZ and PointXYZI) to increase run-time performance as far as possible, reducing type conversion.
But this is not very much critical and simply converting point type at the subscriber callback should be reasonable.