flobz / psa_car_controller

Control psa car with connected_car v4 API.
GNU General Public License v3.0
440 stars 235 forks source link

Weather API can still not get current Temp see (#611) #887

Open libertyx82 opened 6 months ago

libertyx82 commented 6 months ago

The problem from https://github.com/flobz/psa_car_controller/issues/611 still exists, it is not getting the current temperature

2024-05-25 16:23:25,032 :: ERROR :: Unable to get temperature from openweathermap : Traceback (most recent call last): File "/usr/local/lib/python3.9/dist-packages/psa_car_controller/psacc/utils/utils.py", line 20, in get_temp temp = weather_res_json.get("current", weather_res_json.get("main"))["temp"] TypeError: 'NoneType' object is not subscriptable

jellebuitenhuis commented 6 months ago

The fix in this PR should still word:

612

libertyx82 commented 6 months ago

I have installed the newest release 3.5.1 and i am getting this error

2024-05-29 17:35:30,394 :: ERROR :: Unable to get temperature from openweathermap :
Traceback (most recent call last):
File "/usr/local/lib/python3.9/dist-packages/psa_car_controller/psacc/utils/utils.py", line 20, in get_temp
temp = weather_res_json.get("current", weather_res_json.get("main"))["temp"]
TypeError: 'NoneType' object is not subscriptable`

My utils.py

`import logging
import socket

import requests

logger = logging.getLogger(__name__)
TIMEOUT_IN_S = 10

def get_temp(latitude: str, longitude: str, api_key: str) -> float:
    try:
        if latitude and longitude and api_key:
            weather_rep = requests.get("https://api.openweathermap.org/data/2.5/onecall",
                                       params={"lat": latitude, "lon": longitude,
                                               "exclude": "minutely,hourly,daily,alerts",
                                               "appid": api_key,
                                               "units": "metric"},
                                       timeout=TIMEOUT_IN_S)
            temp = weather_rep.json()["main"]["temp"]
            logger.debug("Temperature :%fc", temp)
            return temp
    except ConnectionError:
        logger.error("Can't connect to openweathermap :", exc_info=True)
    except (KeyError, TypeError):
        logger.exception("Unable to get temperature from openweathermap :")
    return None

def is_port_in_use(ip, port):
    with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
        return s.connect_ex((ip, port)) == 0

class Singleton(type):
    _instances = {}

    def __call__(cls, *args, **kwargs):
        if cls not in cls._instances:
            cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)
        return cls._instances[cls]
jellebuitenhuis commented 6 months ago

That's because it is using the one-call-api, which is not free. The free (and correct) api would be this one: https://openweathermap.org/current

libertyx82 commented 6 months ago

I have change it from

if latitude and longitude and api_key:
            weather_rep = requests.get("https://api.openweathermap.org/data/2.5/onecall",
                                       params={"lat": latitude, "lon": longitude,
                                               "exclude": "minutely,hourly,daily,alerts",
                                               "appid": api_key,
                                               "units": "metric"},
                                       timeout=TIMEOUT_IN_S)
            temp = weather_rep.json()["main"]["temp"]
            logger.debug("Temperature :%fc", temp)
            return temp

to

if latitude and longitude and api_key:
            weather_rep = requests.get("https://api.openweathermap.org/data/2.5/weather",
                                       params={"lat": latitude, "lon": longitude,
                                               "exclude": "minutely,hourly,daily,alerts",
                                               "appid": api_key,
                                               "units": "metric"},
                                       timeout=TIMEOUT_IN_S)
            temp = weather_rep.json()["main"]["temp"]
            logger.debug("Temperature :%fc", temp)
            return temp

But i get still that error

Bushrider commented 5 months ago

Can someone explain where to put the API key? Can't figure out. Thx

Knarfgm commented 5 months ago

Can someone explain where to put the API key? Can't figure out. Thx

You need to put API key in the config.json file : at the end you variable : "weather_api". Put your api key between quotes instead of null

Knarfgm commented 5 months ago

I have change it from

if latitude and longitude and api_key:
            weather_rep = requests.get("https://api.openweathermap.org/data/2.5/onecall",
                                       params={"lat": latitude, "lon": longitude,
                                               "exclude": "minutely,hourly,daily,alerts",
                                               "appid": api_key,
                                               "units": "metric"},
                                       timeout=TIMEOUT_IN_S)
            temp = weather_rep.json()["main"]["temp"]
            logger.debug("Temperature :%fc", temp)
            return temp

to

if latitude and longitude and api_key:
            weather_rep = requests.get("https://api.openweathermap.org/data/2.5/weather",
                                       params={"lat": latitude, "lon": longitude,
                                               "exclude": "minutely,hourly,daily,alerts",
                                               "appid": api_key,
                                               "units": "metric"},
                                       timeout=TIMEOUT_IN_S)
            temp = weather_rep.json()["main"]["temp"]
            logger.debug("Temperature :%fc", temp)
            return temp

But i get still that error

I replaced 2.5 by 3.0 and it is running fine for me. But you have to subscribe to the OneCall option (it is free till 1000 call per day). I can also see temp variable is a little bit different in the last release weather_res_json = weather_rep.json() temp = weather_res_json.get("current", weather_res_json.get("main"))["temp"]

I assume replacing 'onecall' by 'weather' should run but you have to remove the param "exclude": "minutely,hourly,daily,alerts", because this is not recognized by this api call. I did not tested this

Bushrider commented 5 months ago

Can someone explain where to put the API key? Can't figure out. Thx

You need to put API key in the config.json file : at the end you variable : "weather_api". Put your api key between quotes instead of null

Thank you! Do I need to reboot the container?

Knarfgm commented 5 months ago

I think so to take the config data into account

grahamrjuk commented 5 months ago

I agree this would work for me if there was a release (I am using Home Assistant) using the 3.0 version of the onecall api. Replacing 2.5 in the URL with 3.0 should fix this with the current basic free subscription.

edsub commented 1 month ago

I agree this would work for me if there was a release (I am using Home Assistant) using the 3.0 version of the onecall api. Replacing 2.5 in the URL with 3.0 should fix this with the current basic free subscription.

I can confirm this solution works with the basic free subscriptionj as per weathermap documentation (https://openweathermap.org/price#weather) without any additional registration @flobz In utils.py line 13, replace the 2.5 by 3.0 so the line looks like: weather_rep = requests.get("https://api.openweathermap.org/data/3.0/onecall",