OpenPilot

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Thanks for doing those changes http://reviews.openpilot.org/static/n81cat/2static/images/wiki/icons/emoticons/smile.gif I think this should be safe to merge to next as soon as the current release ...

Thanks for doing those changes

I think this should be safe to merge to next as soon as the current release is out. Once its merged I'd start work to make the FixedWingPathfollower compatible (step 1) and possibly unify/refactor the pathfollowers into one combined module (step 2) unless someone else wants to have a go at that.

I think - based on the code - even as is it should be safe to fly with both pathfollowers for anyone flying from next, and merging it asap creates less conflicts later

Missing argument for path_endpoint added.

Thanks http://reviews.openpilot.org/static/n81cat/2static/images/wiki/icons/emoticons/smile.gif I'll make the changes based on Eric's comments so it's good if we have some additional testing after ...

Thanks I'll make the changes based on Eric's comments so it's good if we have some additional testing after this. I'll also try to do some flights but at least will do HiTL tests.

Don't limit the path to strictly be between start and endpoint.

I really like it, great job Werner! http://reviews.openpilot.org/static/n81cat/2static/images/wiki/icons/emoticons/wink.gif I'm hoping to be able to test it this weekend

I really like it, great job Werner!

I'm hoping to be able to test it this weekend

Revert to the original behaviour of calling "fly_endpoint" if we have a zero-length path.

Extending the path to infinity was my original concern and the reason why I did the bound. Just as an extra protection against flying away. Normaly there's always a next waypoint (or we switch to f...

Extending the path to infinity was my original concern and the reason why I did the bound. Just as an extra protection against flying away. Normaly there's always a next waypoint (or we switch to fly_endpoint) so this shouldn't be an issue. I'll drop the bound..

I did some more thinking on this. path_direction would remain the same even after the overshoot - correction_direction would - in extremest case - point 180 degrees to it. what the pathfollower wo...

I did some more thinking on this. path_direction would remain the same even after the overshoot - correction_direction would - in extremest case - point 180 degrees to it.

what the pathfollower would actually do out of that would depend on the weighting of correction versus path components, which depends on pathfollower as well as specific pathfollower setttings. most likely the pathfollower would eventually - after overshooting for some distance - reach a point where correction_direction * correctionKp * status->error is greater than <whateverspeedhasbeensetup> * path_direction - at which point it would no longer proceed in path_direction but hover - or in case of fixed wing loiter - at an arbitrary position in space somewhere behind path_end – that is if the non-orthogonality of path_direction and correction_direction doesn't cause further navigation errors.

treating the path as a vector of infinite length, where path_progress indicates the position relative to start and endpoint (not bound to 0<<x<<1) crates a more uniform vector field and makes the behaviour much more predictable

yes, thats actually an issue of the vtolpathfollower, it still uses two separate velocity calculations for endpoint and vector, which was the reason for quite a number of issues I ran into when imp...

yes, thats actually an issue of the vtolpathfollower, it still uses two separate velocity calculations for endpoint and vector, which was the reason for quite a number of issues I ran into when implementing PositionVario. That's one of the many things to address when unifying and cleaning up the pathfollowers, as the pathfollowers should behave mostly the same, in all modes.

fixedwingpathfollower does use path_direction regardless of mode, and in the future - when unifying them vtol will do that, too.

We don't want zero-length path_direction vector.

At least the VTOLPathFollower doesn't use path_direction at all in mode fly_endpoint so any vector will do. 0,0,1 makes the most sense to me..

At least the VTOLPathFollower doesn't use path_direction at all in mode fly_endpoint so any vector will do. 0,0,1 makes the most sense to me..

Maybe it's best to revert to the original behaviour of calling fly_endpoint here. I'll have a look and change this.

Maybe it's best to revert to the original behaviour of calling fly_endpoint here. I'll have a look and change this.

that can be a problem - with fractional_progress limited to 0<<x<<1, the track_point is also limited to that interval, leading to path_direction and correction_direction to become non-orthogonal if...

that can be a problem - with fractional_progress limited to 0<<x<<1, the track_point is also limited to that interval, leading to path_direction and correction_direction to become non-orthogonal if fractional_progress<0 (start waypoint not yet reached) or fractional_progress>1 (overshoot)

at best this would turn any endpoint automatically into a "position hold point" - which is drastically different semantics than the old behaviour.

in practice, most waypoints with pathaction type FlyVector would use legremaining condition with a positive end value between 0 and 1, so it would jump to the next waypoint before this condition is reached.

if another endcondition is used however, the behaviour would differ - possibly drastically

I would like to enforce the rule that correction_direction must be orthogonal to path_direction or zero. I also see no reason to really bound fractional_progress between 0,1 - it might be wiser to indicate possible overshoot to the caller.

I'm pretty sure the fixed wing code needs a nonzero path_direction to work. It also requires path_direction and correction_direction to be orthogonal for some of its math to determine whether to tu...

I'm pretty sure the fixed wing code needs a nonzero path_direction to work. It also requires path_direction and correction_direction to be orthogonal for some of its math to determine whether to turn left or right when heading the wrong way.

yeah. I guess it would be better if we enforced path_direction to always have nonzero values, so there's always a direction to go to. It's probably save to give it a 0,0,1 or 0,0,-1 direction.

yeah. I guess it would be better if we enforced path_direction to always have nonzero values, so there's always a direction to go to. It's probably save to give it a 0,0,1 or 0,0,-1 direction.

sure, np.

sure, np.

If we are very picky and see path_direction as a unit vector it should always have a magnitude of 1. But in the end, it doesn't make a difference here. I'm not totally happy with the current implem...

If we are very picky and see path_direction as a unit vector it should always have a magnitude of 1. But in the end, it doesn't make a difference here. I'm not totally happy with the current implementation that uses a normalized direction vector plus a scalar for magnitude. This makes things a bit more complicate to handle compared to simply using the vector directly to express both, direction & magnitude. But I didn't want to make too many changes here so I kept the original scheme.

correction_direction is set later in the code. Please notice that I removed the "return" statement from the else branch that prevented this in the original code. The idea here is that the path itse...

correction_direction is set later in the code. Please notice that I removed the "return" statement from the else branch that prevented this in the original code. The idea here is that the path itself shrinks to a spot if start- and endpoint are too close to each other and that correction_direction takes care of the neccessary corrections to stay close to that spot. I assumed that for a fixed wing this would automatically lead to circleing that spot but I don't habe explicitly tested this for the fixed wing case.

Ah, yes that's true... the original plan was to remove this fields from the pathstatus before publishing the code because I mainly used them for debugging but on the other hand it's quite nice to h...

Ah, yes that's true... the original plan was to remove this fields from the pathstatus before publishing the code because I mainly used them for debugging but on the other hand it's quite nice to have this information when tuning the path followers so I kept them.

I already looked at the FixedWingPathFollower code but couldn't identify parts that looked like they need any changes. I don't have any experience with the FixedWing code so it would be nice if you...

I already looked at the FixedWingPathFollower code but couldn't identify parts that looked like they need any changes. I don't have any experience with the FixedWing code so it would be nice if you could take care of this, Seems like I overlooked something...

here a 0,0,0 is ok, as its based on dist_diff – any movment in any direction would make dist_diff nonzero and as such prevent a flyaway (as long as pathfollower prevents div by zero which it does) ...

here a 0,0,0 is ok, as its based on dist_diff – any movment in any direction would make dist_diff nonzero and as such prevent a flyaway (as long as pathfollower prevents div by zero which it does)

in the flyvector calculation below its differemt, path_direction only depends on the set of waypoints, and it needs to point back to the end waypoint somehow. the apropriate fallback therefore is to fallback to fly_endpoint of only one point is given as in the old code.

zero length path is not good, fixed wing pathfollower cannot hover on a spot so it needs this "direction to endpoint" fallback to come back after overshoot. otherwise it might flyaway in random dir...

zero length path is not good, fixed wing pathfollower cannot hover on a spot so it needs this "direction to endpoint" fallback to come back after overshoot. otherwise it might flyaway in random direction which is not good. forcing a "positionhold" would be saver

also correction direction unset!

ps: also fixedwingpathfollower should likely set the new fields in pathstatus - obviously http://reviews.openpilot.org/static/n81cat/2static/images/wiki/icons/emoticons/wink.gif

ps: also fixedwingpathfollower should likely set the new fields in pathstatus - obviously

I like this a lot and it looks very well implemented. Kudos on doing both flight and hitl tests http://reviews.openpilot.org/static/n81cat/2static/images/wiki/icons/emoticons/smile.gif I had a qui...

I like this a lot and it looks very well implemented. Kudos on doing both flight and hitl tests

I had a quick look at the fixedwingpathfollower and judging by how its implemented I'd say it definitely needs to be adapted to correctly match this new behaviour (it also currently bases its vertical behaviour on horizontal progress - sortof)

the calculations in FixedWingPathFollower::updatePathVelocity() should be relatively straightforward to convert to 3d. I don't think that altering the stuff in correctCourse() and updateFixedDesiredAttitude() needs any changing, as long as VelocityDesired gets set correctly.

would you like to adapt the fixed wing pathfollower as well, or do you want me to look into it after this is merged?

OP-1415 Repeated_names_in_credits : Updated with Wiki contributors
OP-1415 Repeated_names_in_credits : Updated with Wiki contributors
OP-1415 Repeated_names_in_credits : Updated with Wiki contributors

Improved 3D PathFollower behaviour
Improved 3D PathFollower behaviour
Merge branch 'theothercliff/OP-1421_Cruise_Control_xml_defaultvalue_is_incorrect' into next