Skip to content
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 netgear_lte connection sensors #22558

Merged

Conversation

Projects
None yet
3 participants
@amelchio
Copy link
Contributor

amelchio commented Mar 30, 2019

Description:

This adds several sensors with information about the current connection.

(Introduced upstream in amelchio/eternalegypt#3)

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

Example entry for configuration.yaml (if applicable):

netgear_lte:
  - host: !secret lb2120_hostname
    password: !secret lb2120_password
    sensor:
      monitored_conditions:
        - radio_quality
        - rx_level
        - tx_level
        - upstream
        - connection
        - connection_text
        - connection_type
        - current_ps_service_type
        - register_network_display
        - roaming
        - current_band
        - cell_id

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox.
  • There is no commented out code in this PR.
@amelchio

This comment has been minimized.

Copy link
Contributor Author

amelchio commented Mar 30, 2019

Reopening with an additional sensor just requested in the forum.

@amelchio amelchio reopened this Mar 30, 2019

@wafflebot wafflebot bot added the in progress label Mar 30, 2019

@amelchio amelchio force-pushed the amelchio:netgear_lte-connection-sensors branch from 52a462d to 3087840 Mar 31, 2019

@rohankapoorcom
Copy link
Member

rohankapoorcom left a comment

LGTM.

@rohankapoorcom rohankapoorcom merged commit 282fd22 into home-assistant:dev Apr 1, 2019

13 checks passed

Hound No violations found. Woof!
ci/circleci: pre-install-all-requirements Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.7 Your tests passed on CircleCI!
Details
ci/circleci: pylint Your tests passed on CircleCI!
Details
ci/circleci: static-check Your tests passed on CircleCI!
Details
ci/circleci: test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: test 3.7 Your tests passed on CircleCI!
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 92.939%
Details

@wafflebot wafflebot bot removed the in progress label Apr 1, 2019

'rx_level': 'dBm',
'tx_level': 'dBm',
'upstream': None,
'wire_connected': None,

This comment has been minimized.

Copy link
@rohankapoorcom

rohankapoorcom Apr 1, 2019

Member

Actually, on second thought shouldn't wire_connected and mobile_connected be binary sensors?

This comment has been minimized.

Copy link
@amelchio

amelchio Apr 1, 2019

Author Contributor

Good point. For mobile_connected, states disconnected/standby/connected have been observed. Maybe I should name it mobile_status instead?

The wire_connected is currently binary but I am guessing we could discover more states in the future (from the undocumented API). I am not sure what to do in such a case but I would probably rename it along with the first one.

It seems like roaming would indeed be binary, though.

I will think a bit about this and make a follow-up PR before the next beta. Thanks!

unibeck pushed a commit to unibeck/home-assistant that referenced this pull request Apr 7, 2019

@amelchio amelchio referenced this pull request Apr 8, 2019

Merged

Binary sensors for netgear_lte #22902

9 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.