willasm / obsidian-open-weather

Obsidian plugin for OpenWeather API
MIT License
47 stars 3 forks source link

Metric wind speeds are commonly displayed in m/s #16

Closed mwngrn closed 7 months ago

mwngrn commented 11 months ago

When talking about wind speeds using the metric system (at least in Sweden) you use m/s, not km/h as is currently used.

Looking at the code it seems that this is a conscious decision since the wind speed (and gust) is multiplied with 3.6, effectively converting it from m/s to km/h.

What's the reasoning behind this? Just a misunderstanding and we could/should change it to m/s? Or add a setting/additional string macro that you can use if you want m/s?

willasm commented 11 months ago

In Canada it is measured in km/h so that is what I went with. It should not be a problem to make that optional though.

I actually have not been able to work on this for a while, was without a computer for a few months after my ten year old pc died on me but I finally have a new pc setup now. Being retired and on a tight budget, it took me a while to replace it. There a a couple of things I want to change and I will add this to the list. Might take a while as I will need to refresh myself with where I was at with the code again. Hoping I can get everything done this week though.

mwngrn commented 11 months ago

Ah, ok the world is a big and diverse place 😀

My suggestion then would be to add a macro string, for instance "%wind-speed-ms%" that way no existing users need to change anything and no new settings are needed I guess.

If you want to, I could give it a go and see if I can fix it. Been interested in looking into Obsidian plugins so it could be fun.

willasm commented 11 months ago

This is an excellent idea and will not affect existing users setups. I have VSCode setup and working now and will see if I can get this added tonight. Feel free to play around with the code, it is heavily commented by the way. That is how I learned to write this plugin.

willasm commented 11 months ago

That was easier than I thought. If you want to test, download the main.js file and replace the one in this extensions folder (optionally backup original if you want). Use the macro %wind-speed-ms% to display wind speed in m/s. I still have more work to do before I can upload an update for Obsidian but you can use this fixed version until then.

One thing to note is the API returns the wind speed with 2 decimal places Eg. 3.09 m/s - Would you prefer to have this rounded down Eg. 3 m/s or keep the more precise speed?

If you need help downloading it let me know. Github can be a pain to download single files.

willasm commented 11 months ago

The fix has been uploaded as v1.6.0 and should be available in Obsidian shortly. No need to download the fixed version now. I left the wind speed at 2 decimal places for now. Please let me know if you would prefer this rounded down or leave it as is.

mwngrn commented 11 months ago

Awesome, that was very quick :) downloaded the update and confirmed that it works 👍

Regarding the rounding I think it's probably best to be consistent with the how the km/h is and I think that one is rounded because I haven't seen any decimals for it in my generated Obsidian notes. I also guess that since this is weather forecasts they're aren't that accurate anyway ;)

willasm commented 11 months ago

Thanks for the great feedback, very much appreciated. I will round them on the next update, who really needs that kind of accuracy for something that is constantly changing anyways, lol. I want to update the documentation as well which always takes me longer but expect another update in a day or so.

mwngrn commented 11 months ago

Thank you for developing this awesome plugin 🙏 and just let me know if there's something I can help out with.

willasm commented 7 months ago

Somehow I forgot to add the rounding for this in the last updates (Sorry). This is now corrected in a soon to be released update.