Update this repo with the latest STag fork

[JasperTan97]

Currently, there are too many problems with memory leaks due to the use of raw pointers instead of smart ones. Fortunately the original Stag code has been updated to (hopefully) solve this problem. However, we will need to ensure that issues 1 to 4 are either replicated or somehow brought forward such that the fork is compatible with the pose detection code.

Tasks

Beta Give feedback

1. https://github.com/aica-technology/test_stag/pull/1
2. https://github.com/aica-technology/dynamic-components/issues/359
3. Figure out the best way to merge the 2 repos
4. Update Pose detection cmakelist to have the latest stag repo name

[JasperTan97]

Functionally, the parts of the stag code that is being used is:

bool STagDetector::configure() {
  stag_ = std::make_shared<stag::Stag>(library_->get_value(), error_correction_->get_value(), false);
  return BaseMarkerDetector::configure();
}

std::map<int, std::vector<cv::Point2d>> STagDetector::detect_markers(const cv::Mat& image) {
  cv::Mat gray;
  cv::cvtColor(image, gray, cv::COLOR_RGB2GRAY);
  this->stag_->detectMarkers(gray);
  annotated_image_ = this->stag_->drawMarkers();

  std::map<int, std::vector<cv::Point2d>> markers;
  for (const auto& marker: this->stag_->getMarkerList()) {
    markers.insert_or_assign(marker.id, marker.corners);
  }
  return markers;
}

So I am looking at the part of the code that invokes the Stag class found here, and it uses 3 functions.

1. detectMarkers which was originally part of the code
2. drawMarkers which was resolved in Draw markers #2. We find that it's also added here
3. getMarkerList which was also resolved in Draw markers #2, which is here as well.

And after, there's some clean up with seperating headers and cpp file, as well as making sure all the installation and compilation scripts are working.

We could try to update our Stag library to the latest version, and it would be easier to update down the line too (maybe). Let me know what you think.

Originally posted by @JasperTan97 in https://github.com/aica-technology/stag/issues/6#issuecomment-1916680198

[JasperTan97]

The way I shall do this, after discussion with @eeberhard, is to fork the latest stag repo in a test repo, before merging it into this one.

[eeberhard]

Thanks! When referring to the latest Stag fork, we are talking about https://github.com/manfredstoiber/stag

[JasperTan97]

After updating, here are some learning points. The major changes to the stag library involves shifting from a Stag object to a stag namespace, where the pros and cons can be summarised by an AI chatbot as:

Approach 1 (Using a Stag object):

Pros:

Encapsulation: The Stag object encapsulates all the data and methods related to marker detection and drawing. This makes the code easier to read and maintain.
Flexibility: Since the Stag object is created dynamically, it allows for more flexibility in terms of configuration. For example, you can easily change the parameters of the Stag object without affecting other parts of the code.

Cons:

Memory Management: Dynamic memory allocation can lead to memory leaks if not handled properly. However, this is mitigated by using smart pointers like std::shared_ptr.
Performance: Creating a new object every time might be less efficient than reusing existing objects, especially if the object creation process is expensive.

Approach 2 (Accessing functions in a Stag namespace):

Pros:

Simplicity: This approach is simpler and more straightforward. It doesn't require creating and managing objects.
Efficiency: Since there's no dynamic memory allocation involved, it could be more efficient.

Cons:

Lack of Encapsulation: All the data and methods are in the global namespace, which can lead to naming conflicts and make the code harder to understand and maintain.
Less Flexibility: Changing the behavior of the marker detection and drawing process would require changing the function calls directly, which might not be as flexible as using an object.

To be precise, there is still a (stag, now named) stag_detector object, but it is created and destroyed when the detect_marker function is called and gone out of scope respectively.

Perhaps there were missing new or delete when we tried in #6, but it was too complicated and shall remain a mystery. In any case, the latest update seems stable and successful.

[JasperTan97]

Alright this is now an EPIC, to keep track of everything properly

[eeberhard]

To consider as a followup:

• Add mutex around step callback modulo#67