feat: replace Airmap terrain data with Copernicus

[leonardosimovic]

The Problem

The Airmap terrain servers (and possibly other Airmap services) are shutting down. The docs https://developers.airmap.com/docs/elevation-api are already down while the servers seem still to be running for the time being.

The Solution

This PR replaces the Airmap query with an alternative provider, hosting the Copernicus GLO-30 which was compiled and provided by © Airbus Defence and Space GmbH dataset. The Copernicus dataset has generally better global coverage and accuracy than the previous SRTM dataset.

Followup or Incremental Work

• Remove Airmap references in the code.
• Give credit to © Airbus Defence and Space GmbH also in the UI.

Elevation Provider Credit

Added a small note in the bottom center of terrain plot:

Comparison

Latitude zone I: 0-50 degrees north and south

Airmap

Copernicus

Latitude zone II: 50-70 degrees north and south:

Airmap

Copernicus

Latitude zone III: 70-75 degrees north and south:

Airmap
Note: the SRTM dataset only covers land surface between 60° north and 56° south latitude

Copernicus

Latitude zone IV: 75-80 degrees north and south:

Airmap
Note: the SRTM dataset only covers land surface between 60° north and 56° south latitude

Copernicus

Latitude zone V: 80-90 degrees north and south:

Airmap
Note: the SRTM dataset only covers land surface between 60° north and 56° south latitude

Copernicus

Raw Images and Mission Plans

qgc-airmap-replacement.zip

[leonardosimovic]

Dropped the bi-linear interpolation since it doesn't necessarily yield better results than NN. More advanced interpolation algorithms based on statistical methods, e.g. Kriging or Natural Neighbor, would be ideal.
Sources:

• https://www.sciencedirect.com/science/article/pii/S1110982313000276
• http://www.geocomputation.org/1999/082/gc_082.htm

[Davidsastresas]

Thanks @leonardosimovic

I took a quick look, and regarding the implementation I would only change all the references to Airmap to whatever we want to call this provider. Not 100% sure right now, but doing this also we can probably avoid bumping cache version right?

Regarding the bi-linear approximation or not, what is the gridSize of your tiles right now? The graphs on your comparisons look great, so probably we are good, but I think it would be good to specify somewhere the gridSize of your tiles so we can understand the error we are assuming not doing interpolation, or doing it.

I tested real quick your branch and it works great for me, but I am getting some errors when I create a polygon and then I move it. When creating a survey polygon everything is smooth, I only get the errors when click and drag vertices:

TerrainQueryLog: TerrainTileManager::getAltitudesForCoordinates Internal Error: missing elevation in tile cache
TerrainQueryLog: addPathQuery: signalling failure due to internal error
TerrainTileLog: 0x5580d4118738 Internal error: coordinate outside tile bounds: QGeoCoordinate(40.479999573, -3.3279448085)

I don't think I will be able to spend much more time this week on this matter, so if you could investigate the above it would be great.

Thank you very much!

[leonardosimovic]

Hey @Davidsastresas

Thank you for the feedback!

Not 100% sure right now, but doing this also we can probably avoid bumping cache version right

I would also prefer not upgrading the cache. However, it is necessary because the new data has a different format and therefore is not compatible. Previously, with Airmap, it was assumed that each carpet query would return a fixed array size n_lat x n_lon, regardless of the geolocation, and this has worked well. Our service accounts for converging longitudinal lines as we go to higher latitudes, hence the array size will vary in longitudinal direction.

Regarding the bi-linear approximation or not, what is the gridSize of your tiles right now?

It has the same resolution as Airmap i.e. 30 meters, hence I did not change the static variables or the line sampling method.

I tested real quick your branch and it works great for me, but I am getting some errors when I create a polygon and then I move it. When creating a survey polygon everything is smooth, I only get the errors when click and drag vertices:

I will have a look!
Edit: yup, can reproduce 👍🏼

[Davidsastresas]

Great to hear you can also reproduce those warnings!

Regarding Cache, I am not 100% sure of this, but isn't the cache system based on the hash for each tile, and isn't this hash dependant on the name? So, changing the name of the provider, shouldn't the tiles have a different hash and thus not having need to bump cache version? I am not 100% sure though.

Thanks!

[leonardosimovic]

Hey @Davidsastresas

I looked into the warnings some more and traced it back to rounding errors in the bounds fields for the request:
https://terrain-ce.suite.auterion.com/api/v1/carpet?points=40.42,-3.23,40.43,-3.22.
We will soon deploy an updated service, I will include a fix for this issue.

Regarding Cache, I am not 100% sure of this, but isn't the cache system based on the hash for each tile, and isn't this hash dependant on the name? So, changing the name of the provider, shouldn't the tiles have a different hash and thus not having need to bump cache version? I am not 100% sure though.

This sounds right, but I'm not 100% sure either. When I tested it the cache needed to be bumped otherwise still the perviously cached tiles were loaded 🤔

Edit
Of course! The provider key is still the same i.e. Airmap Elevation. This I wanted to address in an incremental PR anyway (removing all the airmap elevation references). However it makes sense to get it in all at once so we can avoid bumping the cache version.

[leonardosimovic]

@Davidsastresas

I pushed a couple of commits, addressing the Airmap hard-coding + some unrelated performance improvements. I tried to keep the changes to a minimum in order to reduce conflicts with your PR. Let me know what you think!

Also, I will let you know as soon as we have deployed our updated service.

[Davidsastresas]

Thank you, sounds great!

I won't have time to take a look at it until next week I think. Thanks!

[DonLakeFlyer]

Thanks so, so much for this work. I took a quick look through and it looks fine. But at this point @Davidsastresas knows way more about this than I do since he was recently in this code.

My one complaint is in the license string being part of the terrain status display. That takes up really (really) critical vertical real estate on small screens, like small tablets or say a Herelink. Can that go someplace else? Couple options:

Where the QGC version number is:

The help screen in Application settings:

[leonardosimovic]

Hey @DonLakeFlyer! No problem, we're happy to help :)

My one complaint is in the license string being part of the terrain status display. That takes up really (really) critical vertical real estate on small screens, like small tablets or say a Herelink. Can that go someplace else?

That's a valid point! My only concern is with the visibility of the copyright notice if we place it in the menus. Let me check with the team and I'll come back to you.

[DonLakeFlyer]

Is there some legal thing with respect to where it needs to be?

[DonLakeFlyer]

The other options is show it in terrain status for say a few seconds and then hide it.

[Davidsastresas]

Hi!, I think I will be able to check this tomorrow. Thanks for the work!

About the label, I agree with Don, it is really not good there, in devices like Herelink screen real state is precious. The idea about showing it for a few seconds I think is a good compromise. Something like 5 seconds will really be noticed. On the draft PR we were making for supporting several elevation providers we will have a dedicated option in general settings for it. It could be another nice place, in a way that the copyright only shows if we select this provider.

Also, It would be good to somehow link that license to the elevationmap provider, instead of being in QGCUrlmapengine as a global variable. Just thinking on the near future when we support more elevation providers. We can manage that later as well, I don't think that should hold back the PR.

Thanks!

[Davidsastresas]

I just spared some time to check this out. It looks good, but that error when creating a polygon and moving it around is still there:

addPathQuery: signalling failure due to internal error
TerrainTileLog: 0x559173b25078 Internal error: coordinate QGeoCoordinate(-33.380000755, 122.5906058) outside tile bounds
TerrainQueryLog: TerrainTileManager::getAltitudesForCoordinates Internal Error: missing elevation in tile cache

Some notes:

• I think we could squash the first and second commits, as the second only fixes some stuff over TerrainTile, in order to keep the git tree more compact.

• Regarding the 3rd commit, when you declare that kOnlineElevationProviderKey, I think we could do something there that leaves it nicer for future support for more elevation map providers. It could be added as a setting, so the name of the map provider is available globally by QGroundControl settings. Please see this commit to see how it was done on the work we were doing:

Davidsastresas@6b1817c

and then it could be accessed like this:

Davidsastresas@ed2dff4

• For the copyright notice, besides where to place it, I think it would be good to move it to elevationmapprovider files, so that string can be retrieved from a mapprovider method. I think that way it could be nicer, more normalized. I really don't like to use global variables like that.

In any case, I understand you probably don't have a lot of time left to spend into this, and we are very grateful that you managed to spend the time in the first place. So maybe we can push this to a separate branch, not master, and rework it from there, if you can not put the time to fix the above. But it would be good to sort out that error before considering this.

Of course this is only my opinion, and if the rest of developers disagree and we decide to take a different approach I am totally happy with it too.

Thanks!

[mrpollo]

Hey @leonardosimovic thanks for your work we are excited to have this feature in. Is there a way we can get this finalized soon we would like to promote a stable release to deprecate Airmap and bring in a few other fixes

[leonardosimovic]

Hey everyone, sorry for the delay!

The other options is show it in terrain status for say a few seconds and then hide it.

I'm not sure about hiding the label after x seconds from a UX point of view. I don't think the label is super noticeable as it is now, but it's disappearing will cause the plot to resize / elements to shift, this will be noticed for sure by which time the label is already gone.

Other suggestion would be to move outside and on top of the plot. This won't increase terrain plot background size while the label remains visible.

On the draft PR we were making for supporting several elevation providers we will have a dedicated option in general settings for it. It could be another nice place, in a way that the copyright only shows if we select this provider.

This would be a very nice solution!

I just spared some time to check this out. It looks good, but that error when creating a polygon and moving it around is still there

Yeah we haven't yet deployed the fix to the terrain-service. I will let you know as soon as it's in.

Regarding the 3rd commit, when you declare that kOnlineElevationProviderKey, I think we could do something there that leaves it nicer for future support for more elevation map providers. It could be added as a setting, so the name of the map provider is available globally by QGroundControl settings.

That's definitely the way to go and you already implemented in your draft I believe. Hence I went with the bare minimum so it's a) less cumbersome to rebase for you and b) we can safe time by avoid duplicate work.

[DonLakeFlyer]

Other suggestion would be to move outside and on top of the plot.

I like that much better. If it's just text you can still see the map behind it and make sure it in the zorder below the plan items on the map.

[leonardosimovic]

Hey everyone, I cleaned up the commit history and moved the label ontop of the terrain plot:

It's not the prettiest solution but I hope it's acceptable for the time being, until there's a dedicated terrain settings section which @Davidsastresas proposed.

[DonLakeFlyer]

It's not the prettiest solution but I hope it's acceptable for the time being

I think it's good enough for now. @Davidsastresas Want to give one final look see an then you can merge.

[Davidsastresas]

@DonLakeFlyer as said on my previous comment, if the rest of you are fine with this I am fine myself too, so I am happy with this getting merged as well!

Thanks!

[leonardosimovic]

Awesome, thank you all for the reviews!

@Davidsastresas we have redeployed our terrain service, so you shouldn't see the error any more after clearing the map cache 😄