-
Notifications
You must be signed in to change notification settings - Fork 90
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
Feature/enu frame #126
Feature/enu frame #126
Conversation
Ready to review @michaelpantic |
geotf_.initFromRosParam(); | ||
if (geotf_.hasFrame("enu")) reset_position_ = ResetEnuOrigin::kNo; | ||
geotf_.addFrameByEPSG("ecef", 4978); | ||
geotf_.addFrameByEPSG("wgs84", 4326); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that this is likely to happen, but could lead to confusion - what would happen if someone configures some other frame as 4978 as ecef in the rosparams? then this would fail and return weird results.
Maybe we could check return value of addFrameByEPSG (https://github.com/ethz-asl/geodetic_utils/blob/master/geotf/src/geodetic_converter.cc#L90), it gives false if the frame is already defined and output an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment about frame definitions (not so important but could be confusing if it ever happens, as some frames have multiple definitions, e.g. there are different ECEF frames)
otherwise great !!
One approach to publish a position in ENU frame. Does not implement enu with covariance, yet.
#125 and ethz-asl/geodetic_utils#40 should be merged first. Addresses ENU publishing in #102.