-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
feature/webapi-mapfeatures #826
feature/webapi-mapfeatures #826
Conversation
- Add new route to API contract with default values (must be removed after development) - register new MapActionsController at ActionsControllerIndex
- PoC example only - Proove rendering retrievable via api
- Replace default values by "example" values from api contract - Compatible with initial https://github.com/KOKAProduktion/littlenavmap-openlayers/tree/feature/lnm-webapi
- Make copyright note painting setable via MapPaintWidget::setPaintCopyright
- Enable small distance Rect requests (closer zoom) - albar965#784 (comment)
- Implement airports restrieval by rect - Implement /map/features route - Fetches feature results from in-app map cache (wrong) - Extend info builders - Complement API contract Core changes: - Expose MapPaintWidgets MapPaintLayer via getMapPaintLayer() - Extend MapQuery by getAirportsByRect(...)
- Insert build output of KOKAProduktion/littlenavmap-openlayers#1 at KOKAProduktion/littlenavmap-openlayers@94fa623
…le-tiled-output - Resolve conflicts - Apply albar965#790 (comment)
- Apply albar965#790 (comment) - Perform dummy image action and extract results - Expose MapPaintLayer::updateLayers() to allow mapLayer initialisation
- Extend API contract
- Add NDB, VOR, and Marker data - Extend MapQuery by atools Rect query methods - Extend info builder - NDB,VOR,Marker results appear not to match airport retrieval rect
- setShowMapFeatures -> setShowMapObject - setShowMapFeaturesDisplay -> setShowMapObjectDisplay
- setShowMapFeatures -> setShowMapObject - setShowMapFeaturesDisplay -> setShowMapObjectDisplay (cherry picked from commit b3330c5)
// Fill result object | ||
mappixmap.pixmap = mapPaintWidget->getPixmap(width, height); |
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.
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.
Would the merge the problem? Or reverting a change?
And there is still the issue with getIls()
.
I can simply release it the next days. It's an alpha/development version. People can expect issues. 🙂
Alex
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.
Nah, i don't think merging would solve the issue. At least I haven't run into it yet so i didn't fix anything in this PR. So maybe we shouldn't merge before we solve it, trying to avoid adding issues :)
Not retrieving ILS yet on this PR, I'm only that far for now:
const QList<map::MapAirport> airports;
const QList<map::MapNdb> ndbs;
const QList<map::MapVor> vors;
const QList<map::MapMarker> markers;
const QList<map::MapWaypoint> waypoints;
Cheers
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.
Nah, i don't think merging would solve the issue. At least I haven't run into it yet so i didn't fix anything in this PR. So maybe we shouldn't merge before we solve it, trying to avoid adding issues :)
Not retrieving ILS yet on this PR, I'm only that far for now:
Happens during paint in the marble widget.
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'll investigate
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.
Ok. Take your time.
Hi @KOKAProduktion , I looked through, partly skimmed, https://github.com/albar965/littlenavmap/pull/826/files : looks good** ! ** I don't remember the context of everything and didn't "verify" every logic by thinking them through but nothing triggered any "knowledge" of mine about an improvement to / how to improve your additions and changes. To summarise your code and the api contract you currently added returning features such as airport data for a given "LatLonRect" (expression not looked up as being used in the code technically), usable for example to add layers of such data in the future upon a OL tile. I just saw I'll see whether I can report about that too. |
Hi @u-an-i, thanks for your review! It's still a draft since it's missing some features. I'm on it. While developing I've realised that creating layers with interact-able objects is not required in the first step as long as they are still painted on the map image. That's why I diverted to just query objects close to the users click. This also reduces the load on the browser. I'm aiming to finish a replacement for the current mapping solution in the web UI soon, providing all the currently available features. I'm just missing the wind indicator/speed at the top atm... In my implementation The OL map object is available to the web UI's JS and fires subscribe-able events passing requested API data to any consumer. (Aircraft performance, click result, etc). This way we'll be able to reuse the API call results the map requires for its functioning in the web UI JS. Also controlling the map (e.g. by your auto-zoom plugin) is possible through the exposed object. In a second step or when we decide to move painting from LNM/Marble to OL querying for all visible objects in the viewport becomes required. Please give me some more time (2 weeks maybe) to finish this PR for completeness... I don't want to waste your time reviewing unfinished stuff :) Thanks again and cheers! |
Thank you very much for your detailed reply! I understand and agree. I very surely give you more time, as much as you need and even don't need :) |
I would like the functionality of the current web ui (button bar, themes, plugins, framed content eg map) and the legacy ui be present in an ol version. For this you can either replace the map.html called in the new web ui by an map_ol_html or create a v3 an keep the "new web ui" and the legavy web ui as 2 legacy uis. If you go with 2 lgecys uis you are free to do whatever you want - as everyone always is supposed to be - but if you go with replacing the map in the new ui please read the plugin doc and the "planning" doc or how i called it which covers the conformance any update should have. I'm looking intriguedly forward! |
Yep. I'm aiming for that too. Ideally I'd like to let the user toggle the mapping solution only. I'm just afraid that this may increase maintenance on the web ui side (keeping up functionality on both solutions). But I don't assume that everything will be perfectly satisfying on the OL solution up front, so I guess we have to go this way anyway at least in the beginning.
Could you please point me to these docs? Would you like me to update your plugins or would you like to take care of that yourself? Thx and cheers! |
I assume similarly. planning: https://github.com/albar965/littlenavmap/blob/master/web/README_documentation.txt plugins doc: https://github.com/albar965/littlenavmap/blob/master/web/plugins/example-plugin_documentation/index.html plugins API: https://github.com/albar965/littlenavmap/blob/master/web/assets/2021-a/plugins.js toolbar API: https://github.com/albar965/littlenavmap/blob/master/web/assets/2021-a/toolbarAPI.js If you like, update my code as you like. |
- Reinit content iframe manipulation by ol-map logic when returning from other panels to map.html
Alright guys! Finally here it is :) This is the OL map solution inside the LNM web UI, implemented following @u-an-i 's plugin pattern. There are some considerations regarding dynamic objects display and the clickability is already available but not yet connected to anything. Just reflecting the current feature set of the single image mapping solution, from my point of view. Also the plugin active state seems not to be persisted over sessions but since the autozoom plugin doesn't persist too, i guess that's still in progress by @u-an-i. I haven't adapted the autozoom plugin to the OL solution, but the I'd love to contribute much more and faster, but current circumstances force me to take care of other important things. I'm afraid I won't be able to do much the upcoming weeks or even months... Sorry for that! But I'll rejoin as soon as i can and I'm still here for fixes, issues and discussions. Meanwhile I'll try to extend the documentation on this. I hope you like it and I'm very curious about your impressions and remarks! |
Can I merge? I reviewed it already some time ago. |
Yes you can! I think it's stable and not obstructing other features. |
woohoo! :)
this was a decision to not do so but I'm open to change this!
perfectly good!
I'm so too! :) And I'm so for another current project of mine. I guess i look a the web code quickly right away but carefully only later! |
quickly looked: oh, this is concise while spaciously presented: nice! |
Thanks a lot! 🎆 🏅 |
Thank YOU guys! Especially for all the support on the way! It was a lot of fun and I hope I'll be able to return soon, as there's so much more to do :) Cheers! |
Hi @albar965, hi @u-an-i,
this PR includes and therefore replaces #788 .
It provides
This is just to abandon the #788 branch on my side. I don't see the need in maintaining it separately anymore. No review nor merge required at this point.
Cheers!