Skip to content
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

Nbuono/vo #332

Merged
merged 21 commits into from
Dec 3, 2024
Merged

Nbuono/vo #332

merged 21 commits into from
Dec 3, 2024

Conversation

ewfuentes
Copy link
Owner

No description provided.

Copy link
Owner Author

@ewfuentes ewfuentes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Nico, as I mentioned before, this PR looks good to me, but there are a bunch of style changes I'd like to see. Happy to chat if you've got questions.

DataParser(const std::string &image_root_dir, const std::vector<std::string> &survey_list);
~DataParser();

const symphony_lake_dataset::SurveyVector &getSurveys() const { return _surveys; };
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of the codebase uses snake case for function names.


#include "symphony_lake_dataset/SurveyVector.h"

namespace robot::experimental::learn_descriptors::symphony_lake_parser {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, the namespace should match the file path to make things easier to find. Remove symphony_lake_parser from the namespace.

const symphony_lake_dataset::SurveyVector &getSurveys() const { return _surveys; };

private:
symphony_lake_dataset::SurveyVector _surveys;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

member variables should be suffixed with an underscore instead of prefixed.

Comment on lines 17 to 18
std::string image_root_dir = "/home/pizzaroll04/Documents/datasets/symphony_lake_full";
std::string surveys = "140106";
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be const

void hello_world(const std::string &msg);
class DataParser {
public:
DataParser(const std::string &image_root_dir, const std::vector<std::string> &survey_list);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image_root_dir can be a std::filesystem::path

cv::Ptr<cv::Feature2D> _feature_extractor;
cv::Ptr<cv::DescriptorMatcher> _descriptor_matcher;
};
class VO {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider calling this VisualOdometry instead.

@PizzaRoll04 PizzaRoll04 merged commit b6a9feb into main Dec 3, 2024
5 checks passed
@PizzaRoll04 PizzaRoll04 deleted the nbuono/VO branch December 3, 2024 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants