intel / collision-avoidance-library

A framework for testing and benchmarking collision avoidance strategies
Apache License 2.0
81 stars 39 forks source link

Obstacle detector #51

Closed mbelluzzo closed 7 years ago

mbelluzzo commented 7 years ago

All expect the last commit should be good to go. Last commit is here to help drive discussion because I think there are still things pending. See notes at the commit itself.

mbelluzzo commented 7 years ago

Hhmm I can double check that.

On Wed, Jan 18, 2017, 07:10 Rodrigo Chiossi notifications@github.com wrote:

@rchiossi commented on this pull request.

In utils/PPTree.cc https://github.com/01org/collision-avoidance-library/pull/51:

@@ -24,8 +24,8 @@ Set::Set()

Set *Set::repr() {

  • if (this->parent == this)
  • return this;
  • if (!this->parent || this->parent == this)

yeah, I think a warning should be enough.

In detection/DepthImageObstacleDetector.cc https://github.com/01org/collision-avoidance-library/pull/51:

{ this->sensor = depth_camera; this->obstacles.resize(MAX_NUM_BLOBS);

  • this->threshold = (uint16_t)(threshold_meters / depth_camera->get_scale());

Ok. I'll change in my patch and see if there are other places that needs the change.

In detection/DepthImageObstacleDetector.hh https://github.com/01org/collision-avoidance-library/pull/51:

@@ -35,6 +35,8 @@ class DepthImageObstacleDetector : public Detector<DepthCamera, Obstacle> double fov; double scale;

  • bool is_valid(uint16_t depth);

well, when you see a call you see whatever the variable sent to the function is called, not what it is called in the prototype. But that's a minor issue. I'm ok to just keep it as is.

In detection/DepthImageObstacleDetector.cc https://github.com/01org/collision-avoidance-library/pull/51:

     for (int j = 0; j < this->width; j++) {
  • if (is_valid(this->depth_frame[i * this->width + j])) {
  • if (is_valid(this->depth_frame[row_offset + j])) { std::vector neigh_labels(4);
             int num_neighbors = get_neighbors_label(i, j, neigh_labels.data());

Well, C++ is not my strongest side, but I thought you needed to provided a second parameter to resize() to set a default value for the new entries in the vector. I don't think uint16_t is automatically zeroed, but I'm not sure.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/01org/collision-avoidance-library/pull/51, or mute the thread https://github.com/notifications/unsubscribe-auth/ABgsZeG4aF9QgSaUOh1zjzjVxNwTrtcvks5rTit2gaJpZM4Lia5N .