Skip to content

Allow inverting netdata sensor values #21711

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

Merged
merged 3 commits into from
Mar 11, 2019
Merged

Allow inverting netdata sensor values #21711

merged 3 commits into from
Mar 11, 2019

Conversation

michaelarnauts
Copy link
Contributor

@michaelarnauts michaelarnauts commented Mar 6, 2019

Description:

Netdata returns all bandwidth related sensors as positive/negative numbers related to the interface. When you are uploading at 10Mbps, the net.eno1 / sent sensor it will report a speed of -10 Mbps. When you are downloading at 10Mbps, the net.eno / received sensor will report a speed of 10 Mbps. This is only useful when you want to put both values on a graph. For automations, graphs, ... it's counter-intuitive that the value is negative also.

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

Example entry for configuration.yaml (if applicable):

- platform: netdata
  name: netdata-proxmox
  host: proxmox.local
  resources:
    internet_upstream:
      data_group: net.eno2
      element: sent
      icon: mdi:upload-network
      invert: true
    internet_downstream:
      data_group: net.eno2
      element: received
      icon: mdi:download-network

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.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@michaelarnauts michaelarnauts requested a review from fabaff as a code owner March 6, 2019 16:04
@ghost ghost assigned michaelarnauts Mar 6, 2019
@ghost ghost added the in progress label Mar 6, 2019
@@ -88,7 +91,7 @@ class NetdataSensor(Entity):
"""Implementation of a Netdata sensor."""

def __init__(
self, netdata, name, sensor, sensor_name, element, icon, unit):
self, netdata, name, sensor, sensor_name, element, icon, unit, invert):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (83 > 79 characters)

@frenck
Copy link
Member

frenck commented Mar 6, 2019

⚠️ I was unable to find a documentation PR (matching this code change) in our documentation repository.

Please, update the documentation and open a PR for it (be sure to create a documentation PR against the next branch) and link 🔗 this PR and the new documentation PR by mentioning the link to the PR's in both openings posts/descriptions.

🏷 I am adding the docs-missing label until this has been resolved.

@ghost ghost assigned fabaff Mar 7, 2019
@michaelarnauts
Copy link
Contributor Author

The tests seem to fail on No matching distribution found for amcrest==1.2.4

@fabaff
Copy link
Member

fabaff commented Mar 11, 2019

I will merge it and fix the remaining stuff later if needed.

@fabaff fabaff merged commit bc76055 into dev Mar 11, 2019
@delete-merged-branch delete-merged-branch bot deleted the netdata-invert branch March 11, 2019 15:14
@ghost ghost removed the in progress label Mar 11, 2019
@balloob balloob mentioned this pull request Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants