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 Iliad Italy (Mobile Telephony Provider) Sensor #19645

Merged
merged 13 commits into from Jan 27, 2019

Conversation

Projects
None yet
5 participants
@eliseomartelli
Copy link
Contributor

eliseomartelli commented Dec 29, 2018

Description:

Added Iliad Italy Sensor

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7981

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: iliad_italy
    username: USERNAME
    password: PASSWORD

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

eliseomartelli added some commits Dec 29, 2018

Show resolved Hide resolved .coveragerc Outdated
Show resolved Hide resolved homeassistant/components/sensor/iliad_italy.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/iliad_italy.py

@eliseomartelli eliseomartelli referenced this pull request Dec 29, 2018

Merged

Add Iliad Italy Documentation #7981

2 of 2 tasks complete
@eliseomartelli

This comment has been minimized.

Copy link
Contributor Author

eliseomartelli commented Dec 29, 2018

Added the documentation of this sensor here: home-assistant/home-assistant.io#7981

@dgomes

dgomes approved these changes Jan 3, 2019

Copy link
Member

dgomes left a comment

Looks good!

Might I advise you to implement throttling? Don't know how picky Iliad servers might be ;)

@eliseomartelli

This comment has been minimized.

Copy link
Contributor Author

eliseomartelli commented Jan 3, 2019

@dgomes Something like 30 minutes should be fine, together with 1 hour of scan interval.
Adding it now.

Edit: Added.

eliseomartelli added some commits Jan 4, 2019

@eliseomartelli

This comment has been minimized.

Copy link
Contributor Author

eliseomartelli commented Jan 4, 2019

@dgomes lowered the throttle after testing with Iliad servers

@dgomes

This comment has been minimized.

Copy link
Member

dgomes commented Jan 4, 2019

It might be OK if it's just you :) but if more Iliad users start using your sensor, it might become an issue for Iliad...

@eliseomartelli

This comment has been minimized.

Copy link
Contributor Author

eliseomartelli commented Jan 5, 2019

After broad testing with multiple requests per second and looking at how other apps are getting the data, 10 minutes of Throttling seems reasonable enough @dgomes.
An unofficial Android client goes as far as one request for UI change.

@WAPEETY

This comment has been minimized.

Copy link

WAPEETY commented Jan 9, 2019

hi, how i can add this on my home assistant?

@eliseomartelli

This comment has been minimized.

Copy link
Contributor Author

eliseomartelli commented Jan 9, 2019

@WAPEETY As soon as this component gets merged you can pull and use the dev branch (or wait till the next release)

@fabaff

fabaff approved these changes Jan 27, 2019

@fabaff fabaff merged commit 3d4f292 into home-assistant:dev Jan 27, 2019

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.08%) to 92.992%
Details

@wafflebot wafflebot bot removed the in progress label Jan 27, 2019

@eliseomartelli eliseomartelli deleted the eliseomartelli:iliad branch Jan 27, 2019

fredrike added a commit to fredrike/home-assistant that referenced this pull request Jan 30, 2019

Add Iliad Italy (Mobile Telephony Provider) Sensor (home-assistant#19645
)

* working state

* Attrs

* > requirements generated

* linting

* fixes

* coveragerc

* ordered imports and fixed auth

* Added Throttle

* moved throttle to decorator

* remove scan interval

* lower throttle

* moved to scan interval

* Add attribution

@balloob balloob referenced this pull request Feb 6, 2019

Merged

0.87.0 #20794

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment