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

UTM conversions using geographiclib #626

Merged
merged 5 commits into from
Apr 12, 2021

Conversation

Timple
Copy link
Contributor

@Timple Timple commented Feb 23, 2021

For #222 it made sense to use the GeographicLib library that #575 already introduced.

In preparation I rewrote the current UTM functions.

@ayrton04
Copy link
Collaborator

Thanks for this, and for the tests. Are you happy to have this reviewed now?

@Timple
Copy link
Contributor Author

Timple commented Mar 22, 2021

Certainly.


/**
* Convert lat/long to UTM coords. Equations from USGS Bulletin 1532
* Convert lat/long to UTM coords.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also get rid of all of this? Or do we still need it?

const double RADIANS_PER_DEGREE = M_PI/180.0;
const double DEGREES_PER_RADIAN = 180.0/M_PI;
// Grid granularity for rounding UTM coordinates to generate MapXY.
const double grid_size = 100000.0; // 100 km grid
// WGS84 Parameters
#define WGS84_A 6378137.0 // major axis
#define WGS84_B 6356752.31424518 // minor axis
#define WGS84_F 0.0033528107 // ellipsoid flattening
#define WGS84_E 0.0818191908 // first eccentricity
#define WGS84_EP 0.0820944379 // second eccentricity
// UTM Parameters
#define UTM_K0 0.9996 // scale factor
#define UTM_FE 500000.0 // false easting
#define UTM_FN_N 0.0 // false northing, northern hemisphere
#define UTM_FN_S 10000000.0 // false northing, southern hemisphere
#define UTM_E2 (WGS84_E*WGS84_E) // e^2
#define UTM_E4 (UTM_E2*UTM_E2) // e^4
#define UTM_E6 (UTM_E4*UTM_E2) // e^6
#define UTM_EP2 (UTM_E2/(1-UTM_E2)) // e'^2
/**
* Utility function to convert geodetic to UTM position
*
* Units in are floating point degrees (sign for east/west)
*
* Units out are meters
*
* @todo deprecate this interface in favor of LLtoUTM()
*/
static inline void UTM(double lat, double lon, double *x, double *y)
{
// constants
static const double m0 = (1 - UTM_E2/4 - 3*UTM_E4/64 - 5*UTM_E6/256);
static const double m1 = -(3*UTM_E2/8 + 3*UTM_E4/32 + 45*UTM_E6/1024);
static const double m2 = (15*UTM_E4/256 + 45*UTM_E6/1024);
static const double m3 = -(35*UTM_E6/3072);
// compute the central meridian
int cm = ((lon >= 0.0)
? (static_cast<int>(lon) - (static_cast<int>(lon)) % 6 + 3)
: (static_cast<int>(lon) - (static_cast<int>(lon)) % 6 - 3));
// convert degrees into radians
double rlat = lat * RADIANS_PER_DEGREE;
double rlon = lon * RADIANS_PER_DEGREE;
double rlon0 = cm * RADIANS_PER_DEGREE;
// compute trigonometric functions
double slat = sin(rlat);
double clat = cos(rlat);
double tlat = tan(rlat);
// decide the false northing at origin
double fn = (lat > 0) ? UTM_FN_N : UTM_FN_S;
double T = tlat * tlat;
double C = UTM_EP2 * clat * clat;
double A = (rlon - rlon0) * clat;
double M = WGS84_A * (m0*rlat + m1*sin(2*rlat)
+ m2*sin(4*rlat) + m3*sin(6*rlat));
double V = WGS84_A / sqrt(1 - UTM_E2*slat*slat);
// compute the easting-northing coordinates
*x = UTM_FE + UTM_K0 * V * (A + (1-T+C)*pow(A, 3)/6
+ (5-18*T+T*T+72*C-58*UTM_EP2)*pow(A, 5)/120);
*y = fn + UTM_K0 * (M + V * tlat * (A*A/2
+ (5-T+9*C+4*C*C)*pow(A, 4)/24
+ ((61-58*T+T*T+600*C-330*UTM_EP2)
* pow(A, 6)/720)));
return;
}

And if we do get rid of that, can we also get rid of the other comments and update the license?

const double RADIANS_PER_DEGREE = M_PI/180.0;
const double DEGREES_PER_RADIAN = 180.0/M_PI;
// Grid granularity for rounding UTM coordinates to generate MapXY.
const double grid_size = 100000.0; // 100 km grid
// WGS84 Parameters
#define WGS84_A 6378137.0 // major axis
#define WGS84_B 6356752.31424518 // minor axis
#define WGS84_F 0.0033528107 // ellipsoid flattening
#define WGS84_E 0.0818191908 // first eccentricity
#define WGS84_EP 0.0820944379 // second eccentricity
// UTM Parameters
#define UTM_K0 0.9996 // scale factor
#define UTM_FE 500000.0 // false easting
#define UTM_FN_N 0.0 // false northing, northern hemisphere
#define UTM_FN_S 10000000.0 // false northing, southern hemisphere
#define UTM_E2 (WGS84_E*WGS84_E) // e^2
#define UTM_E4 (UTM_E2*UTM_E2) // e^4
#define UTM_E6 (UTM_E4*UTM_E2) // e^6
#define UTM_EP2 (UTM_E2/(1-UTM_E2)) // e'^2
/**
* Utility function to convert geodetic to UTM position
*
* Units in are floating point degrees (sign for east/west)
*
* Units out are meters
*
* @todo deprecate this interface in favor of LLtoUTM()
*/
static inline void UTM(double lat, double lon, double *x, double *y)
{
// constants
static const double m0 = (1 - UTM_E2/4 - 3*UTM_E4/64 - 5*UTM_E6/256);
static const double m1 = -(3*UTM_E2/8 + 3*UTM_E4/32 + 45*UTM_E6/1024);
static const double m2 = (15*UTM_E4/256 + 45*UTM_E6/1024);
static const double m3 = -(35*UTM_E6/3072);
// compute the central meridian
int cm = ((lon >= 0.0)
? (static_cast<int>(lon) - (static_cast<int>(lon)) % 6 + 3)
: (static_cast<int>(lon) - (static_cast<int>(lon)) % 6 - 3));
// convert degrees into radians
double rlat = lat * RADIANS_PER_DEGREE;
double rlon = lon * RADIANS_PER_DEGREE;
double rlon0 = cm * RADIANS_PER_DEGREE;
// compute trigonometric functions
double slat = sin(rlat);
double clat = cos(rlat);
double tlat = tan(rlat);
// decide the false northing at origin
double fn = (lat > 0) ? UTM_FN_N : UTM_FN_S;
double T = tlat * tlat;
double C = UTM_EP2 * clat * clat;
double A = (rlon - rlon0) * clat;
double M = WGS84_A * (m0*rlat + m1*sin(2*rlat)
+ m2*sin(4*rlat) + m3*sin(6*rlat));
double V = WGS84_A / sqrt(1 - UTM_E2*slat*slat);
// compute the easting-northing coordinates
*x = UTM_FE + UTM_K0 * V * (A + (1-T+C)*pow(A, 3)/6
+ (5-18*T+T*T+72*C-58*UTM_EP2)*pow(A, 5)/120);
*y = fn + UTM_K0 * (M + V * tlat * (A*A/2
+ (5-T+9*C+4*C*C)*pow(A, 4)/24
+ ((61-58*T+T*T+600*C-330*UTM_EP2)
* pow(A, 6)/720)));
return;
}

const double RADIANS_PER_DEGREE = M_PI/180.0;
const double DEGREES_PER_RADIAN = 180.0/M_PI;
// Grid granularity for rounding UTM coordinates to generate MapXY.
const double grid_size = 100000.0; // 100 km grid
// WGS84 Parameters
#define WGS84_A 6378137.0 // major axis
#define WGS84_B 6356752.31424518 // minor axis
#define WGS84_F 0.0033528107 // ellipsoid flattening
#define WGS84_E 0.0818191908 // first eccentricity
#define WGS84_EP 0.0820944379 // second eccentricity
// UTM Parameters
#define UTM_K0 0.9996 // scale factor
#define UTM_FE 500000.0 // false easting
#define UTM_FN_N 0.0 // false northing, northern hemisphere
#define UTM_FN_S 10000000.0 // false northing, southern hemisphere
#define UTM_E2 (WGS84_E*WGS84_E) // e^2
#define UTM_E4 (UTM_E2*UTM_E2) // e^4
#define UTM_E6 (UTM_E4*UTM_E2) // e^6
#define UTM_EP2 (UTM_E2/(1-UTM_E2)) // e'^2
/**
* Utility function to convert geodetic to UTM position
*
* Units in are floating point degrees (sign for east/west)
*
* Units out are meters
*
* @todo deprecate this interface in favor of LLtoUTM()
*/
static inline void UTM(double lat, double lon, double *x, double *y)
{
// constants
static const double m0 = (1 - UTM_E2/4 - 3*UTM_E4/64 - 5*UTM_E6/256);
static const double m1 = -(3*UTM_E2/8 + 3*UTM_E4/32 + 45*UTM_E6/1024);
static const double m2 = (15*UTM_E4/256 + 45*UTM_E6/1024);
static const double m3 = -(35*UTM_E6/3072);
// compute the central meridian
int cm = ((lon >= 0.0)
? (static_cast<int>(lon) - (static_cast<int>(lon)) % 6 + 3)
: (static_cast<int>(lon) - (static_cast<int>(lon)) % 6 - 3));
// convert degrees into radians
double rlat = lat * RADIANS_PER_DEGREE;
double rlon = lon * RADIANS_PER_DEGREE;
double rlon0 = cm * RADIANS_PER_DEGREE;
// compute trigonometric functions
double slat = sin(rlat);
double clat = cos(rlat);
double tlat = tan(rlat);
// decide the false northing at origin
double fn = (lat > 0) ? UTM_FN_N : UTM_FN_S;
double T = tlat * tlat;
double C = UTM_EP2 * clat * clat;
double A = (rlon - rlon0) * clat;
double M = WGS84_A * (m0*rlat + m1*sin(2*rlat)
+ m2*sin(4*rlat) + m3*sin(6*rlat));
double V = WGS84_A / sqrt(1 - UTM_E2*slat*slat);
// compute the easting-northing coordinates
*x = UTM_FE + UTM_K0 * V * (A + (1-T+C)*pow(A, 3)/6
+ (5-18*T+T*T+72*C-58*UTM_EP2)*pow(A, 5)/120);
*y = fn + UTM_K0 * (M + V * tlat * (A*A/2
+ (5-T+9*C+4*C*C)*pow(A, 4)/24
+ ((61-58*T+T*T+600*C-330*UTM_EP2)
* pow(A, 6)/720)));
return;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can.
I left them in because they are exposed to other packages. So I'm not sure who might be using these variables...

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a good point. I'll bring this forward to the ROS2 branches, and for Galactic, I'll remove them. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the whole conversions file can be removed in that case. Since there is now a library doing all that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ayrton04 do you still plan for removing these (quite empty) functions for ROS2 and galactic?

Seems like they are re-used: #676
Which is a bit of a waste.

test/test_navsat_conversions.cpp Outdated Show resolved Hide resolved
@ayrton04
Copy link
Collaborator

@SteveMacenski, anything glaring to you on this or #627?

@SteveMacenski
Copy link
Collaborator

I haven't used geographic lib / GPS before in a product so I can't say I have an opinion. Though I'd want a PR analog to this for ROS2 to keep them in sync

@Timple
Copy link
Contributor Author

Timple commented Mar 25, 2021

I could do that. Which branch(es) would I need to target? And which of those would allow breaking changes?

@SteveMacenski
Copy link
Collaborator

The ros2 branch would allow breaking changes

@Timple
Copy link
Contributor Author

Timple commented Apr 6, 2021

Though I'd want a PR analog to this for ROS2 to keep them in sync

#643

I could not target the ROS2 branch because it's too much behind on Foxy.

@SteveMacenski
Copy link
Collaborator

SteveMacenski commented Apr 6, 2021

@Timple what do you mean, I'm looking at the ROS2->Foxy diff right here its only off by 4 commits that are just rclcpp API changes (only covering like ~80 lines). We need any changes to Foxy to be in ros2 as well or else this change will never be reflected into future distributions and we'll be back here every 12 months

@Timple
Copy link
Contributor Author

Timple commented Apr 8, 2021

Well that is strange. Than how come there is no mention of geographiclib in the ros2 package.xml but there is in the foxy-devel branch?

Did a merge go wrong somewhere?

@SteveMacenski
Copy link
Collaborator

It looks like those were added 2 weeks ago only in the ROS1 feature sync PR 3dbd48a

@ayrton04 is it your intention to merge this into ros2? Any conflicts you'd expect from cherry picking between foxy and ros2?

@ayrton04
Copy link
Collaborator

I can't think of anything offhand.

For the record, the model that I originally tried to follow was;

  1. Create a new <release_name>-devel branch prior to the release being finalized
  2. Let all the new featured (and ABI breakages) go into <release_name>-devel
  3. When <release_name>-devel is released, that branch should only accept bug fixes or minor features that don't break ABI
  4. When <release_name>-devel is released, we also create <next_release_name>-devel and start putting both the bug fixes from <release_name>-devel and new features into <next_release_name>-devel.

Obviously, there have been some violations of that paradigm, but that's generally the way I wanted to do things.

The ros2 branch suggests a slightly different model, I think, though I'm not sure what we do for new ROS2 releases.

  1. All new features and bug fixes to any <release_name>-devel branch should find their way to ros2
  2. If a new ROS2 release is cut, do we then branch ros2 into <previous_release>-devel and let all future development happen on the ros2 branch? Or do we just keep using ros2 for all ROS2 releases until ABI breakage or incompatibility with the rest of the stack for a given ROS2 release forces us to branch?

@ayrton04 ayrton04 merged commit 5eec32b into cra-ros-pkg:noetic-devel Apr 12, 2021
@Timple
Copy link
Contributor Author

Timple commented Apr 12, 2021

Both suggestions result in the ros2 branch to be some kind of a 'rolling' branch allowing breaking changes.

All new features and bug fixes to any <release_name>-devel branch should find their way to ros2

Certainly this.


So I'll target the foxy-devel for the backwards compatibility I build in. And I'll target the ros2 (once it gets updated) to include the breaking changes proposed.

@SteveMacenski
Copy link
Collaborator

SteveMacenski commented Apr 12, 2021

The new ROS2-y workflow is

  • Have a master ros2 or similarly named branch that all development is done on, targeting ROS2 rolling / master builds
  • When you release a new distro, you branch off ros2 at the time of a new distro and that's the new distro release to use
  • Over time, sync the distro and ros2 branches by cherry picking commits from ros2 to the distro that are ABI compatible for release to those distros

Depending on my laziness, I also sometimes ask people to target their PR to multiple branches (ros2, and foxy, and noetic or something) to avoid the cherry picking later / forgetting.

ayrton04 pushed a commit that referenced this pull request Jun 3, 2021
* Use GeographicLib for UTMtoLL conversions
@Timple Timple mentioned this pull request Dec 22, 2023
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.

3 participants