Repository navigation
Feat/traversability cleanup - #206
Conversation
nathanhhughes
left a comment
nathanhhughes
left a comment
There was a problem hiding this comment.
We definitely need an archival mechanism for the footprint integrator, but otherwise LGTM! Would like to eventually clean up the TSDF / traversability layer interaction and how resetting works, but probably better in a follow-on PR (or might take a pass at this myself depending on timing)
| protected: | ||
| TraversabilityEstimator::Ptr estimator_; | ||
| const TraversabilityProcessors postprocessing_; | ||
| //! Persistent traversability layer, allocated on the first update. |
There was a problem hiding this comment.
(minor) protected members don't need documentation
| /** | ||
| * @brief Processor whose changes persist, i.e., applied to the stored traversability | ||
| * layer. | ||
| */ |
There was a problem hiding this comment.
(minor) //! Docstring is shorthand for a single @brief entry (and I would prefer single line docstrings when possible)
| //! @brief Minimum corner of the robot footprint in body frame [m]. Only x/y are | ||
| //! used. |
There was a problem hiding this comment.
ditto for single line docstring
| // TODO(lschmid): Find a nicer way than copying the layer here. Should not be too | ||
| // expensive though. |
There was a problem hiding this comment.
Can drop the TODO now that the transient processors require copying the layer
| Eigen::Vector3f bbox_min = Eigen::Vector3f::Zero(); | ||
| //! @brief Maximum corner of the robot footprint in body frame [m]. Only x/y are | ||
| //! used. | ||
| Eigen::Vector3f bbox_max = Eigen::Vector3f::Zero(); |
There was a problem hiding this comment.
Would prefer these to be Eigen::Vector2f given the usage inside the class
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm okay with this as an implementation as long as it archives indices from the footprint. Two concerns: (i) the memory grows without bound and (ii) old footprint indices would mark things as traversable that shouldn't be due to drift or multi-floor environments
| //! Latest received grid, guarded by mutex_. Copied into the extractor's layer on | ||
| //! every update. |
There was a problem hiding this comment.
(minor) no protected docs needed
2 participants
No description provided.