-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
flightplan_nav_fixes #690
flightplan_nav_fixes #690
Conversation
@@ -284,9 +284,11 @@ bool_t nav_approaching_from(uint8_t wp_idx, uint8_t from_idx) { | |||
//printf("dist %d | %d %d\n", dist_to_point,diff.x,diff.y); | |||
//fflush(stdout); | |||
//if (dist_to_point < (ARRIVED_AT_WAYPOINT >> INT32_POS_FRAC)) return TRUE; | |||
if (dist_to_point < (ARRIVED_AT_WAYPOINT >> INT32_POS_FRAC)) time_at_wp++; | |||
if (dist_to_point < (ARRIVED_AT_WAYPOINT >> INT32_POS_FRAC)){ | |||
RunOnceEvery(16, time_at_wp++); |
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.
16Hz for 'nav_approaching_from' function , is it correct?
I think it would be easier to only call nav_periodic_task if in nav mode instead of checking the mode in that function again. Btw, moved the ground_alt computation in master already, so it is only recalculated when the reference actually changed. |
The whole navigation routines are a bit crude at points (mostly initially done to have something similar to fixedwing). The semantics of NavApproaching(target, from, t) in fixedwing firmware are different to rotorcraft.
In any case It might be nicer to actually get the time you are at the waypoint instead of continuing to try to get the frequency a function is called at right and then counting up... |
Calling the nav_periodic_task even if not in NAV mode allows to have the carrot correctly displayed in GCS in other modes. I'd like to keep this, since it is very useful while flight testing and switching between nav and other modes. At least you have an idea of what is suppose to happen. Not sure why the time_at_wp was really needed (looks like I did that but I don't remember why), but the nav_approaching should work the same way it's done in FW nav. |
True, that makes sense... added a comment... |
Felix im out of town now and cannot check the code ın detail. First of all CARROT time is generated by flight plan generator on default if approaching time is not presented and there is a compile error if CARROT is not defined in airframe .xml. |
Right, previously the time was just ignored and hence CARROT not needed... but it would make more sense to put it in navigation.h instead of autopilot.h. Really not sure about only calling nav_periodic_task and flightplan when in nav mode... |
} | ||
} | ||
else wp_reached = FALSE; | ||
|
||
if (from_idx > 0 && from_idx < NB_WAYPOINT) { |
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.
i propose to remove this part of code. Does it calculate an inner product or something else (i can't understand the math behind it) ? But it seems to me, it will return TRUE even if the rotorcraft is not on the desired wp when it cross over the normal line to the path(from_wp to next_wp). Flight plan may switch to the next wp, when there is a deviation from the path line (e.g. disturbance like wind) if the rotorcraft cross over this normal line? If this part will be removed , i propose to change the name of the function 'nav_approachong_from' since the aim will not be approaching from a waypoint, but it will be holding on a waypoint for a desired time. (e.g. 'nav_wp_successed')
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.
You got it correctly, it test if a segment is completed if you cross the normal line to the path at the final point. This is because you don't want to go back to your point while doing a segment or a path, even if you you miss the final point for a few meters. This is especially true for fixedwings, but I think it also applies to rotorcraft. The test is only done if the initial waypoint ID is valid (>0 and <NB_WAYPOINT).
BTW, the commit related with calling 'nav_periodic_task' is removed, you are right about the issue.. |
The time_at_waypoint check (e.g. how much to spend at a waypoint before completing it) is really old stuff and I think it is pretty useless now. If you want to spend some time at a given waypoint you should do it with a flight plan command (like in line_p1_p2 in rotorcraft_basic.xml).
|
Hi Gautier, but in this way isn't there a risk to never reach a waypoint? For example, rotorcraft crosses the normal line without reaching wp then flightplan jumps to next stage(e.g. stay wp2 until stage_time> ..), and the rotorcraft cannot accomplish to reach the wp in specified stage_time. |
If it really matters for a specific flight plan to stay for n seconds at the WP (and not only in the stage), I guess it is possible to keep the time_at_waypoint variable (and make it extern) so you can use it in the until condition: the rotorcraft will have to reach exactly the WP before the timer incrementing the time. |
} | ||
} | ||
else { | ||
time_at_wp = 0; |
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.
indentation please
Look like you will now always only wait 1second at the waypoint, why then bother to record the time at the waypoint at all? |
Hi Felix, My idea was to calculate the time at wp and make it reachable from flight plan to use in exceptions or loops etc with "<stay wp.. " commands . But i missed the point that "stay" commands is not calling the approaching function logically. Do you support to implement a discrete function to calculate time_at_wp which will be called from flight plan? Proximity test(as Gautier mentioned for the fixed wing -> testing ground_speed*time_to_WP ) is not implemented yet for the rotorcrafts. Should we implement this? |
sys_time.nb_sec is in directly dependent on sys_tick_handler. i guess autopilot_flight_time only counts up if in_flight, are you sure about using sys_time.nb_sec? |
Approaching function returns TRUE; NavCheckWaypointTime function added to call from FP, e.g; |
stay "wp1" until= "NavCheckWaypointTime(..)" |
} | ||
else { | ||
time_at_wp = 0; | ||
wp_reached = FALSE; |
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.
If you have a strong gust that makes you leave the proximity circle, the counter restart. Is it what you really want ?
Because if you're really unlucky this may last for a long time...
Hi Gautier, |
I looks good like this, but we should implement the approaching_time correctly in the future anyway |
Fixes on pulling auto_nav function in every ap mode and 'approaching time'