Unable to change locations form settings config

cloneofghosts commented 4 weeks ago

Describe the bug

When I was testing the new update I was looking for a location which had precipitation for the current day. I found a location to test with (Victoria, BC) and after confirming that the API returned data for the current day liquid I went to change my location using the settings config.

After I entered in the new lat/long and the data refreshed it still showed the data for my current location instead of the location I entered. In order to test with the new location I had to setup the integration again in order to test to see if it worked.

Expected behavior

I should be able to change locations from the settings config without having to setup the integration.

Actual behavior

I have to setup the integration again in order for the new location to be used.

Home Assistant version

2024.10.4

Integration version

1.6.1

Other details

No response

Troubleshooting steps

[X] I have updated my Home Assistant installation to the latest version.
[X] I have updated the Pirate Weather Integration to the latest version.
[X] I have gone through the documentation before opening this issue.
[X] I have searched this repository and API Repository to see if the issue has already been reported.
[X] I have restarted my Home Assistant installation.
[X] I have queried the API in my browser to confirm the issue is not with the API.
[X] I have not included my API key anywhere in my issue.
[X] I have written an informative title.

alexander0042 commented 4 weeks ago

So to be honest, I'd forgotten that this was even possible, since my initial instinct was that it wasn't; however, it's clearly in there as an option, so should either work or be removed. Can you see if the logs are showing requests for the new location? And does restarting cause it to flip to the new location?

cloneofghosts commented 4 weeks ago

I didn't try restarting after I changed the location but I can definitely try that. I thought the integration doesn't output the location you queried with the API anymore as a result of #246.

I can add it in to my devcontainer for debugging purposes if it's helpful.

cloneofghosts commented 3 weeks ago

This was what the location I had it set to for testing: (Vancouver)

This is what I changed it to: (Calgary)

This is what was shown in the logs after I changed the location:

2024-11-03 20:18:44.692 INFO (MainThread) [custom_components.pirateweather] Unloading Pirate Weather
2024-11-03 20:18:44.937 DEBUG (MainThread) [custom_components.pirateweather.weather_update_coordinator] Pirate Weather data update
2024-11-03 20:18:44.939 DEBUG (MainThread) [custom_components.pirateweather.weather_update_coordinator] Pirate Weather data update for 48.433, -123.362
2024-11-03 20:18:44.939 DEBUG (MainThread) [custom_components.pirateweather.weather_update_coordinator] Finished fetching pirateweather data in 0.245 seconds (success: True)
2024-11-03 20:18:44.941 INFO (MainThread) [homeassistant.components.sensor] Setting up pirateweather.sensor
2024-11-03 20:18:44.958 INFO (MainThread) [homeassistant.components.weather] Setting up pirateweather.weather

Even after restarting the devcontainer it is still pulling data from the previous location and not the new one which I just configured.

cloneofghosts commented 2 weeks ago

@alexander0042 Any ideas on this one? I could have sworn that this used to work at some point but no longer does for whatever reason.

nbJosh commented 2 weeks ago

I think this is the issue I have been having at very least, the setup process should not recommend using 0,0 for default location.

cloneofghosts commented 2 weeks ago

I think this is the issue I have been having at very least, the setup process should not recommend using 0,0 for default location.

When setting up the integration the integration uses the latitude and longitude that you configured in your Home Assistant install. So I'm guessing the location of your HA install is set to 0,0 which is why it was pre-loaded in? If it's setup correctly then I'd recommend creating a new issue in this repo and then we can troubleshoot from there.

alexander0042 commented 1 week ago

To answer the two questions here, the integration should be pulling the data from the default install, so that's the source of the 0,0.

The moving location issue is a good one though, and definitely something I want to make work! It shouldn't be too tricky, but I need to go through and see what HA does when those variables change

cloneofghosts commented 1 week ago

Was able to figure out the issue. The issue was with these two lines:

https://github.com/Pirate-Weather/pirate-weather-ha/blob/9b2d4d9456930b6020841c48efb131350d983673/custom_components/pirateweather/__init__.py#L47-L48

They always read data from the entry data instead of reading the configured value. Changing it to read from the configured value allows you to change locations from the settings config. Will merge in my update and release a beta which I will watch and see if anything pops up.

While testing this I somehow queried a location which was ~580C, a RH of 233% and pressure was over 3000 hPa but not sure what location it queried as I didn't have the debug log on at that point.

Pirate-Weather / pirate-weather-ha