image_pipeline

image_pipeline copied to clipboard

this pull request attempts to fix #924

changes:

• [x] changed the argument name range_max which was missleading to invalid_depth which captures the true meaning of the paramter
• [x] Introduction of a parameter called invalid_depth, which is the value that will be assigned to points that are not valid according to DepthTraits<T>::valid
• [x] small fixes regarding use of non const shared ptr references
• [x] changed for loop from int to uint32_t avoiding many type casts

Open:

• [ ] UB when reinterpret casting the pointcloud data?
• [ ] introduction of a true "max_depth" or "max_range" paramter

Comment: i think the term range and depth is not always used consistently, my understanding is range meaning point to point distance, and depth point to camera plane distance

in the meantime i further research the UB topic, and after watching Video i am now fairly certain, that most use of reinterpret_cast is actually UB. Should i raise a new issue dedicated to this?

Yeah, go ahead and open a new issue where we can further investigate the reinterpret_cast

Thanks for the feedback. As soon as i have time, i will implement the changes.

• [x] added invalid_depth to parameter description in the docs
• [x] removed comment
• [x] added the invalid_depth to the other non radial nodes as well.

If there is something more to do, please let me know!

This looks good - thanks for the contribution - just waiting on the final CI to pass

i just clicked update branch which i think was a mistake, since now the ci is waiting again. sorry for the confusion :)