-
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
[DRAFT] Include support for Ardupilot terrain servers as elevation data source #10715
[DRAFT] Include support for Ardupilot terrain servers as elevation data source #10715
Conversation
This commit adds support for setting home clicking on map. It is shown as another action in the submenu when we click over the map with an active vehicle online. As per mavlink specs, this command must use AMSL altitude, so we need to first query terrain altitude, and when received send the command. Several checks have been implemented, and in case terrain altitude is not shown at the selected location a popup will appear indicating there are no terrain data for the specified location
…aseclass: in order to make ElevationMapProvider be able to be base class of anything besides Airmap. referrer was hardcoded to the base class
… in elevation map provider: This is to be on the safe side in case we have something in the qeue for downloading, and the responses are somehow messed up if we change provider, in case we are basing the parsing of the response on terrain provider settings. Maybe it is all right and it can be done correctly or even it is already fine, but until super sure of that this is safer
We needed to decuple here in 2 scenarios: 1 - on updateForCurrentView: when _fetchElevation true, we use retrieve the elevation map provider selected from settingsManager->flightMapSettings-> elevationMapProvider() 2 - on startDownload: We need to check here if the set selected corresponds to a terrain map provider, in case the fetch terrain data box is ticked, so we don't download twice. It used to be hardcoded to airmap elevation string
…r providers: - getAltitudesForCoordinates: we get the name of the provider from settings for making the query, so the QGeoTileSpec of the query uses whatever map provider is set on settings. - _terrainDone: this is the response to what we did above. Here we get the string for the map provider from the QGeoTileSpec itself, this way we are sure we are handling it with the proper hash, corresponding to the proper elevation provider. - _getTileHash: we use here the map provider string from settings. This is what is called when getAltitudesForCoordinates is called. So this way we make sure it uses the provider indicated in settings. NOTE: there is still work to do to make all this TerrainQuery file provider agnostic, this only addresses the parts where we were hardcoding the string Airmap Elevation.
d0126e8
to
4d5e823
Compare
I just force pushed after some tree cleanup and some fixes, thank you very much @mwrightE38 for the help. In case anybody else is joining, I think the most urgent stuff to follow after the above is:
After that is figured out, I think it should go here: QGCMapTileset.cpp, _networkReplyFinished() so as long as we fetch it online we unzip it and we store it unzipped, so we no longer need to worry about it.
https://github.com/ArduPilot/MissionPlanner/blob/master/ExtLibs/Utilities/srtm.cs Thanks! |
We used to have in QGCMapTileSet::_networkReplyFinished() this serializeFromAirMapJson hardcoded to the airmap elevation string. Instead we added functions in urlFactory and map providers so we can understand from the map provider hash if such map provider needs its tiles to be serialized, and if so, we call the same provider to perform such serialization. This way the base MapProvider class returns by default that no serialization is needed, and the method to serialize just returns the same QByteArray ( we should never use this, it is just a sanity check ) Then in Airmap elevation map provider we override this, and we implement our own serialization method, which calls the TerrainTile method that was originally called from QGCMapTileSet. This way it is also more obvious when developing support for new elevation map providers, as the relevant methods are contained within the elevation map providers definition files
4d5e823
to
4e8e42b
Compare
…ude srtm1 ardupilot terrain servers
4e8e42b
to
5f4cb31
Compare
We implement again a method on QGCMapUrlEngine, mapProvider and overriden in AP ElevationMapProvider to check if tiles received need to be unzipped, and also a method to unzip them
Small update, I pushed some commits, latest is the work in progress. It was implemented a mechanism to check if map providers need to uncompress tiles, and I am in the step of testing uncompressing. It seems the cache system is getting the tiles and storing them correctly, and the calls to AP elevation server seems to be done correctly. The QByteArray method doesn't work for uncompressing, and even if it does it is not good to use it, it could break functionality at any moment, we need to use a proper zip library there. Zlib is already implemented in QGC, but not for all platforms. We need to investigate that... |
Thanks to @DonLakeFlyer for the tips about uncompressing in QGC, thanks to them I realized we have zlib available for all platforms in QGC. I gave it a try but unfortunately zlib expects the files in zlib format, which apparently is different from .gzip or .zip. I wasn't aware of this, I am not very familiar with compression in general. Next time I spare time for this I will investigate more possibilities for this decompresion... |
This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there: |
any updates on this? |
@cessna1525 yes, it seems Auterion is jumping in to prepare a server that "emulates" how Airmap server behaved, so that will be a more straightforward solution. Once that is done we will continue the work done here. As Auterion jumped in, and there wasn't much interest anyway we slowed down on this a lot, but we will eventually get back to it. Thanks for the interest |
@Davidsastresas This can be closed now right? |
@DonLakeFlyer yes I think so. At some point I would like to work on this again, but I don't know when I will get back to it so we can close it for now. Thanks. |
@Davidsastresas is there still value in this? I can take a look at it since I have been working on the location code. |
I don't see it. But maybe someone has a better reason as to why multiple terrain server support would be important. |
I really think it is! We are now using Auterion servers, and I don't think we have a lot of guarantee that they will commit to maintain them forever. So for me it is definitely a go. I wanted to jump back to this at some point, so it would be amazing to have your help here. Thank you! |
At which point it would make certainly sense to do this. Time would be better spent helping to finish up the ancient pull which allows the import of custom geotiff elevation data. Lot's of folks ask for that. That would be a great new addition to QGC that adds a lot of value. |
I agree. However most of the work done on this PR was towards supporting several providers, it wasn't hardcoding it to AP terrain servers. So I would say priority one would be to de-couple the current Auterion servers implementation to allow several providers, and then work on those geotiffs, and maybe then add AP terrain servers. |
Sorry, I wasn't paying attention and thought this was just an Issue discussion as opposed to an existing Pull. If it's mostly done then it can't hurt. |
I thought it would be good to start making public the current work done over a separate branch, so we can start testing builds when it is functional.
current state of it:
De coupling of Airmap in elevation architecture is mostly done, and database management should be fine for this new provider ( not tested yet ).
latest commit is a work in progress, it doesn't build with it
If the above indeed is good, we would need to sort out:
Thanks @mwrightE38 for joining!