search

ioBroker / ioBroker.type-detector

Helper module (not adapter) to detect types of devices
MIT License
8 stars 9 forks source link

Weather type for AccuWeather driver added #8

Closed algar42 closed 4 years ago

algar42 commented 4 years ago

Accuweather has slightly different states compared to what we already have as Weather type, so I decided to add separate type in order not to break existing weather devices.

blackduck-copilot[bot] commented 4 years ago

Black Duck Security Report

Merging #8 into master will not change security risk.

Added Components

Clean: 4

Removed Components

Clean: 4

Click here to see full report

GermanBluefox commented 4 years ago

Thank you for PR, but I have a couple of questions. The types in type-detector are generic. And that means, that if some Adapter does not pass to one of generic types, so the existing type must be tuned or the adapter must be changed.

Here I can see, that similar to weatherForecast the accuWeatherForecast will be created. This is wrong.

algar42 commented 4 years ago

This is actually not wrong. I really do not want to install all the weather adapters, get APIs etc to check them all. Plus accuWeatherForecast created in a way to receive icon URLs and free-text states and designed specifically for custom lovelace card. The states from weatherForeacst and accuWeatherForecast types are different also by weather source - weatherForecast takes current weather from forecast.0 (which I believe was done because there were no current weather in the adapter for which this type was created), but accuweather has separate current weather. If you have several weather adapters installed including AccuWeather I don't mind if you merge these two types together keeping funcionality of both, but otherwise I would prefer to keep them separate at least for now until someone will test and merge them :)

GermanBluefox commented 4 years ago

Sorry I will not merge this PR, because accuWeatherForecast must be rewritten.