WIP: Update to Tower to support Alt Frames like Terrain #1826
Conversation
- This allow support for using different frames - Absolute (GLOBAL_ABS) which relates to AMSL - Relative (GLOBAL_RELATIVE) which means relative to home location - Terrain (GLOBAL_TERRAIN) which means AGL (via Range Finder or STRM Data) - Tested with “round-robbin” download/upload with APM Planner 2.0 Frame setting is preserved from original setting.
|
|
||
| @Override | ||
| public void onItemSelected(AdapterView<?> parent, View view, int position, long id) { | ||
| frameSpinner = (Spinner) view.findViewById(R.id.frameSpinner); |
There was a problem hiding this comment.
@billbonney You should be able to remove this assignment since frameSpinner is already set in setupFrameSpinner().
| public void onItemSelected(AdapterView<?> parent, View view, int position, long id) { | ||
| frameSpinner = (Spinner) view.findViewById(R.id.frameSpinner); | ||
|
|
||
| if (frameSpinner != null && mSelectedProxies.isEmpty()) |
There was a problem hiding this comment.
@billbonney This has the effect that if frameSpinner is null the early return will be aborted. Is that the intended effect?
|
|
||
| boolean updated = false; | ||
| switch (position) { | ||
| case 0: |
There was a problem hiding this comment.
@billbonney Here and below, use java constants for the case labels.
|
|
||
| MissionItem currentItem = missionItemProxy.getMissionItem(); | ||
|
|
||
| List<MissionItemProxy> updatedItems = new ArrayList<>(); |
There was a problem hiding this comment.
@billbonney Looks like there's at most one entry in the updatedItems list for every loop, hence the list doesn't seem needed.
| } | ||
| if (!updatedList.isEmpty()) { | ||
| mListener.onWaypointTypeChanged(selectedType, updatedList); | ||
| dismiss(); // TODO:!BB! do we want to dismiss the dialog |
There was a problem hiding this comment.
@billbonney I don't think so. It should auto dismiss after the user's selection.
| // List<String> strList = new ArrayList<>("@arrays/") // TODO !BB! Get List from array string | ||
| switch (frame) { | ||
| case GLOBAL_ABS: | ||
| frameSpinner.setSelection(0); |
There was a problem hiding this comment.
@billbonney Use java constants as arguments for the setSelection(...) method.
| @@ -183,7 +182,8 @@ public void onClick(View v) { | |||
| leftDrawable = R.drawable.ic_mission_wp; | |||
| } | |||
|
|
|||
| altitudeView.setCompoundDrawablesWithIntrinsicBounds(leftDrawable, 0, 0, 0); | |||
| String frameType = ""; | |||
There was a problem hiding this comment.
@billbonney No default value for frameType?
| Waypoint waypoint = new Waypoint(); | ||
| waypoint.setCoordinate(new LatLongAlt(point.getLatitude(), point.getLongitude(), | ||
| (float) alt)); | ||
| waypoint.getCoordinate().set(point); |
There was a problem hiding this comment.
@billbonney Here and below, Waypoint#getCoordinate() is not guaranteed to be non-null, so this changes will result in several npe errors.
NOTE: this is a WIP, so comments welcome. Not ready to merge yet.
This is simple changes to support the new Frame in LatLongAlt object. Future changes are to support showing waypoint frame in UI.
Tested so that current setup will not modify the frame if the mission is downloaded from APM Planner 2.0 which allows the frame to be set.
see dependent dronekit-android PR here dronekit/dronekit-android#460