# Working and calculating with geographic positions

I was thinking about how to implement a GPS navigation for my quadcopter. I was writing some functions to calculate the angle between two positions (from home point to the target).

I'd like a review of this code. I am not very sure whether I was calculating the geographic position right.

/*
* Geographic calculations
*/
float UAVNav::width_of_merid_m(const float fLat_deg) {
fdX_m /= 360.f;
return fdX_m;
}

float UAVNav::dist_2_equat_m(const float fLat_deg) {
return (_PERIMETER_EARTH_m / 360.f) * fLat_deg;
}

float UAVNav::dist_2_greenw_m(const float fLat_deg, const float fLon_deg) {
return width_of_merid_m(fLat_deg) * fLon_deg;
}

float UAVNav::home_2_target_deg() {
const float fMod = 10000000.f;
GPSData home = m_pHalBoard->get_gps();

float fLatHome_deg = home.latitude / fMod;
float fLonHome_deg = home.longitude / fMod;

float fLatTarg_deg = target.latitude / fMod;
float fLonTarg_deg = target.longitude / fMod;

float fXHome = dist_2_greenw_m(fLatHome_deg, fLonHome_deg);
float fYHome = dist_2_equat_m(fLatHome_deg);

float fXTarg = dist_2_greenw_m(fLatTarg_deg, fLonTarg_deg);
float fYTarg = dist_2_equat_m(fLatTarg_deg);

float dX = fXTarg - fXHome;
float dY = fYTarg - fYHome;

// Calculate the angle to the destination direction
// NOTE: Here the system is inverted by 90° to align the magnetic north to 0°
m_fDestin_deg = ToDeg(atan2((float)dX, (float)dY) );
return m_fDestin_deg;
}


There are some temporary variables that you might not use, such as fLatTarg_deg and fLotTarg_deg: you can commit the operation along with the function call.

Also, the variables (which are actually costants, probably) _RADIUS_EARTH_m and _PERIMETER_EARTH_m are breaking the standard C++ naming conventions.

• @syb0rg Hope you won't mind, but did you seriously edit my answer to add a new line? – edmz Apr 1 '14 at 16:33
• It seems like he did. Isn't it better that way ? – SylvainD Apr 1 '14 at 16:50
• @no1 I was out of votes for the day, and I had used one of my last votes on your answer. Then I bought the unicorn animation, and I may have wanted to test it out. ;) – syb0rg Apr 1 '14 at 18:00
• @syb0rg shame on you, if you spend 40 votes, why not buy the unicorn animation before doing so ?? – Vogel612 Apr 1 '14 at 18:42

You naming convention is a bit weird. My assumption is that fLat stands for "float latitude". Because it is quite easy to know the type of a variable, there's no need for having the f prefix.

You don't need to cast to float in m_fDestin_deg = ToDeg(atan2((float)dX, (float)dY) );.

You don't need the temporary variable in UAVNav::width_of_merid_m especially as it makes the mathematical formula harder to read.

Your method UAVNav::home_2_target_deg() seems to be doing two things : returning a value and updating a member. It would be clearer to have a (const) method returning a value and another method using it to set a member.

• You don't have std:: in front of cos() and atan2() (which belong to <cmath>), which means you're using using namespace std. This is discouraged as it could cause bugs in cases of name-clashing with existing names used in this namespace (or any namespace, although std is most commonly used). See this for more information.

• I agree with @Josay about the casting being unnecessary in atan2(). Its two arguments are already float, so casting will do nothing.

Moreover, when you do need casts like these, prefer static_cast<>() in C++:

static_cast<type>(object);


• @dgrat: Okay. Without showing the #includes, I would not have been able to tell. – Jamal Apr 9 '14 at 14:18