-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Vehicle: add support for altitude above terrain as fact for telemetry panel #10786
Conversation
509a421
to
be95ad0
Compare
Very useful feature. Thanks! |
About the 500 ms timer, do you think it might be better to use a boolean variable that can be set to false when sending a request and to true in the handler slot. In this case, we can not send new requests until the current one is processed? |
Also regarding naming new facts, maybe altitudeAGL would be preferable? It is more consistent with avionics. |
Thanks for taking a look at it! About the boolean, it would not be as simple because if for some reason the request is not serviced we would have a deadlock. A "similar"idea is already implemented in the terrainquery implementation itself, so It would have been totally feasible and safe to not implement timer at all, terrainquery mechanism takes care of it. I just tested a bit and I realized the terrain query system wasn't able to provide updates at a higher rate than that, so we just save the minimal extra cpu cycles it would take. About the fact name, I totally agree, will fix it this ASAP. Thanks! |
Two things:
|
be95ad0
to
91523d9
Compare
@DonLakeFlyer I agree with your suggestions. I rebased to master and addressed your comments. I needed to check for altitude change as well, because checking only distance would not query new terrain altitude in the situation when a copter gains altitude in the same coordinate. I left a minimum change of .5 meters in altitude and 2 meters in XY for a new query to be executed. I left the commits separated for you to check the changes, but If you confirm it to be fine I will squash them before merging. thanks! |
Looks good. Squash it and you can merge it. |
91523d9
to
aaddeb6
Compare
Thanks, done! |
Add support to have altitude above terrain as displayable telemetry. It queries terrain data when vehicle coordinate is changed to compute the altitude, but it has an elapsed timer to limit the time between those queries to 500ms minimum, because the terrain query system wasn't able to service them at higher rate, and 500ms is fine anyway for this information. This elapsed timer is not super elegant there, if anybody has a nicer idea for it I will be happy to work on it.
Last commit is unrelated, but I realized that function wasn't in the private slot declaration, and it belongs there. Same for first commit.
Thanks to https://araceuas.com for sponsoring this.