New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for sensors from Flu Near You #18136
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome. I had to look hard to find some small comments that you can consider :-)
In case you want to address my comments, I am not marking this as approved. But I am also fine with merging as is.
ATTR_ZIP_CODE = 'zip_code' | ||
|
||
DEFAULT_ATTRIBUTION = 'Data provided by Flu Near You' | ||
DEFAULT_SCAN_INTERVAL = timedelta(minutes=30) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes your sensors update every 30 seconds (the sensor
default) but new data will only be available every 30 minutes.
Consider naming this MIN_TIME_BETWEEN_UPDATES
instead and lower it a bit.
Then set SCAN_INTERVAL
to 30 minutes so Home Assistant actually only updates the sensor every 30 minutes.
client = await create_client(latitude, longitude, websession) | ||
except FluNearYouError as err: | ||
_LOGGER.error('There was an error while setting up: %s', err) | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you expect that it might work later (like when the network is back), you can raise PlatformNotReady
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
☝️ or just return
. Nothing is checking this return value.
client = await create_client(latitude, longitude, websession) | ||
except FluNearYouError as err: | ||
_LOGGER.error('There was an error while setting up: %s', err) | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
☝️ or just return
. Nothing is checking this return value.
if self._category == CATEGORY_CDC_REPORT: | ||
data = self.fny.data[CATEGORY_CDC_REPORT] | ||
self._attrs.update({ | ||
ATTR_REPORTED_DATE: data['week_date'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will be a KeyError
here if the data update errors.
elif self._category == CATEGORY_USER_REPORT: | ||
data = self.fny.data[CATEGORY_USER_REPORT] | ||
self._attrs.update({ | ||
ATTR_CITY: data['city'].split('(')[0], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
TYPE_USER_CHICK, TYPE_USER_DENGUE, TYPE_USER_FLU, | ||
TYPE_USER_LEPTO, TYPE_USER_SYMPTOMS)) | ||
else: | ||
self._state = data[self._kind] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
Thanks, @amelchio and @MartinHjelmare! 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Can be merged when build passes. |
Description:
This PR adds support for sensors from Flu Near You:
Related issue (if applicable): N/A
Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7345
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.