Skip to content

Patch for PhotoAtWaypoint plugin. #2066

Closed
jediofgever wants to merge 10 commits intoros-navigation:mainfrom
jediofgever:main
Closed

Patch for PhotoAtWaypoint plugin. #2066
jediofgever wants to merge 10 commits intoros-navigation:mainfrom
jediofgever:main

Conversation

@jediofgever
Copy link
Copy Markdown
Contributor

#2041 Introduced PhotoAtWaypoint plugin.

A directory path should be given to plugin for saving the photos. If the given directory does not exist we create one with std::experimental::filesystem, but this may throw exceptions if the given path is an invalid one. So a try catch block should surround that part of the code where we try to create directory.
This PR has that.

}
}
} catch (const std::exception & e) {
std::cerr << e.what() << '\n';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use proper logging please

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also if an exception was thrown, is_enabled_ should be false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm unfortunately I have no longer access to my laptop after today(for an indefinite time). Could you take care of this for me please ?
Thank you

@SteveMacenski
Copy link
Copy Markdown
Member

@jediofgever done, I need to open a new PR to merge in those changes since it was on your private fork

@SteveMacenski
Copy link
Copy Markdown
Member

See #2067

@jediofgever
Copy link
Copy Markdown
Contributor Author

@SteveMacenski , thanks a lot , had some issues with my PC and sent it to service.

@SteveMacenski
Copy link
Copy Markdown
Member

Please review #2067

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