[module][refactor] replace traffic_info subsystem with module #1638

kirkscheper · 2016-04-25T11:58:57Z

Splitting #1629. This pull request converts traffic info from subsystem to module and updates the functions that use traffic_info.

Is there any documentation that I should add for this?

flixr · 2016-04-25T12:57:20Z

A detailed description in the module would be great!
The conversion to module really makes sense and is appreciated, although it's a bit more than that ;-)

There are some more things to fix:

fix waypoint coordinates for fixedwing multi/tcas modules which was broken with [rotorcraft] add traffic_info to rotorcraft #1509 (these modules still expect relative UTM coordinates)

flixr · 2016-04-25T13:15:59Z

Also we should be really sure we actually want to use UTM for traffic_info instead of the more common LLA...
@gautierhattenberger @dewagter what do you guys think?

kirkscheper · 2016-04-25T14:46:24Z

Ah yes, I see that now as well. I will wait till the final decision is made on the utm. I am for lla. Then we don't have to worry about zone extending when in a different zone. I am also fo lla in communications as you have one term less (zone).

gautierhattenberger · 2016-04-25T16:37:38Z

I'm fine with storing the state of other aircraft in lla, but then we also need to provide convenient way to use it with several options:

direct comparison of two lla coordinates
convert first in an other frame each time traffic info are used (like "multi" modules)
make a reduced version of the state interface to access these information in any frame
probably more options

The painful to implement option 3 is probably the best since you only compute what you need, and even handle local or global coordinates.

kirkscheper · 2016-05-02T09:28:39Z

Thanks for the feedback. Sounds like I will try to use state as an example for the new and improved traffic. I will also take a look at the multi modules to upgrade them to use true global coordinates. I think with this I can also implement a follow nav block for rotorcraft.

I am working on a paper now so would prefer not to change it right now so I will get back to this in about 2 weeks.

kirkscheper · 2016-06-10T15:12:57Z

after a found break I have found some time to update the traffic. I still have to update the modules with the new protocol but here is a initial preview of what the new traffic info module looks like. I hope this is more in the right direction.

gautierhattenberger · 2016-06-14T09:28:44Z

@kirkscheper it seems you're not pointing to the correct pprzlink version

kirkscheper · 2016-06-14T10:09:21Z

Yea, I know, I am waiting for it to be pulled to master. Currently pointing
to my own branch for testing. This isn't ready for merging yet.

Any comments on the changes? I added an acinfo_lla message, alternatively I
can just replace the utm message. Check changes to server.ml.

I am testing it all right now actually, as soon as I get the thumbs up from
you guys I will clean up the PR and pprlink stuff.
On 14 Jun 2016 11:28 a.m., "Gautier Hattenberger" notifications@github.com
wrote:

@kirkscheper https://github.com/kirkscheper it seems you're not
pointing to the correct pprzlink version

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1638 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ACtEQ1KRRiwd3tm14z0898Eldeqkn4ofks5qLnRPgaJpZM4IO3x3
.

gautierhattenberger · 2016-06-14T11:26:40Z

The overall changes are looking fine. You can probably already make the request for ACINFO_LLA in pprzlink.

flixr · 2016-06-14T12:53:43Z

sw/ground_segment/tmtc/server.ml

@@ -381,8 +380,8 @@ let send_aircraft_msg = fun ac ->
        else
          let deg7_of_rad = fun f -> PprzLink.Int32 (Int32.of_float (Geometry_2d.rad2deg (f *. 1e7))) in
          let ac_info_lla = ["ac_id", PprzLink.String ac;
-                             "lat", deg7_of_rad a.pos.posn_lat;
-                             "lon", deg7_of_rad a.pos.posn_long;
+                             "lla_lat", deg7_of_rad a.pos.posn_lat;


I would keep lat and lon like we have it in the other messages...

kirkscheper · 2016-06-15T13:41:44Z

All cleaned up and ready to go. This does need paparazzi/pprzlink#14 to work fully (weird that all the tests were passed...).

flixr · 2016-06-15T13:43:39Z

Only the configurations in conf_tests.xml are run on every commit. Testing of all confs is only run once per day and not shown in the PR checks...

kirkscheper · 2016-06-15T13:51:30Z

ah, cool. Where can I find the output of the test_all_confs?

flixr · 2016-06-15T13:56:31Z

They are sent to paparazzi-builds@nongnu.org and listed on semaphore: https://semaphoreci.com/paparazziuav/paparazzi (the ones that say "Scheduler rebuilt x" are the ones which compiled all confs)

See also https://wiki.paparazziuav.org/wiki/Builds

flixr · 2016-06-15T14:42:05Z

sw/airborne/modules/multi/traffic_info.h

+/** Get vehicle course (float).
+ * @param[in] aircraft id of aircraft info to get
+ */
+static inline float *acInfoGetCourse(uint8_t ac_id)


I think it would be nicer/better if all get functions returning a scalar actually return a scalar instead of a pointer (like in the state interface).

flixr · 2016-06-15T14:44:24Z

For the future: it makes reviewing much easier if you make separate commits for logical changes and especially separate commits for only formatting changes...

flixr · 2016-06-15T14:47:00Z

sw/airborne/modules/multi/traffic_info.c

+  ti_acs_idx = 2;
+}
+
+int parse_acinfo_dl(void)


seems like it would make more sense to return a bool here?

flixr · 2016-06-15T15:14:20Z

I like the new traffic info, looks well done/structured and nicely commented 👍

Some general higher level questions and remarks:

TCAS should also be usable for rotorcrafts now, right?
Did you test it with a mix of fixedwings and rotorcrafts?
If I see that correctly TCAS, etc. is now done in ENU...
Is this "real" ENU or the "fake" ENU that is used in the state interface in the fixedwing case for "local coordinates in UTM"?

@gautierhattenberger regarding the last point: at a glance it looks like this could be problematic (since the ENU position from traffic info is compared to the output of stateGetPositionEnu_f. Could you please also take a closer look at this? If there is still an issue with how the state interface handles this we need to open an issue for it...

flixr · 2016-06-15T15:47:41Z

sw/airborne/modules/multi/traffic_info.h

+
+/************************ Get functions ****************************/
+
+extern void acInfoCalcPositionUtm_i(uint8_t ac_id);


how do all these functions handle the case where the info for the requested ac_id is not present?

For the set functions, I totally forgot about that. I will update that.

For the get functions, from the current use cases, the position of the other aircraft is used to get a delta which is used to do something so I could return the position of the ownship when the requested ac_id isn't initialised. Velocity is usually used to extrapolate the other ac position with the itow difference so there I would just return zeros. With the current implementation you are likely to get really strange values if the ac isn't initialised yet. Any other ideas?

Now I write this it may also be interesting to have helper functions that just return the relative position of the other vehicle (so do the subtraction for you) cause it is almost always the goal. I was really hoping to be done with this :(

kirkscheper · 2016-06-16T09:42:33Z

Regarding the TCAS stuff, it looks like it only uses relative positions so the ENU vs UTM shouldn't be an issue. For making it work with rotorcraft that needs a little work as it currently uses v_ctl_altitude_setpoint to set the desired alt. I will leave that for another PR.

flixr · 2016-06-16T10:57:30Z

Regarding the TCAS stuff, it looks like it only uses relative positions so the ENU vs UTM shouldn't be an issue.

Unfortunately that is not true. ENU and UTM coordinate systems don't align perfectly if you are not in the center of a UTM zone. In UTM north always points north and in ENU north is only exactly north at the origin. So if you are further away from the origin the axis don't align...

kirkscheper · 2016-06-16T11:04:52Z

yes yes, this problem I know very well, I have already mentioned before that the conversions in state.c are not correct. you first have to go to lla then to enu. The last time I mentioned this someone mentioned that it was done so as to not have the computation penalty on limited platforms and that the issue would be fixed when the navigation firmware is unified. I really really don't like the hack in state...

kirkscheper · 2016-06-16T11:48:35Z

Oh gosh, I also forgot about ellipsoid to hsml conversions in the frame calculations face palm.

kirkscheper · 2016-06-16T15:54:39Z

I think I got most of the comments. ps. if you didn't like what I did in the UTM PR you probably won't like what I did here either. I have a geoid_height variable to convert from ellipsoid to hsml and vise-versa. My plan was that when more accurate methods are developed they could be included in the function which sets the geoid_height variable (traffic_info.c).

flixr · 2016-06-16T17:51:18Z

sw/airborne/modules/multi/tcas.c

    }
    // Bound alt
-    tcas_alt_setpoint = Max(GROUND_ALT + SECURITY_HEIGHT, tcas_alt_setpoint);


wonder if we shouldn't rather use ground_alt + SECURITY_HEIGHT so it takes the geo init changes into account...

gautierhattenberger · 2016-06-16T20:49:11Z

I'm not really sure that the current conversions are really matching. Here, when traffic info UTM position is stored in UTM format, and when ENU is requested, it is computed from UTM->LLA->ENU, which means that it is not compatible for fixed-wing ENU (which are still "local utm / fake enu"). Actually the conversion is not even done since LLA->ENU needs a ned_origin, while only utm_origin is provided in the case of FW.
So if we want to make it work with less efforts (but still some prayers), UTM should be used as it was before, so that when ACINFO is received and used, it is already in the correct format and if it is ACINFO_LLA, LLA->UTM conversion is correct.
If we want to use ENU/NED, we either solve the issue in state (which is not so obvious) or we do the same trick in traffic_info: test utm_origin and ned_origin to decide how the conversion should be done.

…ded interface

gautierhattenberger · 2016-06-19T14:08:02Z

It looks okay now, I think it is almost ready to merge.

flixr · 2016-06-19T15:57:58Z

It's still not clear to me in which cases this would now work and in which cases it would be wrong.
Can we please at least try to tease out the remaining cases that are not yet working correctly (and why) before we merge it?
Then we can open appropriate issues for them so it's at least documented and we can more easily track the remaining points and fix them bit by bit...

gautierhattenberger · 2016-06-19T20:27:45Z

It will take same time to analyze and test all of them, but in general, since external positions are received in global coordinates, and that transformation to local ones is done the same (eventually bad) way than state interface and based on the same reference, it should already work in most cases (if not all).

flixr · 2016-06-19T20:32:31Z

OK...

kirkscheper · 2016-06-20T19:10:59Z

With the most recent commit, this is what I think.
When it should work and does:
Rotorcraft firmware with a gps with LLA messages

When it should work but not tested:
Fixedwing firmware using state interface with UTM = ENU + UTM_ORIGIN
Fixedwing firmware with other vehicles in a different utm zone.

When it doesn't work:
No airframes currently use traffic_info so it might not be tested and updated, (I hope to push a conf with this shortly, maybe even an example!).
Strange situations with uninitialised reference frames...

Is this almost done now? What's missing?

gautierhattenberger · 2016-06-20T21:11:52Z

For points when it doesn't work:

apparently no example airframes are using "multiple aircraft" functionalities (tcas, formation flight,...) so nothing should be really broken
this is also true for state interface anyway

I think we are ready to merge. @flixr any objections ?

flixr mentioned this pull request Apr 26, 2016

[modules] Swarming #1630

Closed

flixr reviewed Jun 14, 2016
View reviewed changes

kirkscheper force-pushed the traffic_refactor branch from f13ca52 to 631242f Compare June 15, 2016 09:21

flixr reviewed Jun 15, 2016
View reviewed changes

kirkscheper force-pushed the traffic_refactor branch from 7bb08a3 to c125a05 Compare June 16, 2016 14:42

flixr reviewed Jun 16, 2016
View reviewed changes

kirkscheper force-pushed the traffic_refactor branch from c125a05 to 8c5be3f Compare June 17, 2016 06:26

kirkscheper added 3 commits June 17, 2016 16:17

[refactor] Converted traffic_info from subsystem to module with upgra…

4f8fd9c

…ded interface

change enu calc when utm_origin used

5bb3a72

replace SECURITY_ALT with more robust safety height

55cbeeb

kirkscheper force-pushed the traffic_refactor branch from 5bcfd50 to 55cbeeb Compare June 17, 2016 14:17

kirkscheper added 2 commits June 20, 2016 17:12

fix unused variable warning

866138b

[traffic info] add support for zone extension

e00a7d7

flixr merged commit f0e5df6 into paparazzi:master Jun 20, 2016

kirkscheper mentioned this pull request Jun 20, 2016

[Feature request] Inter-vehicle position tracking for rotorcraft #1463

Closed

kirkscheper deleted the traffic_refactor branch July 11, 2016 09:33

flixr mentioned this pull request Sep 20, 2016

convert subsystems to modules #1475

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[module][refactor] replace traffic_info subsystem with module #1638

[module][refactor] replace traffic_info subsystem with module #1638

kirkscheper commented Apr 25, 2016

flixr commented Apr 25, 2016

flixr commented Apr 25, 2016

kirkscheper commented Apr 25, 2016

gautierhattenberger commented Apr 25, 2016

kirkscheper commented May 2, 2016

kirkscheper commented Jun 10, 2016

gautierhattenberger commented Jun 14, 2016

kirkscheper commented Jun 14, 2016

gautierhattenberger commented Jun 14, 2016

flixr Jun 14, 2016

kirkscheper commented Jun 15, 2016

flixr commented Jun 15, 2016

kirkscheper commented Jun 15, 2016

flixr commented Jun 15, 2016

flixr Jun 15, 2016

flixr commented Jun 15, 2016

flixr Jun 15, 2016

flixr commented Jun 15, 2016

flixr Jun 15, 2016

kirkscheper Jun 16, 2016

kirkscheper commented Jun 16, 2016

flixr commented Jun 16, 2016

kirkscheper commented Jun 16, 2016

kirkscheper commented Jun 16, 2016

kirkscheper commented Jun 16, 2016

flixr Jun 16, 2016

gautierhattenberger commented Jun 16, 2016

gautierhattenberger commented Jun 19, 2016

flixr commented Jun 19, 2016

gautierhattenberger commented Jun 19, 2016

flixr commented Jun 19, 2016

kirkscheper commented Jun 20, 2016

gautierhattenberger commented Jun 20, 2016


		/********************** Get functions **************************/

		extern void acInfoCalcPositionUtm_i(uint8_t ac_id);

[module][refactor] replace traffic_info subsystem with module #1638

[module][refactor] replace traffic_info subsystem with module #1638

Conversation

kirkscheper commented Apr 25, 2016

flixr commented Apr 25, 2016

flixr commented Apr 25, 2016

kirkscheper commented Apr 25, 2016

gautierhattenberger commented Apr 25, 2016

kirkscheper commented May 2, 2016

kirkscheper commented Jun 10, 2016

gautierhattenberger commented Jun 14, 2016

kirkscheper commented Jun 14, 2016

gautierhattenberger commented Jun 14, 2016

flixr Jun 14, 2016

Choose a reason for hiding this comment

kirkscheper commented Jun 15, 2016

flixr commented Jun 15, 2016

kirkscheper commented Jun 15, 2016

flixr commented Jun 15, 2016

flixr Jun 15, 2016

Choose a reason for hiding this comment

flixr commented Jun 15, 2016

flixr Jun 15, 2016

Choose a reason for hiding this comment

flixr commented Jun 15, 2016

flixr Jun 15, 2016

Choose a reason for hiding this comment

kirkscheper Jun 16, 2016

Choose a reason for hiding this comment

kirkscheper commented Jun 16, 2016

flixr commented Jun 16, 2016

kirkscheper commented Jun 16, 2016

kirkscheper commented Jun 16, 2016

kirkscheper commented Jun 16, 2016

flixr Jun 16, 2016

Choose a reason for hiding this comment

gautierhattenberger commented Jun 16, 2016

gautierhattenberger commented Jun 19, 2016

flixr commented Jun 19, 2016

gautierhattenberger commented Jun 19, 2016

flixr commented Jun 19, 2016

kirkscheper commented Jun 20, 2016

gautierhattenberger commented Jun 20, 2016