udacity / udacidrone

An API for working with flying objects, simulated, unidentified, and otherwise.
https://udacity.github.io/udacidrone
129 stars 96 forks source link

frame_utils.py ignores global_home[2] altitude #41

Open petercerno opened 6 years ago

petercerno commented 6 years ago

Hi,

I have noticed that both methods global_to_local(global_position, global_home) and local_to_global(local_position, global_home) ignore global_home[2] altitude. Do we assume that global_home[2] == 0? If so, I would suggest to state this explicitly in the comments. Otherwise, the code should be modified like this:

def global_to_local(global_position, global_home):
    ...
    local_position = np.array([
        north - north_home,
        east - east_home,
        -(global_position[2] - global_home[2])])
    return local_position

def local_to_global(local_position, global_home):
    ...
    lla = np.array([lon, lat, -local_position[2] + global_home[2]])
    return lla
domluna commented 6 years ago

@petercerno global_home[2] just isn't relevant to the conversion. Local and global position measure altitude by the same the same units (meters). The only thing we do is tack on or remove a negative sign. In local positions down is negative and in global position altitude is positive.

petercerno commented 6 years ago

Well, use your code (without global_home[2]) in Lessons 6: Geodetic to NED Exercise, then it gives wrong altitude (at least according to the instruction of the exercise). Also, the solution:

https://view8ea55b2f.udacity-student-workspaces.com/notebooks/Geodetic%20to%20NED_solution.ipynb

uses the global_home[2] as I have described.

So now I am little bit confused. Either the Geodetic to NED Exercise and its solution are wrong, or this code is wrong.

domluna commented 6 years ago

Hmmm, I could envision a scenario where both would be correct, depending on what you want. Maybe something like

def global_to_local(global_position, global_home, preverse_altitude=False):

    (east_home, north_home, _, _) = utm.from_latlon(global_home[1], global_home[0])

    (east, north, _, _) = utm.from_latlon(global_position[1], global_position[0])

    if preserve_altitude:
        local_position = numpy.array([north - north_home, east - east_home, -(global_position[2])])
    else:
        local_position = numpy.array([north - north_home, east - east_home, -(global_position[2] - global_home[2])])

    return local_position

would work

@adrnp thoughts?

petercerno commented 6 years ago

Note that both methods need to modified (as I suggested), not only global_to_local.

Semantically, both global_position and global_home contain absolute altitudes. The current code completely ignores global_home[2] altitude, which is OK as long as this home altitude is always equal to zero (which is usually the case, as the drone usually starts from the zero altitude).

However, my proposed solution works even if the home altitude is non-zero.