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

Better default temperature sensor limits #7754

Merged
merged 2 commits into from Nov 17, 2017

Conversation

Projects
None yet
5 participants
@kd7lxl
Contributor

kd7lxl commented Nov 17, 2017

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

This pull request contains two patches to improve temperature sensor discovery.

One changes the default temperature sensor limits from scaled to static. Before this patch, the default temperature sensor limits were 70% through 160% of the current value at time of discovery. At 20C this resulted in a pretty useful range of 14-32C. However, for devices in spaces with lower temperatures, 3C for example, it set default temperature limits of 2.1-4.8C. At negative temperatures, the limits flip flop! This has nothing to do with the temperature specification of the device and for values near zero it causes alert flapping. My solution is to make the defaults static (yet relative), rather than scaled, so that single digit temperatures have ranges just as broad as double digit temperatures: $current - 10 through $current + 20. At a typical temperature of 20C, this is roughly equivalent to the limits set by the old scaling.

The other patch adds defaults for Mikrotik RouterOS hardware. A quick survey of Mikrotik hardware shows that most devices are rated for or close to -40C through 70C.

Rather than let LibreNMS choose defaults for these based on the temperature at the time of discovery, I've hardcoded the operating temperatures specified in the Mikrotik data sheets.

kd7lxl added some commits Nov 17, 2017

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Nov 17, 2017

CLA assistant check
All committers have signed the CLA.

CLAassistant commented Nov 17, 2017

CLA assistant check
All committers have signed the CLA.

@kd7lxl kd7lxl changed the title from Temperature to Better default temperature sensor limits Nov 17, 2017

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Nov 17, 2017

The inspection completed: No new issues

scrutinizer-notifier commented Nov 17, 2017

The inspection completed: No new issues

@laf laf added Blocker 🚫 and removed Blocker 🚫 labels Nov 17, 2017

@laf laf merged commit b42c7de into librenms:master Nov 17, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

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