search

grafana-toolbox / panodata-map-panel

Map Panel for Grafana with improved convenience, robustness and features. Friendly fork of the original Grafana Worldmap Panel. Currently not maintained, but verified to work up to Grafana 9.
https://community.panodata.org/t/grafana-map-panel/121
GNU Affero General Public License v3.0
88 stars 31 forks source link

Add image overlay option #84

Closed jreyesr closed 3 years ago

jreyesr commented 3 years ago

(This is my first contribution to the project, so my apologies if I have overlooked anything obvious)

This PR adds support for an optional "image overlay" to be shown over the basemap and below the data. It uses the ImageOverlay function of Leaflet, see here for an example.

I needed this functionality to show interpolated data from a wireless sensor network, but it could also be used, for example, to show a more updated map (perhaps from a drone flight?) over the (potentially old or low-res) basemap.

I am still writing the tests. I would welcome guidance as to potential improvements, bugs or suggested changes. Documentation is also missing (I imagine that it would be necessary to update the README to explain the new config options).

amotl commented 3 years ago

Dear @jreyesr,

thanks a bunch, we appreciate your contribution. The patch looks excellent, I will just address a minor nit.

I am still writing the tests.

Having tests which cover the new feature indeed would be nice!

Documentation is also missing.

Adding some words about the new feature would also be nice. Thanks for your prudence.

With kind regards, Andreas.

P.S.: Please note that I've also just merged #81, so you might want to rebase on top of current master.

jreyesr commented 3 years ago

I've added some tests, an explanation of the settings in the README, and fixed the variable names as requested.

I'm still unsure of how to handle an image added to the README. The URL is currently relative to the README, since https://raw.githubusercontent.com/panodata/grafana-map-panel/master/src/images/overlay_example.png doesn't exist yet, but that will probably cause problems if the README is used on other webpages, such as the Grafana plugin page. Should I include an invalid URL anyway, and wait until the merge so that it becomes valid?

amotl commented 3 years ago

I've added some tests, an explanation of the settings in the README, and fixed the variable names as requested.

Thank you so much!

I'm still unsure of how to handle an image added to the README. Should I include an invalid URL anyway, and wait until the merge so that it becomes valid?

Exactly.

So, I believe this would be ready to merge? Thanks again, your contribution went along pretty awesome.

jreyesr commented 3 years ago

Done. The link should now work, once the PR gets merged.

So, I believe this would be ready to merge?

Yes, as far as I can tell. I've just marked the PR as ready for review.

Thanks again

Of course! Happy to help.

amotl commented 3 years ago

Thanks a bunch. We just released grafana-map-panel 0.14.0 which includes your improvement.

falconhome commented 3 years ago

Hi Guys! This overlay function is very useful, thanks a lot! I think my request is a "feature". It is possible to add "Reload image on dasboard refresh" checkbox under the Custom image overlay settings?

Now the overlay image load when the dashboard load, and not refreshing when the grafana base dashboard refresh occure. (Only the dataset updated.) In my case the overlay image updating dynamically (under a same name) but i can reload it only with user interruption (CTRL + F5).

It is possible to add somehow this feature?

Thank you so much!

jreyesr commented 3 years ago

@falconhome Hi! It's nice to know that someone else apart from me used the overlays :)

Just to check: as I understand it, your issue/enhancement is that, say, http://somepage.com/imgs/overlay host an image, which is updated periodically (but the URL stays the same), and you want to fetch the most recent version of the image every time the dashboard is manually or automatically refreshed (via the Refresh button or the timer), since it currently fetches the image one time, on page load, and the only way to refresh it is Ctrl+F5 or closing+reopening the tab.

falconhome commented 3 years ago

Yes, you understand correctly! I tried to figure out, how can be add this function, but i failed on "yarn dev" + i did not find where the refresh should be implemented in a correct way. Thanks a lot! ps.: and the reason why i proposed the config settings too, because in many application they use static images and refreshing only the datas. In this case the periodically reload of the image is not required + it put extra load to the renderer...

jreyesr commented 3 years ago

OK. I think that it is possible to implement the described feature.

This PR was closed some months ago, and I don't think that it should handle more commits. Could you open an issue describing your feature request? That would get the official project maintainers involved (I am not a maintainer, by the way), and check if such an enhancement would be welcome or on scope for the project (I expect it would be, but it's probably a good idea to check with the maintainers first). You can mention that I am willing to implement the feature, since I developed the original overlay code, so there would be very little extra work for the maintainers.

After the issue is opened, we can take any further discussion there and leave this old PR to rest.

falconhome commented 3 years ago

Hi @jreyesr I opened a ticket/feature request as you mentioned! ;) https://github.com/panodata/grafana-map-panel/issues/97 We can continue the conversation under that thread. Thanks a lot!