WIP: Adding Alt Frames (ie. Terrain) to API #460
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package com.o3dr.services.android.lib.coordinate; | ||
|
|
||
| import com.MAVLink.enums.MAV_FRAME; | ||
|
|
||
| /** TODO This is the start of using a more generic location/frame class | ||
| * The frame for LatLongAlt would only ever be GLOBAL and not LOCAL. | ||
| */ | ||
|
|
||
| public enum Frame { | ||
| GLOBAL_ABS ("Absolute" , "Abs", 0 ), // Absolute means Above Mean Sea Level AMSL | ||
| LOCAL_NED ("Local NED", "NED", 1 ), | ||
| MISSION ("Mission" , "MIS", 2 ), | ||
| GLOBAL_RELATIVE ("Relative" , "Rel", 3 ), // Relative to HOME location | ||
| LOCAL_ENU ("Local ENU", "ENU", 4 ), | ||
| GLOBAL_TERRAIN ("Terrain" , "Ter", 10); // Relative to Terrain Level. (Either measured or from STRM) | ||
|
|
||
| private final int frame; | ||
| private final String name; | ||
| private final String abbreviation; | ||
|
|
||
| Frame(String name, String abbreviation, int frame ) { | ||
| this.name = name; | ||
| this.abbreviation = abbreviation; | ||
| this.frame = frame; | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
||
| public String getAbbreviation() { | ||
| return abbreviation; | ||
| } | ||
|
|
||
| public int asInt() { | ||
| return frame; | ||
| } | ||
|
|
||
| public static Frame getFrame(int mavFrame) { | ||
|
|
||
| switch (mavFrame) { | ||
| case MAV_FRAME.MAV_FRAME_GLOBAL: | ||
| case MAV_FRAME.MAV_FRAME_GLOBAL_INT: | ||
| return Frame.GLOBAL_ABS; | ||
|
|
||
| case MAV_FRAME.MAV_FRAME_MISSION: | ||
| return Frame.MISSION; | ||
|
|
||
| case MAV_FRAME.MAV_FRAME_LOCAL_NED: | ||
| return Frame.LOCAL_NED; | ||
|
|
||
| case MAV_FRAME.MAV_FRAME_LOCAL_ENU: | ||
| return Frame.LOCAL_NED; | ||
|
|
||
| case MAV_FRAME.MAV_FRAME_GLOBAL_TERRAIN_ALT: | ||
| case MAV_FRAME.MAV_FRAME_GLOBAL_TERRAIN_ALT_INT: | ||
| return Frame.GLOBAL_TERRAIN; | ||
|
|
||
| case MAV_FRAME.MAV_FRAME_GLOBAL_RELATIVE_ALT: | ||
| case MAV_FRAME.MAV_FRAME_GLOBAL_RELATIVE_ALT_INT: | ||
| default: | ||
| return Frame.GLOBAL_RELATIVE; | ||
| } | ||
|
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,11 @@ public class LatLong implements Parcelable, Serializable { | |
| private double latitude; | ||
| private double longitude; | ||
|
|
||
| public LatLong(){ | ||
|
There was a problem hiding this comment. @billbonney I don't agree with having a default constructor. This is usually the cause of subtle and hard to find bugs. In which scenario would having a default constructor be helpful? |
||
| this.latitude = -100.0; // TODO: Should we set this to invalid ie -100.0 ? | ||
| this.longitude = -190.0;// TODO: Should we set this to invalid ie -190.0 ? | ||
| } | ||
|
|
||
| public LatLong(double latitude, double longitude){ | ||
| this.latitude = latitude; | ||
| this.longitude = longitude; | ||
|
|
@@ -32,6 +37,23 @@ public void set(LatLong update){ | |
| this.longitude = update.longitude; | ||
| } | ||
|
|
||
| /** | ||
| * @return if this is a valid LatLong global point | ||
| */ | ||
| public boolean isValid() { | ||
|
|
||
| if ( this.longitude > 180.0 || this.latitude > 90.0 | ||
| || this.longitude < -180.0 || this.latitude < -90.0 ) { | ||
| return false; // Not a valid location | ||
|
|
||
| } if (Double.compare(this.longitude, 0.0) == 0 && Double.compare(this.latitude, 0.0) == 0) { | ||
| return false; // Rarely in 0.0,0.0 a valid location, so reject. | ||
|
|
||
| } else { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @return the latitude in degrees | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| /** | ||
| * Stores latitude, longitude, and altitude information for a coordinate. | ||
| */ | ||
|
|
||
| public class LatLongAlt extends LatLong { | ||
|
|
||
| private static final long serialVersionUID =-4771550293045623743L; | ||
|
|
@@ -14,24 +15,34 @@ public class LatLongAlt extends LatLong { | |
| * Stores the altitude in meters. | ||
| */ | ||
| private double mAltitude; | ||
| private Frame mFrame; | ||
|
|
||
| public LatLongAlt() { | ||
|
There was a problem hiding this comment. @billbonney Same comment as with There was a problem hiding this comment. I can refactor this. It was a 'quick fix'. To resolve some npe I was having. LATER: The issue is this in BaseSpatial. And that it constructed without a point. Which means it's partially created object. And then set to call here The fix was to have getCoordinate to create a LatLngAlt when none existed. I can add the parameter list there. The real fix is to refactor BaseSpatial on construction to take a point so it's not null in the first place (but that may break getNewItem for other mission items (i'll need to think about that) There was a problem hiding this comment. it looks like the above is related with change b343b8c |
||
| super(); | ||
| mAltitude = 0.0; | ||
| mFrame = Frame.GLOBAL_RELATIVE; | ||
| } | ||
|
|
||
| public LatLongAlt(double latitude, double longitude, double altitude, Frame frame) { | ||
|
There was a problem hiding this comment. @billbonney Given that There was a problem hiding this comment. I did that for a reason in that it exposes the parts in the code that sets the frame. I.e you can see explicitly what frame is being assumed. You don't want to accidentally put abs alt into relative. It would seem like a flyaway. (LATER: Ironically, this is similar thought why default constructors are bad, it hides assumptions) |
||
| super(latitude, longitude); | ||
| mAltitude = altitude; | ||
| mFrame = frame; | ||
| } | ||
|
|
||
| public LatLongAlt(LatLong location, double altitude, Frame frame){ | ||
| super(location); | ||
| mAltitude = altitude; | ||
| mFrame = frame; | ||
| } | ||
|
|
||
| public LatLongAlt(LatLongAlt copy) { | ||
| this(copy.getLatitude(), copy.getLongitude(), copy.getAltitude(), copy.getFrame()); | ||
| } | ||
|
|
||
| public void set(LatLongAlt source){ // TODO: this looks wrong | ||
|
There was a problem hiding this comment. It's like a shallow copy, i'm not sure when set is called and why? why is using set different from |
||
| super.set(source); | ||
| this.mAltitude = source.mAltitude; | ||
| this.mFrame = source.mFrame; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -45,6 +56,18 @@ public void setAltitude(double altitude) { | |
| this.mAltitude = altitude; | ||
| } | ||
|
|
||
| public Frame getFrame() { | ||
| return mFrame; | ||
| } | ||
|
|
||
| public boolean setFrame(Frame frame) { | ||
| if (mFrame != frame) { | ||
| mFrame = frame; | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
|
|
@@ -53,7 +76,9 @@ public boolean equals(Object o) { | |
|
|
||
| LatLongAlt that = (LatLongAlt) o; | ||
|
|
||
| if ((Double.compare(that.mAltitude, mAltitude) != 0) | ||
| && (that.mFrame.asInt() != mFrame.asInt()) ) | ||
|
There was a problem hiding this comment. @billbonney Since There was a problem hiding this comment. In addition, the There was a problem hiding this comment. Ok, good tip... I'll check that. |
||
| return false; | ||
|
|
||
| return true; | ||
| } | ||
|
|
@@ -63,7 +88,7 @@ public int hashCode() { | |
| int result = super.hashCode(); | ||
| long temp; | ||
| temp = Double.doubleToLongBits(mAltitude); | ||
| result = 31 * result + (int) (temp ^ (temp >>> 32)); // TODO Check this hash is OK with frame | ||
| return result; | ||
| } | ||
|
|
||
|
|
@@ -73,6 +98,7 @@ public String toString() { | |
| return "LatLongAlt{" + | ||
| superToString + | ||
| ", mAltitude=" + mAltitude + | ||
| ", mFrame=" + mFrame.getAbbreviation() + | ||
| '}'; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -277,7 +277,7 @@ public static ConnectionParameter newSoloConnection(String ssid, @Nullable Strin | |
| eventsDispatchingPeriod); | ||
| } | ||
|
|
||
| public ConnectionParameter(@ConnectionType.Type int connectionType, Bundle paramsBundle){ | ||
|
There was a problem hiding this comment. @billbonney Why make this There was a problem hiding this comment. I was having a build issue with this being private. I haven't investigated, and had not meant for it to be checked in. I should put a TODO on it, and come back and investigate. There was a problem hiding this comment. As far as I remember class BasicTest complains about the access to this constructor while compilation: There was a problem hiding this comment. @mariansoban Thanks. I can revert this change for now and we can look at fixing the test cases |
||
| this(connectionType, paramsBundle, null); | ||
| } | ||
|
|
||
|
|
||
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.
@billbonney I'd advocate for the removal of the
framefield since it can be substitute by the enum itself.