Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
LUA example Glide into wind #27413
base: master
Are you sure you want to change the base?
LUA example Glide into wind #27413
Changes from 4 commits
9acbdd9
4dc13d8
d82602d
c9c9d2e
bc157e1
e39d223
212c782
f604749
dab8d17
d5b673b
ff13cc5
18e0003
54c7bdb
6e964ab
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is unused, and your instructions wouldn't make this parameter visible to the user.
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.
Thank you! In an earlier version of the script I used it.. I will evaluate if it should be used or removed. Update to come later tonight.
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.
Adding times is usually a pattern considered harmful as things go really wonky when the numbers wrap.
It's not a problem here, but a pattern we do try to avoid.
I can't see why you haven't calculated
link_lost_for
where needed by subtracting gcs:last_seen() from the current time. That does mean finding a time in the same "frame" as gcs:last_seen(), but I think you'll find the current time routines will return that.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 agree that adding up time like this builds bigger and bigger errors and in general is not a good solution.
The reason is that I cannot properly find the time, it baffles me but I'm not the only one. There is GNSS dependent solution to extract time from the GNSS signals, but does not make sense either.
The highest resolution of os.time() is seconds.
For this use, I think it is ok. As a general solution for checking how much time has elapsed it is not a good practice.
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 haven't explained the reason for this anywhere, in the PR or (preferably) in a comment at the top of the script.
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 are right, I shall explain that and put it in the top comment of the script. I will commit an updated version later today.
Except that I don't like RC-override in general, there are 2-3 reasons:
There is a problem in the current implementation:
If the RC pilot wants to cancel the ongoing glide into wind before the script says 'no more steering' and disables the flag override_enable, he/she flips flight mode and gets full control, ok. Later when the RC-pilot returns to FBWA, and the GCS link has still not recovered, the script again will try to steer into wind.
I will present an update version later today where this is adressed.
edit: typo
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.
So now I'm curious.
An alternate formulation of this script would have all of the same detection logic for entering the glide-into-wind failsafe logic, but would avoid using RC overrides to achieve the desired heading.
Instead, it would change the vehicle into GUIDED mode and use some mavlink guided-mode binding to achieve the desired heading (
vehicle:update_target_location
if nothing else)This would have several advantages
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.
Seems a little strange; you could just do the comparison against the
mode_XXX
value instead of converting to a string and then comparing...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.
Completely agree! Will update tonight